Skip to content

Firestore: query.test.ts: improve the test that resumes a query with existence filter to actually validate the existence filter. #7149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions integration/firestore/gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function copyTests() {
testBase + '/integration/util/events_accumulator.ts',
testBase + '/integration/util/helpers.ts',
testBase + '/integration/util/settings.ts',
testBase + '/integration/util/testing_hooks_util.ts',
testBase + '/util/equality_matcher.ts',
testBase + '/util/promise.ts'
],
Expand Down
1 change: 1 addition & 0 deletions packages/firestore/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,4 @@ export type { ByteString as _ByteString } from './util/byte_string';
export { logWarn as _logWarn } from './util/log';
export { EmptyAuthCredentialsProvider as _EmptyAuthCredentialsProvider } from './api/credentials';
export { EmptyAppCheckTokenProvider as _EmptyAppCheckTokenProvider } from './api/credentials';
export { TestingHooks as _TestingHooks } from './util/testing_hooks';
5 changes: 5 additions & 0 deletions packages/firestore/src/remote/watch_change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { logDebug } from '../util/log';
import { primitiveComparator } from '../util/misc';
import { SortedMap } from '../util/sorted_map';
import { SortedSet } from '../util/sorted_set';
import { TestingHooks } from '../util/testing_hooks';

import { ExistenceFilter } from './existence_filter';
import { RemoteEvent, TargetChange } from './remote_event';
Expand Down Expand Up @@ -414,6 +415,10 @@ export class WatchChangeAggregator {
// snapshot with `isFromCache:true`.
this.resetTarget(targetId);
this.pendingTargetResets = this.pendingTargetResets.add(targetId);
TestingHooks.instance?.notifyOnExistenceFilterMismatch({
localCacheCount: currentSize,
existenceFilterCount: watchChange.existenceFilter.count
});
}
}
}
Expand Down
116 changes: 116 additions & 0 deletions packages/firestore/src/util/testing_hooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/**
* @license
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* Manages "testing hooks", hooks into the internals of the SDK to verify
* internal state and events during integration tests. Do not use this class
* except for testing purposes.
*
* There are two ways to retrieve the global singleton instance of this class:
* 1. The `instance` property, which returns null if the global singleton
* instance has not been created. Use this property if the caller should
* "do nothing" if there are no testing hooks registered, such as when
* delivering an event to notify registered callbacks.
* 2. The `getOrCreateInstance()` method, which creates the global singleton
* instance if it has not been created. Use this method if the instance is
* needed to, for example, register a callback.
*
* @internal
*/
export class TestingHooks {
private readonly onExistenceFilterMismatchCallbacks = new Map<
Symbol,
ExistenceFilterMismatchCallback
>();

private constructor() {}

/**
* Returns the singleton instance of this class, or null if it has not been
* initialized.
*/
static get instance(): TestingHooks | null {
return gTestingHooksSingletonInstance;
}

/**
* Returns the singleton instance of this class, creating it if is has never
* been created before.
*/
static getOrCreateInstance(): TestingHooks {
if (gTestingHooksSingletonInstance === null) {
gTestingHooksSingletonInstance = new TestingHooks();
}
return gTestingHooksSingletonInstance;
}

/**
* Registers a callback to be notified when an existence filter mismatch
* occurs in the Watch listen stream.
*
* The relative order in which callbacks are notified is unspecified; do not
* rely on any particular ordering. If a given callback is registered multiple
* times then it will be notified multiple times, once per registration.
*
* @param callback the callback to invoke upon existence filter mismatch.
*
* @return a function that, when called, unregisters the given callback; only
* the first invocation of the returned function does anything; all subsequent
* invocations do nothing.
*/
onExistenceFilterMismatch(
callback: ExistenceFilterMismatchCallback
): () => void {
const key = Symbol();
this.onExistenceFilterMismatchCallbacks.set(key, callback);
return () => this.onExistenceFilterMismatchCallbacks.delete(key);
}

/**
* Invokes all currently-registered `onExistenceFilterMismatch` callbacks.
* @param info Information about the existence filter mismatch.
*/
notifyOnExistenceFilterMismatch(info: ExistenceFilterMismatchInfo): void {
this.onExistenceFilterMismatchCallbacks.forEach(callback => callback(info));
}
}

/**
* Information about an existence filter mismatch, as specified to callbacks
* registered with `TestingUtils.onExistenceFilterMismatch()`.
*/
export interface ExistenceFilterMismatchInfo {
/** The number of documents that matched the query in the local cache. */
localCacheCount: number;

/**
* The number of documents that matched the query on the server, as specified
* in the ExistenceFilter message's `count` field.
*/
existenceFilterCount: number;
}

/**
* The signature of callbacks registered with
* `TestingUtils.onExistenceFilterMismatch()`.
*/
export type ExistenceFilterMismatchCallback = (
info: ExistenceFilterMismatchInfo
) => void;

/** The global singleton instance of `TestingHooks`. */
let gTestingHooksSingletonInstance: TestingHooks | null = null;
37 changes: 36 additions & 1 deletion packages/firestore/test/integration/api/query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import {
withTestDb
} from '../util/helpers';
import { USE_EMULATOR } from '../util/settings';
import { captureExistenceFilterMismatches } from '../util/testing_hooks_util';

apiDescribe('Queries', (persistence: boolean) => {
addEqualityMatcher();
Expand Down Expand Up @@ -2092,7 +2093,10 @@ apiDescribe('Queries', (persistence: boolean) => {
await new Promise(resolve => setTimeout(resolve, 10000));

// Resume the query and save the resulting snapshot for verification.
const snapshot2 = await getDocs(coll);
// Use some internal testing hooks to "capture" the existence filter
// mismatches to verify them.
const [existenceFilterMismatches, snapshot2] =
await captureExistenceFilterMismatches(() => getDocs(coll));

// Verify that the snapshot from the resumed query contains the expected
// documents; that is, that it contains the 50 documents that were _not_
Expand All @@ -2114,6 +2118,37 @@ apiDescribe('Queries', (persistence: boolean) => {
expectedDocumentIds
);
}

// Skip the verification of the existence filter mismatch when persistence
// is disabled because without persistence there is no resume token
// specified in the subsequent call to getDocs(), and, therefore, Watch
// will _not_ send an existence filter.
// TODO(b/272754156) Re-write this test using a snapshot listener instead
// of calls to getDocs() and remove this check for disabled persistence.
if (!persistence) {
return;
}

// Skip the verification of the existence filter mismatch when testing
// against the Firestore emulator because the Firestore emulator fails to
// to send an existence filter at all.
// TODO(b/270731363): Enable the verification of the existence filter
// mismatch once the Firestore emulator is fixed to send an existence
// filter.
if (USE_EMULATOR) {
return;
}

// Verify that Watch sent an existence filter with the correct counts when
// the query was resumed.
expect(
existenceFilterMismatches,
'existenceFilterMismatches'
).to.have.length(1);
const { localCacheCount, existenceFilterCount } =
existenceFilterMismatches[0];
expect(localCacheCount, 'localCacheCount').to.equal(100);
expect(existenceFilterCount, 'existenceFilterCount').to.equal(50);
});
}).timeout('90s');
});
Expand Down
67 changes: 67 additions & 0 deletions packages/firestore/test/integration/util/testing_hooks_util.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
* @license
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { _TestingHooks as TestingHooks } from './firebase_export';

/**
* Captures all existence filter mismatches in the Watch 'Listen' stream that
* occur during the execution of the given code block.
* @param callback The callback to invoke; during the invocation of this
* callback all existence filter mismatches will be captured.
* @return the captured existence filter mismatches and the result of awaiting
* the given callback.
*/
export async function captureExistenceFilterMismatches<T>(
callback: () => Promise<T>
): Promise<[ExistenceFilterMismatchInfo[], T]> {
const results: ExistenceFilterMismatchInfo[] = [];
const onExistenceFilterMismatchCallback = (
info: ExistenceFilterMismatchInfo
): void => {
results.push(info);
};

const unregister =
TestingHooks.getOrCreateInstance().onExistenceFilterMismatch(
onExistenceFilterMismatchCallback
);

let callbackResult: T;
try {
callbackResult = await callback();
} finally {
unregister();
}

return [results, callbackResult];
}

/**
* Information about an existence filter mismatch, captured during an invocation
* of `captureExistenceFilterMismatches()`.
*
* See the documentation of `TestingHooks.notifyOnExistenceFilterMismatch()`
* for the meaning of these values.
*
* TODO: Delete this "interface" definition and instead use the one from
* testing_hooks.ts. I tried to do this but couldn't figure out how to get it to
* work in a way that survived bundling and minification.
*/
export interface ExistenceFilterMismatchInfo {
localCacheCount: number;
existenceFilterCount: number;
}