From ab6286dee39fdc66dda67f0783004ac6a4b5074d Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 24 Jan 2024 15:18:30 -0500 Subject: [PATCH 01/33] add listen source, add integration tests --- packages/firestore/src/api.ts | 6 +- packages/firestore/src/api/reference_impl.ts | 21 +- packages/firestore/src/core/event_manager.ts | 76 +- .../firestore/src/core/firestore_client.ts | 12 +- .../firestore/src/core/sync_engine_impl.ts | 45 +- .../test/integration/api/bundle.test.ts | 58 +- .../test/integration/api/query.test.ts | 2 +- .../api/snasphotListenerSource.test.ts | 839 ++++++++++++++++++ .../test/unit/core/event_manager.test.ts | 33 +- 9 files changed, 1036 insertions(+), 56 deletions(-) create mode 100644 packages/firestore/test/integration/api/snasphotListenerSource.test.ts diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 510d95c8a89..bcfa6dc5f34 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -139,7 +139,11 @@ export { WhereFilterOp } from './api/filter'; -export { SnapshotListenOptions, Unsubscribe } from './api/reference_impl'; +export { + ListenSource, + SnapshotListenOptions, + Unsubscribe +} from './api/reference_impl'; export { TransactionOptions } from './api/transaction_options'; diff --git a/packages/firestore/src/api/reference_impl.ts b/packages/firestore/src/api/reference_impl.ts index 13cfb09f518..90914344f83 100644 --- a/packages/firestore/src/api/reference_impl.ts +++ b/packages/firestore/src/api/reference_impl.ts @@ -78,6 +78,21 @@ export interface SnapshotListenOptions { * changed. Default is false. */ readonly includeMetadataChanges?: boolean; + + /** + * Set the source the query listens to. Default to ListenSource.Default, which + * listens to both cache and server. + */ + readonly source?: ListenSource; +} + +/** Describe the source a query listens to. */ +export const enum ListenSource { + /** Listen to both cache and server changes */ + Default, + + /** Listen to changes in cache only */ + Cache } /** @@ -668,7 +683,8 @@ export function onSnapshot( reference = getModularInstance(reference); let options: SnapshotListenOptions = { - includeMetadataChanges: false + includeMetadataChanges: false, + source: ListenSource.Default }; let currArg = 0; if (typeof args[currArg] === 'object' && !isPartialObserver(args[currArg])) { @@ -677,7 +693,8 @@ export function onSnapshot( } const internalOptions = { - includeMetadataChanges: options.includeMetadataChanges + includeMetadataChanges: options.includeMetadataChanges, + source: options.source }; if (isPartialObserver(args[currArg])) { diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index 1723abd15d9..7bb9ed0959d 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -15,6 +15,7 @@ * limitations under the License. */ +import { ListenSource } from '../api/reference_impl'; import { debugAssert, debugCast } from '../util/assert'; import { wrapInUserErrorIfRecoverable } from '../util/async_queue'; import { FirestoreError } from '../util/error'; @@ -32,6 +33,14 @@ import { ChangeType, DocumentViewChange, ViewSnapshot } from './view_snapshot'; class QueryListenersInfo { viewSnap: ViewSnapshot | undefined = undefined; listeners: QueryListener[] = []; + + // Helper methods to filter by listener type + getCacheListeners(): QueryListener[] { + return this.listeners.filter(l => l.options.source === ListenSource.Cache); + } + getServerListeners(): QueryListener[] { + return this.listeners.filter(l => l.options.source !== ListenSource.Cache); + } } /** @@ -52,8 +61,13 @@ export interface Observer { * allows users to tree-shake the Watch logic. */ export interface EventManager { - onListen?: (query: Query) => Promise; - onUnlisten?: (query: Query) => Promise; + onListen?: ( + query: Query, + enableRemoteListen: boolean + ) => Promise; + onUnlisten?: (query: Query, disableRemoteListen: boolean) => Promise; + onRemoteStoreListen?: (query: Query) => Promise; + onRemoteStoreUnlisten?: (query: Query) => Promise; } export function newEventManager(): EventManager { @@ -71,9 +85,23 @@ export class EventManagerImpl implements EventManager { snapshotsInSyncListeners: Set> = new Set(); /** Callback invoked when a Query is first listen to. */ - onListen?: (query: Query) => Promise; + onListen?: ( + query: Query, + enableRemoteListen: boolean + ) => Promise; /** Callback invoked once all listeners to a Query are removed. */ - onUnlisten?: (query: Query) => Promise; + onUnlisten?: (query: Query, disableRemoteListen: boolean) => Promise; + + /** + * Callback invoked when a Query starts listening to the remote store, while + * already listening to the cache. + */ + onRemoteStoreListen?: (query: Query) => Promise; + /** + * Callback invoked when a Query stops listening to the remote store, while + * still listening to the cache. + */ + onRemoteStoreUnlisten?: (query: Query) => Promise; } export async function eventManagerListen( @@ -83,6 +111,11 @@ export async function eventManagerListen( const eventManagerImpl = debugCast(eventManager, EventManagerImpl); debugAssert(!!eventManagerImpl.onListen, 'onListen not set'); + debugAssert( + !!eventManagerImpl.onRemoteStoreListen, + 'onRemoteStoreListen not set' + ); + const query = listener.query; let firstListen = false; @@ -92,9 +125,17 @@ export async function eventManagerListen( queryInfo = new QueryListenersInfo(); } + const listenToServer = listener.options.source !== ListenSource.Cache; + const firstListenToServer = + listenToServer && queryInfo.getServerListeners().length === 0; + if (firstListen) { + // When first listening to a query, try { - queryInfo.viewSnap = await eventManagerImpl.onListen(query); + queryInfo.viewSnap = await eventManagerImpl.onListen( + query, + firstListenToServer + ); } catch (e) { const firestoreError = wrapInUserErrorIfRecoverable( e as Error, @@ -103,6 +144,8 @@ export async function eventManagerListen( listener.onError(firestoreError); return; } + } else if (firstListenToServer) { + await eventManagerImpl.onRemoteStoreListen(query); } eventManagerImpl.queries.set(query, queryInfo); @@ -132,8 +175,15 @@ export async function eventManagerUnlisten( const eventManagerImpl = debugCast(eventManager, EventManagerImpl); debugAssert(!!eventManagerImpl.onUnlisten, 'onUnlisten not set'); + debugAssert( + !!eventManagerImpl.onRemoteStoreUnlisten, + 'onRemoteStoreUnlisten not set' + ); + const query = listener.query; + const listenToServer = listener.options.source !== ListenSource.Cache; let lastListen = false; + let lastListenToServer = false; const queryInfo = eventManagerImpl.queries.get(query); if (queryInfo) { @@ -141,12 +191,16 @@ export async function eventManagerUnlisten( if (i >= 0) { queryInfo.listeners.splice(i, 1); lastListen = queryInfo.listeners.length === 0; + lastListenToServer = + listenToServer && queryInfo.getServerListeners().length === 0; } } if (lastListen) { eventManagerImpl.queries.delete(query); - return eventManagerImpl.onUnlisten(query); + return eventManagerImpl.onUnlisten(query, lastListenToServer); + } else if (lastListenToServer) { + return eventManagerImpl.onRemoteStoreUnlisten(query); } } @@ -250,6 +304,9 @@ export interface ListenOptions { * offline. */ readonly waitForSyncWhenOnline?: boolean; + + /** Set the source events raised from. */ + readonly source?: ListenSource; } /** @@ -265,7 +322,7 @@ export class QueryListener { */ private raisedInitialEvent = false; - private options: ListenOptions; + readonly options: ListenOptions; private snap: ViewSnapshot | null = null; @@ -359,6 +416,11 @@ export class QueryListener { return true; } + // Always raise event if listening to cache + if (this.options.source === ListenSource.Cache) { + return true; + } + // NOTE: We consider OnlineState.Unknown as online (it should become Offline // or Online if we wait long enough). const maybeOnline = onlineState !== OnlineState.Offline; diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index d1b66d86e2f..aea7c4b4606 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -86,7 +86,9 @@ import { syncEngineLoadBundle, syncEngineRegisterPendingWritesCallback, syncEngineUnlisten, - syncEngineWrite + syncEngineWrite, + triggerRemoteStoreListen, + triggerRemoteStoreUnlisten } from './sync_engine_impl'; import { Transaction } from './transaction'; import { TransactionOptions } from './transaction_options'; @@ -397,6 +399,14 @@ export async function getEventManager( null, onlineComponentProvider.syncEngine ); + eventManager.onRemoteStoreListen = triggerRemoteStoreListen.bind( + null, + onlineComponentProvider.syncEngine + ); + eventManager.onRemoteStoreUnlisten = triggerRemoteStoreUnlisten.bind( + null, + onlineComponentProvider.syncEngine + ); return eventManager; } diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index 18a1dc681f5..eb6ac925296 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -292,7 +292,8 @@ export function newSyncEngine( */ export async function syncEngineListen( syncEngine: SyncEngine, - query: Query + query: Query, + shouldListenToRemote: boolean = true ): Promise { const syncEngineImpl = ensureWatchCallbacks(syncEngine); @@ -319,6 +320,7 @@ export async function syncEngineListen( const status = syncEngineImpl.sharedClientState.addLocalQueryTarget( targetData.targetId ); + targetId = targetData.targetId; viewSnapshot = await initializeViewAndComputeSnapshot( syncEngineImpl, @@ -328,7 +330,7 @@ export async function syncEngineListen( targetData.resumeToken ); - if (syncEngineImpl.isPrimaryClient) { + if (syncEngineImpl.isPrimaryClient && shouldListenToRemote) { remoteStoreListen(syncEngineImpl.remoteStore, targetData); } } @@ -336,6 +338,22 @@ export async function syncEngineListen( return viewSnapshot; } +export async function triggerRemoteStoreListen( + syncEngine: SyncEngine, + query: Query +): Promise { + const syncEngineImpl = ensureWatchCallbacks(syncEngine); + + const targetData = await localStoreAllocateTarget( + syncEngineImpl.localStore, + queryToTarget(query) + ); + + if (syncEngineImpl.isPrimaryClient) { + remoteStoreListen(syncEngineImpl.remoteStore, targetData); + } +} + /** * Registers a view for a previously unknown query and computes its initial * snapshot. @@ -393,7 +411,8 @@ async function initializeViewAndComputeSnapshot( /** Stops listening to the query. */ export async function syncEngineUnlisten( syncEngine: SyncEngine, - query: Query + query: Query, + disableRemoteListen: boolean ): Promise { const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl); const queryView = syncEngineImpl.queryViewsByQuery.get(query)!; @@ -430,7 +449,9 @@ export async function syncEngineUnlisten( ) .then(() => { syncEngineImpl.sharedClientState.clearQueryState(queryView.targetId); - remoteStoreUnlisten(syncEngineImpl.remoteStore, queryView.targetId); + if (disableRemoteListen) { + remoteStoreUnlisten(syncEngineImpl.remoteStore, queryView.targetId); + } removeAndCleanupTarget(syncEngineImpl, queryView.targetId); }) .catch(ignoreIfPrimaryLeaseLoss); @@ -445,6 +466,22 @@ export async function syncEngineUnlisten( } } +export async function triggerRemoteStoreUnlisten( + syncEngine: SyncEngine, + query: Query +): Promise { + const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl); + const queryView = syncEngineImpl.queryViewsByQuery.get(query)!; + debugAssert( + !!queryView, + 'Trying to unlisten on query not found:' + stringifyQuery(query) + ); + const queries = syncEngineImpl.queriesByTarget.get(queryView.targetId)!; + if (queries.length === 1) { + remoteStoreUnlisten(syncEngineImpl.remoteStore, queryView.targetId); + } +} + /** * Initiates the write of local mutation batch which involves adding the * writes to the mutation queue, notifying the remote store about new diff --git a/packages/firestore/test/integration/api/bundle.test.ts b/packages/firestore/test/integration/api/bundle.test.ts index f06a8b4c7b8..f4d6c182de3 100644 --- a/packages/firestore/test/integration/api/bundle.test.ts +++ b/packages/firestore/test/integration/api/bundle.test.ts @@ -41,7 +41,35 @@ import { export const encoder = new TextEncoder(); -function verifySuccessProgress(p: LoadBundleTaskProgress): void { +/** + * Returns a valid bundle string from replacing project id in `BUNDLE_TEMPLATE` with the given + * db project id (also recalculate length prefixes). + */ +export function bundleString(db: Firestore): string { + const projectId: string = db.app.options.projectId!; + + // Extract elements from BUNDLE_TEMPLATE and replace the project ID. + const elements = BUNDLE_TEMPLATE.map(e => + e.replace('{0}', projectId).replace('(default)', db._databaseId.database) + ); + + // Recalculating length prefixes for elements that are not BundleMetadata. + let bundleContent = ''; + for (const element of elements.slice(1)) { + const length = encoder.encode(element).byteLength; + bundleContent += `${length}${element}`; + } + + // Update BundleMetadata with new totalBytes. + const totalBytes = encoder.encode(bundleContent).byteLength; + const metadata = JSON.parse(elements[0]); + metadata.metadata.totalBytes = totalBytes; + const metadataContent = JSON.stringify(metadata); + const metadataLength = encoder.encode(metadataContent).byteLength; + return `${metadataLength}${metadataContent}${bundleContent}`; +} + +export function verifySuccessProgress(p: LoadBundleTaskProgress): void { expect(p.taskState).to.equal('Success'); expect(p.bytesLoaded).to.be.equal(p.totalBytes); expect(p.documentsLoaded).to.equal(p.totalDocuments); @@ -77,34 +105,6 @@ apiDescribe('Bundles', persistence => { ]); } - /** - * Returns a valid bundle string from replacing project id in `BUNDLE_TEMPLATE` with the given - * db project id (also recalculate length prefixes). - */ - function bundleString(db: Firestore): string { - const projectId: string = db.app.options.projectId!; - - // Extract elements from BUNDLE_TEMPLATE and replace the project ID. - const elements = BUNDLE_TEMPLATE.map(e => - e.replace('{0}', projectId).replace('(default)', db._databaseId.database) - ); - - // Recalculating length prefixes for elements that are not BundleMetadata. - let bundleContent = ''; - for (const element of elements.slice(1)) { - const length = encoder.encode(element).byteLength; - bundleContent += `${length}${element}`; - } - - // Update BundleMetadata with new totalBytes. - const totalBytes = encoder.encode(bundleContent).byteLength; - const metadata = JSON.parse(elements[0]); - metadata.metadata.totalBytes = totalBytes; - const metadataContent = JSON.stringify(metadata); - const metadataLength = encoder.encode(metadataContent).byteLength; - return `${metadataLength}${metadataContent}${bundleContent}`; - } - it('load with documents only with on progress and promise interface', () => { return withTestDb(persistence, async db => { const progressEvents: LoadBundleTaskProgress[] = []; diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 7f1d10dd7c1..7468b39bfeb 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -2937,7 +2937,7 @@ apiDescribe('Hanging query issue - #7652', persistence => { } }); -function verifyDocumentChange( +export function verifyDocumentChange( change: DocumentChange, id: string, oldIndex: number, diff --git a/packages/firestore/test/integration/api/snasphotListenerSource.test.ts b/packages/firestore/test/integration/api/snasphotListenerSource.test.ts new file mode 100644 index 00000000000..274de2d554a --- /dev/null +++ b/packages/firestore/test/integration/api/snasphotListenerSource.test.ts @@ -0,0 +1,839 @@ +/** + * @license + * Copyright 2024 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 { expect } from 'chai'; + +import { EventsAccumulator } from '../util/events_accumulator'; +import { + addDoc, + collection, + deleteDoc, + disableNetwork, + doc, + enableNetwork, + getDocs, + limit, + limitToLast, + ListenSource, + loadBundle, + namedQuery, + onSnapshot, + orderBy, + query, + QuerySnapshot, + runTransaction, + setDoc, + updateDoc, + where +} from '../util/firebase_export'; +import { + apiDescribe, + toDataArray, + withTestCollection, + withTestDb +} from '../util/helpers'; + +import { bundleString, verifySuccessProgress } from './bundle.test'; +import { verifyDocumentChange } from './query.test'; + +apiDescribe('Snapshot Listener source options ', persistence => { + // eslint-disable-next-line no-restricted-properties + (persistence.gc === 'lru' ? describe : describe.skip)( + 'listen to persistence cache', + () => { + it('can listen to source==cache', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + await getDocs(coll); // Populate the cache. + + const storeEvent = new EventsAccumulator(); + const unsubscribe = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeEvent.storeEvent + ); + + let snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + await addDoc(coll, { k: 'b', sort: 1 }); + snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + + await storeEvent.assertNoAdditionalEvents(); + unsubscribe(); + }); + }); + + it('listen to cache would not be affected by online status change', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async (coll, db) => { + await getDocs(coll); // Populate the cache. + + const storeEvent = new EventsAccumulator(); + const unsubscribe = onSnapshot( + coll, + { includeMetadataChanges: true, source: ListenSource.Cache }, + storeEvent.storeEvent + ); + + const snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + await disableNetwork(db); + await enableNetwork(db); + + await storeEvent.assertNoAdditionalEvents(); + unsubscribe(); + }); + }); + + it('can attach multiple source==cache listeners', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + await getDocs(coll); // Populate the cache. + + const storeEvent = new EventsAccumulator(); + const unsubscribe1 = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeEvent.storeEvent + ); + + const unsubscribe2 = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeEvent.storeEvent + ); + + let snapshots = await storeEvent.awaitEvents(2); + expect(snapshots[0].metadata.fromCache).to.equal(true); + expect(toDataArray(snapshots[0])).to.deep.equal([ + { k: 'a', sort: 0 } + ]); + expect(snapshots[0].metadata.fromCache).to.equal( + snapshots[1].metadata.fromCache + ); + expect(toDataArray(snapshots[0])).to.deep.equal( + toDataArray(snapshots[1]) + ); + + await addDoc(coll, { k: 'b', sort: 1 }); + + snapshots = await storeEvent.awaitEvents(2); + expect(snapshots[0].metadata.fromCache).to.equal(true); + expect(toDataArray(snapshots[0])).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshots[0].metadata.fromCache).to.equal( + snapshots[1].metadata.fromCache + ); + expect(toDataArray(snapshots[0])).to.deep.equal( + toDataArray(snapshots[1]) + ); + + // Detach one listener, and do a local mutation. The other listener should not be affected. + unsubscribe1(); + + await addDoc(coll, { k: 'c', sort: 2 }); + + const snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 }, + { k: 'c', sort: 2 } + ]); + await storeEvent.assertNoAdditionalEvents(); + unsubscribe2(); + }); + }); + + // Two queries that mapped to the same target ID are referred to as + // "mirror queries". An example for a mirror query is a limitToLast() + // query and a limit() query that share the same backend Target ID. + // Since limitToLast() queries are sent to the backend with a modified + // orderBy() clause, they can map to the same target representation as + // limit() query, even if both queries appear separate to the user. + it('can listen/unlisten/relisten to mirror queries', () => { + const testDocs = { + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 }, + c: { k: 'c', sort: 1 } + }; + return withTestCollection(persistence, testDocs, async coll => { + await getDocs(coll); // Populate the cache. + + // Setup `limit` query + const storeLimitEvent = new EventsAccumulator(); + let limitUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc'), limit(2)), + { source: ListenSource.Cache }, + storeLimitEvent.storeEvent + ); + + // Setup mirroring `limitToLast` query + const storeLimitToLastEvent = new EventsAccumulator(); + let limitToLastUnlisten = onSnapshot( + query(coll, orderBy('sort', 'desc'), limitToLast(2)), + { source: ListenSource.Cache }, + storeLimitToLastEvent.storeEvent + ); + + // Verify both queries get expected results. + let snapshot = await storeLimitEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.fromCache).to.equal(true); + snapshot = await storeLimitToLastEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'b', sort: 1 }, + { k: 'a', sort: 0 } + ]); + expect(snapshot.metadata.fromCache).to.equal(true); + + // Unlisten then relisten limit query. + limitUnlisten(); + limitUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc'), limit(2)), + { source: ListenSource.Cache }, + storeLimitEvent.storeEvent + ); + + // Verify `limit` query still works. + snapshot = await storeLimitEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.fromCache).to.equal(true); + + // Add a document that would change the result set. + await addDoc(coll, { k: 'd', sort: -1 }); + + // Verify both queries get expected results. + snapshot = await storeLimitEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'd', sort: -1 }, + { k: 'a', sort: 0 } + ]); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(snapshot.metadata.fromCache).to.equal(true); + + snapshot = await storeLimitToLastEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'd', sort: -1 } + ]); + expect(snapshot.metadata.fromCache).to.equal(true); + + // Unlisten to limitToLast, update a doc, then relisten limitToLast. + limitToLastUnlisten(); + + await updateDoc(doc(coll, 'a'), { k: 'a', sort: -2 }); + limitToLastUnlisten = onSnapshot( + query(coll, orderBy('sort', 'desc'), limitToLast(2)), + { source: ListenSource.Cache }, + storeLimitToLastEvent.storeEvent + ); + + // Verify both queries get expected results. + snapshot = await storeLimitEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: -2 }, + { k: 'd', sort: -1 } + ]); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(snapshot.metadata.fromCache).to.equal(true); + + snapshot = await storeLimitToLastEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'd', sort: -1 }, + { k: 'a', sort: -2 } + ]); + expect(snapshot.metadata.fromCache).to.equal(true); + + limitUnlisten(); + limitToLastUnlisten(); + }); + }); + + it('can listen to default source first and then cache', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + // Listen to the query with default options, which will also populates the cache + const storeDefaultEvent = new EventsAccumulator(); + const defaultUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + storeDefaultEvent.storeEvent + ); + let snapshot = await storeDefaultEvent.awaitRemoteEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + // Listen to the same query from cache + const storeCacheEvent = new EventsAccumulator(); + const cacheUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeCacheEvent.storeEvent + ); + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + await storeDefaultEvent.assertNoAdditionalEvents(); + await storeCacheEvent.assertNoAdditionalEvents(); + + // Unlisten to the default listener first then cache. + defaultUnlisten(); + cacheUnlisten(); + }); + }); + + it('can listen to cache source first and then default', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + await getDocs(coll); // Populate the cache. + + // Listen to the cache + const storeCacheEvent = new EventsAccumulator(); + const cacheUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeCacheEvent.storeEvent + ); + let snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + // Listen to the same query with default options + const storeDefaultEvent = new EventsAccumulator(); + const defaultUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + storeDefaultEvent.storeEvent + ); + snapshot = await storeDefaultEvent.awaitRemoteEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + await storeDefaultEvent.assertNoAdditionalEvents(); + await storeCacheEvent.assertNoAdditionalEvents(); + + // Unlisten to the cache listener first then default. + cacheUnlisten(); + defaultUnlisten(); + }); + }); + + it('can unlisten to default source while still listening to cache', () => { + const testDocs = { + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } + }; + return withTestCollection(persistence, testDocs, async coll => { + // Listen to the query with both source options + const storeDefaultEvent = new EventsAccumulator(); + const defaultUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + storeDefaultEvent.storeEvent + ); + await storeDefaultEvent.awaitEvent(); + const storeCacheEvent = new EventsAccumulator(); + const cacheUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeCacheEvent.storeEvent + ); + await storeCacheEvent.awaitEvent(); + + // Unlisten to the default listener. + defaultUnlisten(); + await storeDefaultEvent.assertNoAdditionalEvents(); + + // Add a document that would change the result set. + await addDoc(coll, { k: 'c', sort: -1 }); + + // Verify listener to cache works as expected + const snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'c', sort: -1 }, + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + + await storeCacheEvent.assertNoAdditionalEvents(); + cacheUnlisten(); + }); + }); + + it('can unlisten to cache while still listening to server', () => { + const testDocs = { + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } + }; + return withTestCollection(persistence, testDocs, async coll => { + // Listen to the query with both source options + const storeDefaultEvent = new EventsAccumulator(); + const defaultUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + storeDefaultEvent.storeEvent + ); + await storeDefaultEvent.awaitEvent(); + const storeCacheEvent = new EventsAccumulator(); + const cacheUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeCacheEvent.storeEvent + ); + await storeCacheEvent.awaitEvent(); + + // Unlisten to the cache. + cacheUnlisten(); + await storeCacheEvent.assertNoAdditionalEvents(); + + // Add a document that would change the result set. + await addDoc(coll, { k: 'c', sort: -1 }); + + // Verify listener to server works as expected + const snapshot = await storeDefaultEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'c', sort: -1 }, + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + + await storeDefaultEvent.assertNoAdditionalEvents(); + defaultUnlisten(); + }); + }); + + it('can listen/un-listen/re-listen to same query with different source options', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + // Listen to the query with default options, which also populates the cache + const storeDefaultEvent = new EventsAccumulator(); + let defaultUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + storeDefaultEvent.storeEvent + ); + let snapshot = await storeDefaultEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(snapshot.metadata.fromCache).to.equal(false); + + // Listen to the same query from cache + const storeCacheEvent = new EventsAccumulator(); + let cacheUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeCacheEvent.storeEvent + ); + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + // The metadata is sync with server due to the default listener + expect(snapshot.metadata.fromCache).to.equal(false); + + // Un-listen to the default listener, add a doc and re-listen.. + defaultUnlisten(); + await addDoc(coll, { k: 'b', sort: 1 }); + + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + // This is failing, why? + // expect(snapshot.metadata.fromCache).to.equal(true); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + + defaultUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + storeDefaultEvent.storeEvent + ); + snapshot = await storeDefaultEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.fromCache).to.equal(false); + // This is failing, why? + // expect(snapshot.metadata.hasPendingWrites).to.equal(false); + + // Unlisten to cache, update a doc, then re-listen to cache. + cacheUnlisten(); + await updateDoc(doc(coll, 'a'), { k: 'a', sort: 2 }); + + snapshot = await storeDefaultEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'b', sort: 1 }, + { k: 'a', sort: 2 } + ]); + expect(snapshot.metadata.fromCache).to.equal(false); + + cacheUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeCacheEvent.storeEvent + ); + + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'b', sort: 1 }, + { k: 'a', sort: 2 } + ]); + expect(snapshot.metadata.fromCache).to.equal(false); + + defaultUnlisten(); + cacheUnlisten(); + }); + }); + + it('will not get metadata only updates if only listening to cache', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + await getDocs(coll); // Populate the cache. + + const storeEvent = new EventsAccumulator(); + const unsubscribe = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { includeMetadataChanges: true, source: ListenSource.Cache }, + storeEvent.storeEvent + ); + + let snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + await addDoc(coll, { k: 'b', sort: 1 }); + + snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + + // As we are not listening to server, the listener will not get notified + // when local mutation is acknowledged by server. + await storeEvent.assertNoAdditionalEvents(); + unsubscribe(); + }); + }); + + it('will have synced metadata updates when listening to both cache and default source', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + const storeDefaultEvent = new EventsAccumulator(); + const defaultUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { includeMetadataChanges: true }, + storeDefaultEvent.storeEvent + ); + let snapshot = await storeDefaultEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(snapshot.metadata.hasPendingWrites).to.equal(false); + expect(snapshot.metadata.fromCache).to.equal(false); + + // Listen to the same query from cache + const storeCacheEvent = new EventsAccumulator(); + const cacheUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { includeMetadataChanges: true, source: ListenSource.Cache }, + storeCacheEvent.storeEvent + ); + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + // metadata is synced with the default source listener + expect(snapshot.metadata.hasPendingWrites).to.equal(false); + expect(snapshot.metadata.fromCache).to.equal(false); + + await addDoc(coll, { k: 'b', sort: 1 }); + + // snapshot gets triggered by local mutation + snapshot = await storeDefaultEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(snapshot.metadata.fromCache).to.equal(false); + + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(snapshot.metadata.fromCache).to.equal(false); + + // Local mutation gets acknowledged by the server + snapshot = await storeDefaultEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.hasPendingWrites).to.equal(false); + expect(snapshot.metadata.fromCache).to.equal(false); + + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.hasPendingWrites).to.equal(false); + expect(snapshot.metadata.fromCache).to.equal(false); + + cacheUnlisten(); + defaultUnlisten(); + }); + }); + + it('maintains correct DocumentChange indexes', async () => { + const testDocs = { + 'a': { order: 1 }, + 'b': { order: 2 }, + 'c': { 'order': 3 } + }; + await withTestCollection(persistence, testDocs, async coll => { + await getDocs(coll); // Populate the cache. + + const accumulator = new EventsAccumulator(); + const unsubscribe = onSnapshot( + query(coll, orderBy('order')), + { source: ListenSource.Cache }, + accumulator.storeEvent + ); + await accumulator + .awaitEvent() + .then(querySnapshot => { + const changes = querySnapshot.docChanges(); + expect(changes.length).to.equal(3); + verifyDocumentChange(changes[0], 'a', -1, 0, 'added'); + verifyDocumentChange(changes[1], 'b', -1, 1, 'added'); + verifyDocumentChange(changes[2], 'c', -1, 2, 'added'); + }) + .then(() => setDoc(doc(coll, 'b'), { order: 4 })) + .then(() => accumulator.awaitEvent()) + .then(querySnapshot => { + const changes = querySnapshot.docChanges(); + expect(changes.length).to.equal(1); + verifyDocumentChange(changes[0], 'b', 1, 2, 'modified'); + }) + .then(() => deleteDoc(doc(coll, 'c'))) + .then(() => accumulator.awaitEvent()) + .then(querySnapshot => { + const changes = querySnapshot.docChanges(); + expect(changes.length).to.equal(1); + verifyDocumentChange(changes[0], 'c', 1, -1, 'removed'); + }); + + unsubscribe(); + }); + }); + + it('can listen to composite index queries', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + await getDocs(coll); // Populate the cache. + + const query_ = query( + coll, + where('k', '<=', 'a'), + where('sort', '>=', 0) + ); + const storeEvent = new EventsAccumulator(); + const unsubscribe = onSnapshot( + query_, + { source: ListenSource.Cache }, + storeEvent.storeEvent + ); + + const snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + unsubscribe(); + }); + }); + + it('can raise initial snapshot from cache, even if it is empty', () => { + return withTestCollection(persistence, {}, async coll => { + let snapshot = await getDocs(coll); // Populate the cache. + expect(snapshot.metadata.fromCache).to.be.false; + expect(toDataArray(snapshot)).to.deep.equal([]); // Precondition check. + + // Add a snapshot listener whose first event should be raised from cache. + const storeEvent = new EventsAccumulator(); + onSnapshot( + coll, + { source: ListenSource.Cache }, + storeEvent.storeEvent + ); + snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.be.true; + expect(toDataArray(snapshot)).to.deep.equal([]); + }); + }); + + it('load with documents already pulled from backend', () => { + return withTestDb(persistence, async db => { + await setDoc(doc(db, 'coll-1/a'), { k: 'a', bar: 0 }); + await setDoc(doc(db, 'coll-1/b'), { k: 'b', bar: 0 }); + + const accumulator = new EventsAccumulator(); + onSnapshot( + collection(db, 'coll-1'), + { source: ListenSource.Cache }, + accumulator.storeEvent + ); + await accumulator.awaitEvent(); + const encoder = new TextEncoder(); + const progress = await loadBundle( + db, + // Testing passing in non-string bundles. + encoder.encode(bundleString(db)) + ); + + verifySuccessProgress(progress); + // The test bundle is holding ancient documents, so no events are + // generated as a result. The case where a bundle has newer doc than + // cache can only be tested in spec tests. + await accumulator.assertNoAdditionalEvents(); + + let snap = await getDocs((await namedQuery(db, 'limit'))!); + expect(toDataArray(snap)).to.deep.equal([{ k: 'b', bar: 0 }]); + + snap = await getDocs((await namedQuery(db, 'limit-to-last'))!); + expect(toDataArray(snap)).to.deep.equal([{ k: 'a', bar: 0 }]); + }); + }); + + it('Will not be triggered by transactions', () => { + return withTestCollection(persistence, {}, async (coll, db) => { + const accumulator = new EventsAccumulator(); + const unsubscribe = onSnapshot( + query(coll, orderBy('sort')), + { source: ListenSource.Cache }, + accumulator.storeEvent + ); + + const snapshot = await accumulator.awaitEvent(); + expect(snapshot.metadata.fromCache).to.be.true; + expect(toDataArray(snapshot)).to.deep.equal([]); + + const docRef = doc(coll); + // Use a transaction to perform a write without triggering any local events. + await runTransaction(db, async txn => { + txn.set(docRef, { k: 'a', sort: 0 }); + }); + + await accumulator.assertNoAdditionalEvents(); + unsubscribe(); + }); + }); + + it('triggered by server side updates when listening to both cache and default', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async (coll, db) => { + // Listen to the query with default options, which will also populates the cache + const storeDefaultEvent = new EventsAccumulator(); + const defaultUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + storeDefaultEvent.storeEvent + ); + let snapshot = await storeDefaultEvent.awaitRemoteEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + // Listen to the same query from cache + const storeCacheEvent = new EventsAccumulator(); + const cacheUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { source: ListenSource.Cache }, + storeCacheEvent.storeEvent + ); + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + let docRef = doc(coll); + // Use a transaction to perform a write without triggering any local events. + await runTransaction(db, async txn => { + txn.set(docRef, { k: 'b', sort: 1 }); + }); + + snapshot = await storeDefaultEvent.awaitRemoteEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.fromCache).to.be.false; + + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.fromCache).to.be.false; + + defaultUnlisten(); + docRef = doc(coll); + // Use a transaction to perform a write without triggering any local events. + await runTransaction(db, async txn => { + txn.set(docRef, { k: 'c', sort: 2 }); + }); + + // Add a document that would change the result set. + await addDoc(coll, { k: 'c', sort: -1 }); + + // Verify listener to cache works as expected + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'c', sort: -1 }, + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + cacheUnlisten(); + }); + }); + } + ); +}); diff --git a/packages/firestore/test/unit/core/event_manager.test.ts b/packages/firestore/test/unit/core/event_manager.test.ts index 293514ceb6d..24371b90dc0 100644 --- a/packages/firestore/test/unit/core/event_manager.test.ts +++ b/packages/firestore/test/unit/core/event_manager.test.ts @@ -25,7 +25,8 @@ import { newEventManager, eventManagerOnWatchChange, QueryListener, - eventManagerOnOnlineStateChange + eventManagerOnOnlineStateChange, + EventManager } from '../../../src/core/event_manager'; import { Query } from '../../../src/core/query'; import { OnlineState } from '../../../src/core/types'; @@ -50,6 +51,7 @@ describe('EventManager', () => { function fakeQueryListener(query: Query): any { return { query, + options: {}, onViewSnapshot: () => {}, onError: () => {}, applyOnlineStateChange: () => {} @@ -57,22 +59,34 @@ describe('EventManager', () => { } // mock objects. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let onListenSpy: any, onUnlistenSpy: any; + /* eslint-disable @typescript-eslint/no-explicit-any */ + let onListenSpy: any, + onUnlistenSpy: any, + onRemoteStoreListenSpy: any, + onRemoteStoreUnlistenSpy: any; + /* eslint-enable @typescript-eslint/no-explicit-any */ beforeEach(() => { onListenSpy = sinon.stub().returns(Promise.resolve(0)); onUnlistenSpy = sinon.spy(); + onRemoteStoreListenSpy = sinon.spy(); + onRemoteStoreUnlistenSpy = sinon.spy(); }); + function eventManagerBindSpy(eventManager: EventManager): void { + eventManager.onListen = onListenSpy.bind(null); + eventManager.onUnlisten = onUnlistenSpy.bind(null); + eventManager.onRemoteStoreListen = onRemoteStoreListenSpy.bind(null); + eventManager.onRemoteStoreUnlisten = onRemoteStoreUnlistenSpy.bind(null); + } + it('handles many listenables per query', async () => { const query1 = query('foo/bar'); const fakeListener1 = fakeQueryListener(query1); const fakeListener2 = fakeQueryListener(query1); const eventManager = newEventManager(); - eventManager.onListen = onListenSpy.bind(null); - eventManager.onUnlisten = onUnlistenSpy.bind(null); + eventManagerBindSpy(eventManager); await eventManagerListen(eventManager, fakeListener1); expect(onListenSpy.calledWith(query1)).to.be.true; @@ -92,8 +106,7 @@ describe('EventManager', () => { const fakeListener1 = fakeQueryListener(query1); const eventManager = newEventManager(); - eventManager.onListen = onListenSpy.bind(null); - eventManager.onUnlisten = onUnlistenSpy.bind(null); + eventManagerBindSpy(eventManager); await eventManagerUnlisten(eventManager, fakeListener1); expect(onUnlistenSpy.callCount).to.equal(0); @@ -118,8 +131,7 @@ describe('EventManager', () => { }; const eventManager = newEventManager(); - eventManager.onListen = onListenSpy.bind(null); - eventManager.onUnlisten = onUnlistenSpy.bind(null); + eventManagerBindSpy(eventManager); await eventManagerListen(eventManager, fakeListener1); await eventManagerListen(eventManager, fakeListener2); @@ -150,8 +162,7 @@ describe('EventManager', () => { }; const eventManager = newEventManager(); - eventManager.onListen = onListenSpy.bind(null); - eventManager.onUnlisten = onUnlistenSpy.bind(null); + eventManagerBindSpy(eventManager); await eventManagerListen(eventManager, fakeListener1); expect(events).to.deep.equal([OnlineState.Unknown]); From 8d8b77177bd4fed4f28dab92c003aca5fd57bd9e Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 24 Jan 2024 16:10:08 -0500 Subject: [PATCH 02/33] add spec tests --- .../test/unit/specs/bundle_spec.test.ts | 2 +- .../unit/specs/listen_source_spec.test.ts | 423 ++++++++++++++++++ .../firestore/test/unit/specs/spec_builder.ts | 47 +- .../test/unit/specs/spec_test_runner.ts | 35 +- 4 files changed, 492 insertions(+), 15 deletions(-) create mode 100644 packages/firestore/test/unit/specs/listen_source_spec.test.ts diff --git a/packages/firestore/test/unit/specs/bundle_spec.test.ts b/packages/firestore/test/unit/specs/bundle_spec.test.ts index 2e63799d80d..5a88dc8691c 100644 --- a/packages/firestore/test/unit/specs/bundle_spec.test.ts +++ b/packages/firestore/test/unit/specs/bundle_spec.test.ts @@ -52,7 +52,7 @@ interface TestBundledQuery { limitType?: LimitType; } -function bundleWithDocumentAndQuery( +export function bundleWithDocumentAndQuery( testDoc: TestBundleDocument, testQuery?: TestBundledQuery ): string { diff --git a/packages/firestore/test/unit/specs/listen_source_spec.test.ts b/packages/firestore/test/unit/specs/listen_source_spec.test.ts new file mode 100644 index 00000000000..491c354a4ba --- /dev/null +++ b/packages/firestore/test/unit/specs/listen_source_spec.test.ts @@ -0,0 +1,423 @@ +/** + * @license + * Copyright 2024 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 { deletedDoc, doc, filter, query } from '../../util/helpers'; + +import { bundleWithDocumentAndQuery } from './bundle_spec.test'; +import { describeSpec, specTest } from './describe_spec'; +import { spec } from './spec_builder'; + +describeSpec('Listens source options:', [], () => { + specTest( + 'Contents of query are cleared when listen is removed.', + ['eager-gc'], + 'Explicitly tests eager GC behavior', + () => { + const query_ = query('collection'); + const docA = doc('collection/a', 0, { key: 'a' }).setHasLocalMutations(); + return ( + spec() + .userSets('collection/a', { key: 'a' }) + .userListensToCache(query_) + .expectEvents(query_, { + added: [docA], + hasPendingWrites: true, + fromCache: true + }) + .userUnlistensToCache(query_) + .writeAcks('collection/a', 1000) + // Cache is empty as docA is GCed. + .userListensToCache(query_) + .expectEvents(query_, { added: [], fromCache: true }) + ); + } + ); + + specTest( + 'Documents are cleared when listen is removed.', + ['eager-gc'], + '', + () => { + const filteredQuery = query('collection', filter('matches', '==', true)); + const unfilteredQuery = query('collection'); + const docA = doc('collection/a', 0, { + matches: true + }).setHasLocalMutations(); + const docB = doc('collection/b', 0, { + matches: true + }).setHasLocalMutations(); + return ( + spec() + .userSets('collection/a', { matches: true }) + .userSets('collection/b', { matches: true }) + .userListensToCache(filteredQuery) + .expectEvents(filteredQuery, { + added: [docA, docB], + hasPendingWrites: true, + fromCache: true + }) + .writeAcks('collection/a', 1000) + .writeAcks('collection/b', 2000) + .userSets('collection/b', { matches: false }) + // DocB doesn't match because of a pending mutation + .expectEvents(filteredQuery, { + removed: [docB], + hasPendingWrites: true, + fromCache: true + }) + .userUnlistensToCache(filteredQuery) + .writeAcks('collection/b', 3000) + // Should get no events since documents were GCed + .userListensToCache(unfilteredQuery) + .expectEvents(unfilteredQuery, { added: [], fromCache: true }) + .userUnlistensToCache(unfilteredQuery) + ); + } + ); + + specTest("Doesn't include unknown documents in cached result", [], () => { + const query_ = query('collection'); + const existingDoc = doc('collection/exists', 0, { + key: 'a' + }).setHasLocalMutations(); + return spec() + .userSets('collection/exists', { key: 'a' }) + .userPatches('collection/unknown', { key: 'b' }) + .userListensToCache(query_) + .expectEvents(query_, { + added: [existingDoc], + fromCache: true, + hasPendingWrites: true + }); + }); + + specTest("Doesn't raise 'hasPendingWrites' for deletes", [], () => { + const query_ = query('collection'); + const docA = doc('collection/a', 1000, { key: 'a' }); + + return ( + spec() + .ensureManualLruGC() + // Populate the cache first + .userListens(query_) + .watchAcksFull(query_, 1000, docA) + .expectEvents(query_, { added: [docA] }) + .userUnlistens(query_) + .watchRemoves(query_) + + .userListensToCache(query_) + .expectEvents(query_, { added: [docA], fromCache: true }) + .userDeletes('collection/a') + .expectEvents(query_, { removed: [docA], fromCache: true }) + .writeAcks('collection/a', 2000) + .watchSends({ affects: [query_] }, deletedDoc('collection/a', 2000)) + .watchSnapshots(2000) + ); + }); + + specTest('onSnapshotsInSync fires for multiple listeners', [], () => { + const query1 = query('collection'); + const docAv1 = doc('collection/a', 1000, { v: 1 }); + const docAv2Local = doc('collection/a', 1000, { + v: 2 + }).setHasLocalMutations(); + const docAv3Local = doc('collection/a', 1000, { + v: 3 + }).setHasLocalMutations(); + const docAv4Local = doc('collection/a', 1000, { + v: 4 + }).setHasLocalMutations(); + + return ( + spec() + .ensureManualLruGC() + // Populate the cache first + .userListens(query1) + .watchAcksFull(query1, 1000, docAv1) + .expectEvents(query1, { added: [docAv1] }) + .userUnlistens(query1) + .watchRemoves(query1) + + .userListensToCache(query1) + .expectEvents(query1, { added: [docAv1], fromCache: true }) + .userAddsSnapshotsInSyncListener() + .expectSnapshotsInSyncEvent() + .userSets('collection/a', { v: 2 }) + .expectEvents(query1, { + hasPendingWrites: true, + modified: [docAv2Local], + fromCache: true + }) + .expectSnapshotsInSyncEvent() + .userAddsSnapshotsInSyncListener() + .expectSnapshotsInSyncEvent() + .userAddsSnapshotsInSyncListener() + .expectSnapshotsInSyncEvent() + .userSets('collection/a', { v: 3 }) + .expectEvents(query1, { + hasPendingWrites: true, + modified: [docAv3Local], + fromCache: true + }) + .expectSnapshotsInSyncEvent(3) + .userRemovesSnapshotsInSyncListener() + .userSets('collection/a', { v: 4 }) + .expectEvents(query1, { + hasPendingWrites: true, + modified: [docAv4Local], + fromCache: true + }) + .expectSnapshotsInSyncEvent(2) + ); + }); + + specTest('Empty initial snapshot is raised from cache', [], () => { + const query1 = query('collection'); + return ( + spec() + // Disable GC so the cache persists across listens. + .ensureManualLruGC() + // Populate the cache with the empty query results. + .userListens(query1) + .watchAcksFull(query1, 1000) + .expectEvents(query1, { fromCache: false }) + .userUnlistens(query1) + .watchRemoves(query1) + // Listen to the query again and verify that the empty snapshot is + // raised from cache. + .userListensToCache(query1, { resumeToken: 'resume-token-1000' }) + .expectEvents(query1, { fromCache: true }) + ); + }); + + specTest( + 'Empty-due-to-delete initial snapshot is raised from cache', + [], + () => { + const query1 = query('collection'); + const doc1 = doc('collection/a', 1000, { v: 1 }); + return ( + spec() + // Disable GC so the cache persists across listens. + .ensureManualLruGC() + // Populate the cache with the empty query results. + .userListens(query1) + .watchAcksFull(query1, 1000, doc1) + .expectEvents(query1, { added: [doc1] }) + .userUnlistens(query1) + .watchRemoves(query1) + // Delete the only document in the result set locally on the client. + .userDeletes('collection/a') + // Listen to the query again and verify that the empty snapshot is + // raised from cache, even though the write is not yet acknowledged. + .userListensToCache(query1, { resumeToken: 'resume-token-1000' }) + .expectEvents(query1, { fromCache: true }) + ); + } + ); + + specTest('Newer docs from bundles should overwrite cache', [], () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { value: 'a' }); + const docAChanged = doc('collection/a', 2999, { value: 'b' }); + + const bundleString = bundleWithDocumentAndQuery({ + key: docA.key, + readTime: 3000, + createTime: 1999, + updateTime: 2999, + content: { value: 'b' } + }); + + return ( + spec() + .ensureManualLruGC() + // Populate the cache first + .userListens(query1) + .watchAcksFull(query1, 1000, docA) + .expectEvents(query1, { added: [docA] }) + .userUnlistens(query1) + .watchRemoves(query1) + + .userListensToCache(query1) + .expectEvents(query1, { added: [docA], fromCache: true }) + .loadBundle(bundleString) + .expectEvents(query1, { modified: [docAChanged], fromCache: true }) + ); + }); + + specTest( + 'Newer deleted docs from bundles should delete cached docs', + [], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { value: 'a' }); + const bundleString = bundleWithDocumentAndQuery({ + key: docA.key, + readTime: 3000 + }); + + return ( + spec() + .ensureManualLruGC() + // Populate the cache first + .userListens(query1) + .watchAcksFull(query1, 1000, docA) + .expectEvents(query1, { added: [docA] }) + .userUnlistens(query1) + .watchRemoves(query1) + + .userListensToCache(query1) + .expectEvents(query1, { added: [docA], fromCache: true }) + .loadBundle(bundleString) + .expectEvents(query1, { removed: [docA], fromCache: true }) + ); + } + ); + + specTest('Older deleted docs from bundles should do nothing', [], () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { value: 'a' }); + const bundleString = bundleWithDocumentAndQuery({ + key: docA.key, + readTime: 999 + }); + + return ( + spec() + .ensureManualLruGC() + // Populate the cache first + .userListens(query1) + .watchAcksFull(query1, 1000, docA) + .expectEvents(query1, { added: [docA] }) + .userUnlistens(query1) + .watchRemoves(query1) + + .userListensToCache(query1) + .expectEvents(query1, { added: [docA], fromCache: true }) + // No events are expected here. + .loadBundle(bundleString) + ); + }); + + specTest( + 'Newer docs from bundles should keep not raise snapshot if there are unacknowledged writes', + [], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 250, { value: 'a' }); + const bundleString = bundleWithDocumentAndQuery({ + key: docA.key, + readTime: 1001, + createTime: 250, + updateTime: 1001, + content: { value: 'fromBundle' } + }); + + return ( + spec() + .ensureManualLruGC() + // Populate the cache first + .userListens(query1) + .watchAcksFull(query1, 250, docA) + .expectEvents(query1, { added: [docA] }) + .userUnlistens(query1) + .watchRemoves(query1) + + .userListensToCache(query1) + .expectEvents(query1, { + added: [doc('collection/a', 250, { value: 'a' })], + fromCache: true + }) + .userPatches('collection/a', { value: 'patched' }) + .expectEvents(query1, { + modified: [ + doc('collection/a', 250, { + value: 'patched' + }).setHasLocalMutations() + ], + hasPendingWrites: true, + fromCache: true + }) + // Loading the bundle will not raise snapshots, because the + // mutation has not been acknowledged. + .loadBundle(bundleString) + ); + } + ); + + specTest( + 'Newer docs from bundles should raise snapshot only when Watch catches up with acknowledged writes', + [], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 250, { value: 'a' }); + const bundleBeforeMutationAck = bundleWithDocumentAndQuery({ + key: docA.key, + readTime: 500, + createTime: 250, + updateTime: 500, + content: { value: 'b' } + }); + + const bundleAfterMutationAck = bundleWithDocumentAndQuery({ + key: docA.key, + readTime: 1001, + createTime: 250, + updateTime: 1001, + content: { value: 'fromBundle' } + }); + return ( + spec() + .ensureManualLruGC() + // Populate the cache first + .userListens(query1) + .watchAcksFull(query1, 250, docA) + .expectEvents(query1, { added: [docA] }) + .userUnlistens(query1) + .watchRemoves(query1) + + .userListensToCache(query1) + .expectEvents(query1, { + added: [doc('collection/a', 250, { value: 'a' })], + fromCache: true + }) + .userPatches('collection/a', { value: 'patched' }) + .expectEvents(query1, { + modified: [ + doc('collection/a', 250, { + value: 'patched' + }).setHasLocalMutations() + ], + hasPendingWrites: true, + fromCache: true + }) + .writeAcks('collection/a', 1000) + // loading bundleBeforeMutationAck will not raise snapshots, because its + // snapshot version is older than the acknowledged mutation. + .loadBundle(bundleBeforeMutationAck) + // loading bundleAfterMutationAck will raise a snapshot, because it is after + // the acknowledged mutation. + .loadBundle(bundleAfterMutationAck) + .expectEvents(query1, { + modified: [doc('collection/a', 1001, { value: 'fromBundle' })], + fromCache: true + }) + ); + } + ); +}); diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index 44dc623cfe1..df3a7d48f3e 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -16,7 +16,11 @@ */ import { IndexConfiguration } from '../../../src/api/index_configuration'; -import { ExpUserDataWriter } from '../../../src/api/reference_impl'; +import { + ExpUserDataWriter, + ListenSource +} from '../../../src/api/reference_impl'; +import { ListenOptions } from '../../../src/core/event_manager'; import { FieldFilter, Filter } from '../../../src/core/filter'; import { LimitType, @@ -266,7 +270,11 @@ export class SpecBuilder { return this; } - userListens(query: Query, resume?: ResumeSpec): this { + private addUserListenStep( + query: Query, + resume?: ResumeSpec, + options: ListenOptions = {} + ): void { this.nextStep(); const target = queryToTarget(query); @@ -275,7 +283,7 @@ export class SpecBuilder { if (this.injectFailures) { // Return a `userListens()` step but don't advance the target IDs. this.currentStep = { - userListen: { targetId, query: SpecBuilder.queryToSpec(query) } + userListen: { targetId, query: SpecBuilder.queryToSpec(query), options } }; } else { if (this.queryMapping.has(target)) { @@ -285,12 +293,30 @@ export class SpecBuilder { } this.queryMapping.set(target, targetId); - this.addQueryToActiveTargets(targetId, query, resume); + + if (options?.source !== ListenSource.Cache) { + // Active targets are created if listener is not sourcing from cache + this.addQueryToActiveTargets(targetId, query, resume); + } + this.currentStep = { - userListen: { targetId, query: SpecBuilder.queryToSpec(query) }, + userListen: { + targetId, + query: SpecBuilder.queryToSpec(query), + options + }, expectedState: { activeTargets: { ...this.activeTargets } } }; } + } + + userListens(query: Query, resume?: ResumeSpec): this { + this.addUserListenStep(query, resume); + return this; + } + + userListensToCache(query: Query, resume?: ResumeSpec): this { + this.addUserListenStep(query, resume, { source: ListenSource.Cache }); return this; } @@ -320,14 +346,16 @@ export class SpecBuilder { return this; } - userUnlistens(query: Query): this { + userUnlistens(query: Query, shouldRemoveWatchTarget: boolean = true): this { this.nextStep(); const target = queryToTarget(query); if (!this.queryMapping.has(target)) { throw new Error('Unlistening to query not listened to: ' + query); } const targetId = this.queryMapping.get(target)!; - this.removeQueryFromActiveTargets(query, targetId); + if (shouldRemoveWatchTarget) { + this.removeQueryFromActiveTargets(query, targetId); + } if (this.config.useEagerGCForMemory && !this.activeTargets[targetId]) { this.queryMapping.delete(target); @@ -341,6 +369,11 @@ export class SpecBuilder { return this; } + userUnlistensToCache(query: Query): this { + // Listener sourced from cache do not need to close watch stream. + return this.userUnlistens(query, /** shouldRemoveWatchTarget= */ false); + } + userSets(key: string, value: JsonObject): this { this.nextStep(); this.currentStep = { diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 1245a1c0231..72dea3b01ff 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -27,6 +27,7 @@ import { IndexConfiguration, parseIndexes } from '../../../src/api/index_configuration'; +import { ListenSource } from '../../../src/api/reference_impl'; import { User } from '../../../src/auth/user'; import { ComponentConfiguration } from '../../../src/core/component_provider'; import { DatabaseInfo } from '../../../src/core/database_info'; @@ -37,7 +38,8 @@ import { Observer, QueryListener, removeSnapshotsInSyncListener, - addSnapshotsInSyncListener + addSnapshotsInSyncListener, + ListenOptions } from '../../../src/core/event_manager'; import { canonifyQuery, @@ -59,7 +61,9 @@ import { syncEngineListen, syncEngineLoadBundle, syncEngineUnlisten, - syncEngineWrite + syncEngineWrite, + triggerRemoteStoreListen, + triggerRemoteStoreUnlisten } from '../../../src/core/sync_engine_impl'; import { TargetId } from '../../../src/core/types'; import { @@ -353,6 +357,15 @@ abstract class TestRunner { this.syncEngine ); + this.eventManager.onRemoteStoreListen = triggerRemoteStoreListen.bind( + null, + this.syncEngine + ); + this.eventManager.onRemoteStoreUnlisten = triggerRemoteStoreUnlisten.bind( + null, + this.syncEngine + ); + await this.persistence.setDatabaseDeletedListener(async () => { await this.shutdown(); }); @@ -482,11 +495,14 @@ abstract class TestRunner { } this.pushEvent(e); }); - // TODO(dimond): Allow customizing listen options in spec tests + const options = { - includeMetadataChanges: true, - waitForSyncWhenOnline: false + includeMetadataChanges: + listenSpec.options?.includeMetadataChanges ?? true, + waitForSyncWhenOnline: false, + source: listenSpec.options?.source ?? ListenSource.Default }; + const queryListener = new QueryListener(query, aggregator, options); this.queryListeners.set(query, queryListener); @@ -509,8 +525,12 @@ abstract class TestRunner { ); } - if (this.isPrimaryClient && this.networkEnabled) { - // Open should always have happened after a listen + if ( + this.isPrimaryClient && + this.networkEnabled && + options.source !== ListenSource.Cache + ) { + // Unless listened to cache, open always have happened after a listen. await this.connection.waitForWatchOpen(); } } @@ -1542,6 +1562,7 @@ export interface SpecStep { export interface SpecUserListen { targetId: TargetId; query: string | SpecQuery; + options?: ListenOptions; } /** [, ] */ From 3ab04f416214eb6f54b2c3a872d73be76196202b Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 25 Jan 2024 12:57:23 -0500 Subject: [PATCH 03/33] Update sync_engine_impl.ts --- packages/firestore/src/core/sync_engine_impl.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index eb6ac925296..844c413ea42 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -412,7 +412,7 @@ async function initializeViewAndComputeSnapshot( export async function syncEngineUnlisten( syncEngine: SyncEngine, query: Query, - disableRemoteListen: boolean + shouldUnlistenToRemote: boolean ): Promise { const syncEngineImpl = debugCast(syncEngine, SyncEngineImpl); const queryView = syncEngineImpl.queryViewsByQuery.get(query)!; @@ -449,7 +449,7 @@ export async function syncEngineUnlisten( ) .then(() => { syncEngineImpl.sharedClientState.clearQueryState(queryView.targetId); - if (disableRemoteListen) { + if (shouldUnlistenToRemote) { remoteStoreUnlisten(syncEngineImpl.remoteStore, queryView.targetId); } removeAndCleanupTarget(syncEngineImpl, queryView.targetId); From d334f75df0e155d1aa34cc2cfea637a6123e8d8f Mon Sep 17 00:00:00 2001 From: milaGGL Date: Thu, 25 Jan 2024 18:12:36 +0000 Subject: [PATCH 04/33] Update API reports --- common/api-review/firestore.api.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index ee4fcc842ff..0bf6e70bce9 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -350,6 +350,12 @@ export function limit(limit: number): QueryLimitConstraint; // @public export function limitToLast(limit: number): QueryLimitConstraint; +// @public +export const enum ListenSource { + Cache = 1, + Default = 0 +} + // @public export function loadBundle(firestore: Firestore, bundleData: ReadableStream | ArrayBuffer | string): LoadBundleTask; @@ -651,6 +657,7 @@ export function snapshotEqual(le // @public export interface SnapshotListenOptions { readonly includeMetadataChanges?: boolean; + readonly source?: ListenSource; } // @public From d1000a07abd0243eba170d35a005ed12dc2c417c Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 29 Jan 2024 15:26:24 -0500 Subject: [PATCH 05/33] add spec tests for multi-client --- docs-devsite/firestore_.md | 24 ++ .../firestore_.snapshotlistenoptions.md | 11 + .../firestore/src/core/sync_engine_impl.ts | 30 +- ...st.ts => snasphot_listener_source.test.ts} | 16 +- .../unit/specs/listen_source_spec.test.ts | 343 +++++++++++++++++- .../firestore/test/unit/specs/spec_builder.ts | 41 ++- 6 files changed, 449 insertions(+), 16 deletions(-) rename packages/firestore/test/integration/api/{snasphotListenerSource.test.ts => snasphot_listener_source.test.ts} (97%) diff --git a/docs-devsite/firestore_.md b/docs-devsite/firestore_.md index 07bb175ee86..7e213d118f0 100644 --- a/docs-devsite/firestore_.md +++ b/docs-devsite/firestore_.md @@ -157,6 +157,12 @@ https://github.com/firebase/firebase-js-sdk | [Transaction](./firestore_.transaction.md#transaction_class) | A reference to a transaction.The Transaction object passed to a transaction's updateFunction provides the methods to read and write data within the transaction context. See [runTransaction()](./firestore_.md#runtransaction_6f03ec4). | | [WriteBatch](./firestore_.writebatch.md#writebatch_class) | A write batch, used to perform multiple writes as a single atomic unit.A WriteBatch object can be acquired by calling [writeBatch()](./firestore_.md#writebatch_231a8e0). It provides methods for adding writes to the write batch. None of the writes will be committed (or visible locally) until [WriteBatch.commit()](./firestore_.writebatch.md#writebatchcommit) is called. | +## Enumerations + +| Enumeration | Description | +| --- | --- | +| [ListenSource](./firestore_.md#listensource) | Describe the source a query listens to. | + ## Interfaces | Interface | Description | @@ -2716,3 +2722,21 @@ export declare type WithFieldValue = T | (T extends Primitive ? T : T extends [K in keyof T]: WithFieldValue | FieldValue; } : never); ``` + +## ListenSource + +Describe the source a query listens to. + +Signature: + +```typescript +export declare const enum ListenSource +``` + +## Enumeration Members + +| Member | Value | Description | +| --- | --- | --- | +| Cache | 1 | Listen to changes in cache only | +| Default | 0 | Listen to both cache and server changes | + diff --git a/docs-devsite/firestore_.snapshotlistenoptions.md b/docs-devsite/firestore_.snapshotlistenoptions.md index 7551b0a2f23..2e11cc35ad1 100644 --- a/docs-devsite/firestore_.snapshotlistenoptions.md +++ b/docs-devsite/firestore_.snapshotlistenoptions.md @@ -23,6 +23,7 @@ export declare interface SnapshotListenOptions | Property | Type | Description | | --- | --- | --- | | [includeMetadataChanges](./firestore_.snapshotlistenoptions.md#snapshotlistenoptionsincludemetadatachanges) | boolean | Include a change even if only the metadata of the query or of a document changed. Default is false. | +| [source](./firestore_.snapshotlistenoptions.md#snapshotlistenoptionssource) | [ListenSource](./firestore_.md#listensource) | Set the source the query listens to. Default to ListenSource.Default, which listens to both cache and server. | ## SnapshotListenOptions.includeMetadataChanges @@ -33,3 +34,13 @@ Include a change even if only the metadata of the query or of a document changed ```typescript readonly includeMetadataChanges?: boolean; ``` + +## SnapshotListenOptions.source + +Set the source the query listens to. Default to ListenSource.Default, which listens to both cache and server. + +Signature: + +```typescript +readonly source?: ListenSource; +``` diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index 844c413ea42..38d335104db 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -317,9 +317,12 @@ export async function syncEngineListen( queryToTarget(query) ); - const status = syncEngineImpl.sharedClientState.addLocalQueryTarget( - targetData.targetId - ); + // Multi-tab + const status = shouldListenToRemote + ? syncEngineImpl.sharedClientState.addLocalQueryTarget( + targetData.targetId + ) + : 'not-current'; targetId = targetData.targetId; viewSnapshot = await initializeViewAndComputeSnapshot( @@ -349,6 +352,9 @@ export async function triggerRemoteStoreListen( queryToTarget(query) ); + // Multi-tab + syncEngineImpl.sharedClientState.addLocalQueryTarget(targetData.targetId); + if (syncEngineImpl.isPrimaryClient) { remoteStoreListen(syncEngineImpl.remoteStore, targetData); } @@ -477,6 +483,10 @@ export async function triggerRemoteStoreUnlisten( 'Trying to unlisten on query not found:' + stringifyQuery(query) ); const queries = syncEngineImpl.queriesByTarget.get(queryView.targetId)!; + + // Multi-tab + syncEngineImpl.sharedClientState.removeLocalQueryTarget(queryView.targetId); + if (queries.length === 1) { remoteStoreUnlisten(syncEngineImpl.remoteStore, queryView.targetId); } @@ -1541,8 +1551,12 @@ export async function syncEngineApplyActiveTargetsChange( } for (const targetId of added) { - if (syncEngineImpl.queriesByTarget.has(targetId)) { - // A target might have been added in a previous attempt + // A target might have been added in a previous attempt, or could have listened to cache, + // but no remote store connection has been established by the primary client. + const targetHasBeenListenedTo = + syncEngineImpl.queriesByTarget.has(targetId) && + syncEngineImpl.sharedClientState.isActiveQueryTarget(targetId); + if (targetHasBeenListenedTo) { logDebug(LOG_TAG, 'Adding an already active target ' + targetId); continue; } @@ -1580,7 +1594,11 @@ export async function syncEngineApplyActiveTargetsChange( /* keepPersistedTargetData */ false ) .then(() => { - remoteStoreUnlisten(syncEngineImpl.remoteStore, targetId); + // A target could have listened to cache only and no remote store connection has been + // established by the primary client. + if (syncEngineImpl.sharedClientState.isActiveQueryTarget(targetId)) { + remoteStoreUnlisten(syncEngineImpl.remoteStore, targetId); + } removeAndCleanupTarget(syncEngineImpl, targetId); }) .catch(ignoreIfPrimaryLeaseLoss); diff --git a/packages/firestore/test/integration/api/snasphotListenerSource.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts similarity index 97% rename from packages/firestore/test/integration/api/snasphotListenerSource.test.ts rename to packages/firestore/test/integration/api/snasphot_listener_source.test.ts index 274de2d554a..e6172f40fff 100644 --- a/packages/firestore/test/integration/api/snasphotListenerSource.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -74,6 +74,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); await addDoc(coll, { k: 'b', sort: 1 }); + snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.equal(true); expect(snapshot.metadata.hasPendingWrites).to.equal(true); @@ -301,6 +302,8 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); let snapshot = await storeDefaultEvent.awaitRemoteEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + // No cached result, only raise snapshot from server result + expect(snapshot.metadata.fromCache).to.equal(false); // Listen to the same query from cache const storeCacheEvent = new EventsAccumulator(); @@ -311,6 +314,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(snapshot.metadata.fromCache).to.equal(false); await storeDefaultEvent.assertNoAdditionalEvents(); await storeCacheEvent.assertNoAdditionalEvents(); @@ -337,15 +341,22 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); let snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(snapshot.metadata.fromCache).to.equal(true); // Listen to the same query with default options const storeDefaultEvent = new EventsAccumulator(); const defaultUnlisten = onSnapshot( query(coll, orderBy('sort', 'asc')), + { includeMetadataChanges: true }, storeDefaultEvent.storeEvent ); - snapshot = await storeDefaultEvent.awaitRemoteEvent(); + snapshot = await storeDefaultEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + // First snapshot will be raised from cache. + expect(snapshot.metadata.fromCache).to.equal(true); + snapshot = await storeDefaultEvent.awaitEvent(); + // Second snapshot raised from server result + expect(snapshot.metadata.fromCache).to.equal(false); await storeDefaultEvent.assertNoAdditionalEvents(); await storeCacheEvent.assertNoAdditionalEvents(); @@ -391,6 +402,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'a', sort: 0 }, { k: 'b', sort: 1 } ]); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); await storeCacheEvent.assertNoAdditionalEvents(); cacheUnlisten(); @@ -432,6 +444,8 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'a', sort: 0 }, { k: 'b', sort: 1 } ]); + expect(snapshot.metadata.fromCache).to.equal(false); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); await storeDefaultEvent.assertNoAdditionalEvents(); defaultUnlisten(); diff --git a/packages/firestore/test/unit/specs/listen_source_spec.test.ts b/packages/firestore/test/unit/specs/listen_source_spec.test.ts index 491c354a4ba..5f7e687bf4a 100644 --- a/packages/firestore/test/unit/specs/listen_source_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_source_spec.test.ts @@ -15,11 +15,12 @@ * limitations under the License. */ -import { deletedDoc, doc, filter, query } from '../../util/helpers'; +import { LimitType, queryWithLimit } from '../../../src/core/query'; +import { deletedDoc, doc, filter, orderBy, query } from '../../util/helpers'; import { bundleWithDocumentAndQuery } from './bundle_spec.test'; import { describeSpec, specTest } from './describe_spec'; -import { spec } from './spec_builder'; +import { client, spec } from './spec_builder'; describeSpec('Listens source options:', [], () => { specTest( @@ -420,4 +421,342 @@ describeSpec('Listens source options:', [], () => { ); } ); + + specTest( + 'Primary client should not invoke listen to server while all clients are listening to cache', + ['multi-client'], + () => { + const query1 = query('collection'); + + return client(0) + .becomeVisible() + .client(1) + .userListensToCache(query1) + .expectEvents(query1, { added: [], fromCache: true }) + .client(0) + .expectListenToCache(query1) + .client(1) + .userUnlistensToCache(query1) + .client(0) + .expectUnlistenToCache(query1); + } + ); + + specTest( + 'Local mutations notifies listeners that source==cache in all tabs', + ['multi-client'], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 0, { + key: 'a' + }).setHasLocalMutations(); + + return client(0) + .becomeVisible() + .userListensToCache(query1) + .expectEvents(query1, { added: [], fromCache: true }) + .client(1) + .userListensToCache(query1) + .expectEvents(query1, { added: [], fromCache: true }) + .client(0) + .userSets('collection/a', { key: 'a' }) + .expectEvents(query1, { + hasPendingWrites: true, + added: [docA], + fromCache: true + }) + .client(1) + .expectEvents(query1, { + hasPendingWrites: true, + added: [docA], + fromCache: true + }); + } + ); + + specTest( + 'Query is shared between primary and secondary clients', + ['multi-client'], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { key: 'a' }); + const docB = doc('collection/b', 2000, { key: 'a' }); + + return ( + client(0) + .becomeVisible() + // Listen to server in the primary client + .userListens(query1) + .watchAcksFull(query1, 1000, docA) + .expectEvents(query1, { added: [docA] }) + .client(1) + .userListensToCache(query1) + .expectEvents(query1, { added: [docA], fromCache: true }) + .client(2) + .userListensToCache(query1) + .expectEvents(query1, { added: [docA], fromCache: true }) + // Updates in the primary client notifies listeners sourcing from cache in secondary clients + .client(0) + .watchSends({ affects: [query1] }, docB) + .watchSnapshots(2000) + .expectEvents(query1, { added: [docB] }) + .client(1) + .expectEvents(query1, { added: [docB] }) + .client(2) + .expectEvents(query1, { added: [docB] }) + ); + } + ); + + specTest('Query is executed by primary client', ['multi-client'], () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { key: 'a' }); + + return ( + client(0) + .becomeVisible() + // Listen to cache in primary client + .userListensToCache(query1) + .expectEvents(query1, { added: [], fromCache: true }) + // Listen to server in primary client + .client(1) + .userListens(query1) + // The query is executed in the primary client + .client(0) + .expectListen(query1) + // Updates in the primary client notifies both listeners + .watchAcks(query1) + .watchSends({ affects: [query1] }, docA) + .watchSnapshots(1000) + .expectEvents(query1, { added: [docA], fromCache: true }) + .client(1) + .expectEvents(query1, { added: [docA], fromCache: true }) + .client(0) + .watchCurrents(query1, 'resume-token-2000') + .watchSnapshots(2000) + // Listeners in both tabs are in sync + .expectEvents(query1, { fromCache: false }) + .client(1) + .expectEvents(query1, { fromCache: false }) + ); + }); + + specTest( + 'Query only raises events in participating clients', + ['multi-client'], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { key: 'a' }); + + return client(0) + .becomeVisible() + .client(1) + .client(2) + .userListensToCache(query1) + .expectEvents(query1, { added: [], fromCache: true }) + .client(3) + .userListens(query1) + .client(0) // No events + .expectListen(query1) + .watchAcksFull(query1, 1000, docA) + .client(1) // No events + .client(2) + .expectEvents(query1, { added: [docA] }) + .client(3) + .expectEvents(query1, { added: [docA] }); + } + ); + + specTest('Mirror queries from different clients', ['multi-client'], () => { + const fullQuery = query('collection'); + const limit = queryWithLimit( + query('collection', orderBy('sort', 'asc')), + 2, + LimitType.First + ); + const limitToLast = queryWithLimit( + query('collection', orderBy('sort', 'desc')), + 2, + LimitType.Last + ); + const docA = doc('collection/a', 1000, { sort: 0 }); + const docB = doc('collection/b', 1000, { sort: 1 }); + const docC = doc('collection/c', 1000, { sort: 1 }); + const docCV2 = doc('collection/c', 2000, { + sort: -1 + }).setHasLocalMutations(); + + return ( + client(0) + .becomeVisible() + // Populate the cache first + .userListens(fullQuery) + .watchAcksFull(fullQuery, 1000, docA, docB, docC) + .expectEvents(fullQuery, { added: [docA, docB, docC] }) + .userUnlistens(fullQuery) + .watchRemoves(fullQuery) + + // Listen to mirror queries in 2 tabs + .userListensToCache(limit) + .expectEvents(limit, { added: [docA, docB], fromCache: true }) + .client(1) + .userListensToCache(limitToLast) + .expectEvents(limitToLast, { added: [docB, docA], fromCache: true }) + // Un-listen to the query in primary tab and do a local mutation + .client(0) + .userUnlistensToCache(limit) + .userSets('collection/c', { sort: -1 }) + // Listener in the other tab should work as expected + .client(1) + .expectEvents(limitToLast, { + hasPendingWrites: true, + added: [docCV2], + removed: [docB], + fromCache: true + }) + ); + }); + + specTest( + 'Mirror queries from same secondary client', + ['multi-client'], + () => { + const fullQuery = query('collection'); + const limit = queryWithLimit( + query('collection', orderBy('sort', 'asc')), + 2, + LimitType.First + ); + const limitToLast = queryWithLimit( + query('collection', orderBy('sort', 'desc')), + 2, + LimitType.Last + ); + const docA = doc('collection/a', 1000, { sort: 0 }); + const docB = doc('collection/b', 1000, { sort: 1 }); + const docC = doc('collection/c', 1000, { sort: 1 }); + const docCV2 = doc('collection/c', 2000, { + sort: -1 + }).setHasLocalMutations(); + + return ( + client(0) + .becomeVisible() + // Populate the cache first + .userListens(fullQuery) + .watchAcksFull(fullQuery, 1000, docA, docB, docC) + .expectEvents(fullQuery, { added: [docA, docB, docC] }) + .userUnlistens(fullQuery) + .watchRemoves(fullQuery) + + // Listen to mirror queries in a secondary tab + .client(1) + .userListensToCache(limit) + .expectEvents(limit, { added: [docA, docB], fromCache: true }) + .userListensToCache(limitToLast) + .expectEvents(limitToLast, { added: [docB, docA], fromCache: true }) + // Un-listen to one of the query and do a local mutation + .userUnlistensToCache(limit) + .userSets('collection/c', { sort: -1 }) + // The other listener should work as expected + .expectEvents(limitToLast, { + hasPendingWrites: true, + added: [docCV2], + removed: [docB], + fromCache: true + }) + ); + } + ); + + specTest( + 'Mirror queries from different sources while listening to server in primary tab', + ['multi-client'], + () => { + const limit = queryWithLimit( + query('collection', orderBy('sort', 'asc')), + 2, + LimitType.First + ); + const limitToLast = queryWithLimit( + query('collection', orderBy('sort', 'desc')), + 2, + LimitType.Last + ); + const docA = doc('collection/a', 1000, { sort: 0 }); + const docB = doc('collection/b', 1000, { sort: 1 }); + const docC = doc('collection/c', 2000, { sort: -1 }); + + return ( + // Listen to server in primary client + client(0) + .becomeVisible() + .userListens(limit) + .expectListen(limit) + .watchAcksFull(limit, 1000, docA, docB) + .expectEvents(limit, { added: [docA, docB] }) + //Listen to cache in secondary client + .client(1) + .userListensToCache(limitToLast) + .expectEvents(limitToLast, { added: [docB, docA], fromCache: true }) + // Watch sends document changes + .client(0) + .watchSends({ affects: [limit] }, docC) + .watchSnapshots(2000) + .expectEvents(limit, { added: [docC], removed: [docB] }) + // Cache listener gets notified as well. + .client(1) + .expectEvents(limitToLast, { added: [docC], removed: [docB] }) + // Un-listen to both queries + .client(0) + .userUnlistens(limit) + .client(1) + .userUnlistensToCache(limitToLast) + ); + } + ); + + specTest( + 'Mirror queries from different sources while listening to server in secondary tab', + ['multi-client'], + () => { + const limit = queryWithLimit( + query('collection', orderBy('sort', 'asc')), + 2, + LimitType.First + ); + const limitToLast = queryWithLimit( + query('collection', orderBy('sort', 'desc')), + 2, + LimitType.Last + ); + const docA = doc('collection/a', 1000, { sort: 0 }); + const docB = doc('collection/b', 1000, { sort: 1 }); + const docC = doc('collection/c', 2000, { sort: -1 }); + + return ( + client(0) + .becomeVisible() + // Listen to server in the secondary client + .client(1) + .userListens(limit) + .client(0) + .expectListen(limit) + .watchAcksFull(limit, 1000, docA, docB) + .client(1) + .expectEvents(limit, { added: [docA, docB] }) + + // Listen to cache in primary client + .client(0) + .userListensToCache(limitToLast) + .expectEvents(limitToLast, { added: [docB, docA], fromCache: true }) + // Watch sends document changes + .watchSends({ affects: [limit] }, docC) + .watchSnapshots(2000) + .expectEvents(limitToLast, { added: [docC], removed: [docB] }) + .client(1) + .expectEvents(limit, { added: [docC], removed: [docB] }) + ); + } + ); }); diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index df3a7d48f3e..cca4c5bcc81 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -962,30 +962,46 @@ export class SpecBuilder { return this; } - /** Registers a query that is active in another tab. */ - expectListen(query: Query, resume?: ResumeSpec): this { + private registerQuery( + query: Query, + shouldAddWatchTarget: boolean, + resume?: ResumeSpec + ): this { this.assertStep('Expectations require previous step'); const target = queryToTarget(query); const targetId = this.queryIdGenerator.cachedId(target); this.queryMapping.set(target, targetId); - this.addQueryToActiveTargets(targetId, query, resume); - + if (shouldAddWatchTarget) { + this.addQueryToActiveTargets(targetId, query, resume); + } const currentStep = this.currentStep!; currentStep.expectedState = currentStep.expectedState || {}; currentStep.expectedState.activeTargets = { ...this.activeTargets }; return this; } - /** Removes a query that is no longer active in any tab. */ - expectUnlisten(query: Query): this { + /** Registers a query that is active in another tab. */ + expectListen(query: Query, resume?: ResumeSpec): this { + return this.registerQuery(query, true, resume); + } + + /** Registers a query that is listening to cache and active in another tab. */ + expectListenToCache(query: Query, resume?: ResumeSpec): this { + // Listeners that source from cache would not send watch request. + return this.registerQuery(query, false, resume); + } + + removeQuery(query: Query, shouldRemoveWatchTarget: boolean = true): this { this.assertStep('Expectations require previous step'); const target = queryToTarget(query); const targetId = this.queryMapping.get(target)!; - this.removeQueryFromActiveTargets(query, targetId); + if (shouldRemoveWatchTarget) { + this.removeQueryFromActiveTargets(query, targetId); + } if (this.config.useEagerGCForMemory && !this.activeTargets[targetId]) { this.queryMapping.delete(target); @@ -998,6 +1014,17 @@ export class SpecBuilder { return this; } + /** Removes a query that is no longer active in any tab. */ + expectUnlisten(query: Query): this { + return this.removeQuery(query); + } + + /** Removes a query that is listening to cache and no longer active in any tab. */ + expectUnlistenToCache(query: Query): this { + // Listeners that source from cache did not establish watch connection, so no active targets to remove. + return this.removeQuery(query, false); + } + /** * Verifies the total number of requests sent to the write backend since test * initialization. From 3859f1242cf7037a3a398b11f37c76e6b994d0dc Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 29 Jan 2024 17:45:28 -0500 Subject: [PATCH 06/33] format --- packages/firestore/src/core/event_manager.ts | 5 +- .../unit/specs/listen_source_spec.test.ts | 50 +++++++++++++++++-- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index 7bb9ed0959d..95141c7b9e5 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -34,10 +34,7 @@ class QueryListenersInfo { viewSnap: ViewSnapshot | undefined = undefined; listeners: QueryListener[] = []; - // Helper methods to filter by listener type - getCacheListeners(): QueryListener[] { - return this.listeners.filter(l => l.options.source === ListenSource.Cache); - } + // Helper methods to filter listeners that sends watch request. getServerListeners(): QueryListener[] { return this.listeners.filter(l => l.options.source !== ListenSource.Cache); } diff --git a/packages/firestore/test/unit/specs/listen_source_spec.test.ts b/packages/firestore/test/unit/specs/listen_source_spec.test.ts index 5f7e687bf4a..c04d74a5f08 100644 --- a/packages/firestore/test/unit/specs/listen_source_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_source_spec.test.ts @@ -470,7 +470,11 @@ describeSpec('Listens source options:', [], () => { hasPendingWrites: true, added: [docA], fromCache: true - }); + }) + .userUnlistensToCache(query1) + .client(0) + .userUnlistensToCache(query1) + .expectUnlistenToCache(query1); } ); @@ -504,6 +508,46 @@ describeSpec('Listens source options:', [], () => { .expectEvents(query1, { added: [docB] }) .client(2) .expectEvents(query1, { added: [docB] }) + .client(0) + .userUnlistens(query1) + // There should be no active watch targets left + .expectActiveTargets() + ); + } + ); + + specTest( + 'Tabs can have multiple listeners with different sources', + ['multi-client'], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { key: 'a' }); + const docB = doc('collection/b', 2000, { key: 'a' }); + + return ( + client(0) + .becomeVisible() + // Listen to server in the primary client + .userListens(query1) + .watchAcksFull(query1, 1000, docA) + .expectEvents(query1, { added: [docA] }) + .userListensToCache(query1) + .expectEvents(query1, { added: [docA] }) + // Listen to server and cache in the secondary client + .client(1) + .userListens(query1) + .expectEvents(query1, { added: [docA] }) + .userListensToCache(query1) + .expectEvents(query1, { added: [docA] }) + // Updates in the primary client notifies all listeners + .client(0) + .watchSends({ affects: [query1] }, docB) + .watchSnapshots(2000) + .expectEvents(query1, { added: [docB] }) + .expectEvents(query1, { added: [docB] }) + .client(1) + .expectEvents(query1, { added: [docB] }) + .expectEvents(query1, { added: [docB] }) ); } ); @@ -567,7 +611,7 @@ describeSpec('Listens source options:', [], () => { } ); - specTest('Mirror queries from different clients', ['multi-client'], () => { + specTest('Mirror queries in different clients', ['multi-client'], () => { const fullQuery = query('collection'); const limit = queryWithLimit( query('collection', orderBy('sort', 'asc')), @@ -596,7 +640,7 @@ describeSpec('Listens source options:', [], () => { .userUnlistens(fullQuery) .watchRemoves(fullQuery) - // Listen to mirror queries in 2 tabs + // Listen to mirror queries in 2 different tabs .userListensToCache(limit) .expectEvents(limit, { added: [docA, docB], fromCache: true }) .client(1) From 8f9188ddd98e7013f521469e8b9f4ddfd62af3ca Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 29 Jan 2024 18:27:54 -0500 Subject: [PATCH 07/33] flaky test --- .../test/integration/api/snasphot_listener_source.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index e6172f40fff..f4a62ba50ea 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -488,9 +488,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'a', sort: 0 }, { k: 'b', sort: 1 } ]); - // This is failing, why? - // expect(snapshot.metadata.fromCache).to.equal(true); - expect(snapshot.metadata.hasPendingWrites).to.equal(true); defaultUnlisten = onSnapshot( query(coll, orderBy('sort', 'asc')), @@ -501,9 +498,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'a', sort: 0 }, { k: 'b', sort: 1 } ]); - expect(snapshot.metadata.fromCache).to.equal(false); - // This is failing, why? - // expect(snapshot.metadata.hasPendingWrites).to.equal(false); // Unlisten to cache, update a doc, then re-listen to cache. cacheUnlisten(); @@ -514,7 +508,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'b', sort: 1 }, { k: 'a', sort: 2 } ]); - expect(snapshot.metadata.fromCache).to.equal(false); cacheUnlisten = onSnapshot( query(coll, orderBy('sort', 'asc')), @@ -527,7 +520,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'b', sort: 1 }, { k: 'a', sort: 2 } ]); - expect(snapshot.metadata.fromCache).to.equal(false); defaultUnlisten(); cacheUnlisten(); From ba33e812712b5436d8e99a7d42a4f9abfb80e0a9 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 30 Jan 2024 15:34:58 -0500 Subject: [PATCH 08/33] format --- packages/firestore/src/core/event_manager.ts | 82 ++++-- .../firestore/src/core/sync_engine_impl.ts | 12 +- .../src/local/shared_client_state.ts | 2 +- .../api/snasphot_listener_source.test.ts | 32 +- .../unit/specs/listen_source_spec.test.ts | 278 ++++++++++-------- 5 files changed, 236 insertions(+), 170 deletions(-) diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index 95141c7b9e5..4e9acf050b0 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -34,8 +34,8 @@ class QueryListenersInfo { viewSnap: ViewSnapshot | undefined = undefined; listeners: QueryListener[] = []; - // Helper methods to filter listeners that sends watch request. - getServerListeners(): QueryListener[] { + // Helper methods to filter listeners that listens to watch changes. + getRemoteListeners(): QueryListener[] { return this.listeners.filter(l => l.options.source !== ListenSource.Cache); } } @@ -101,17 +101,29 @@ export class EventManagerImpl implements EventManager { onRemoteStoreUnlisten?: (query: Query) => Promise; } -export async function eventManagerListen( - eventManager: EventManager, - listener: QueryListener -): Promise { - const eventManagerImpl = debugCast(eventManager, EventManagerImpl); +function listensToRemoteStore(listener: QueryListener): boolean { + return listener.options.source !== ListenSource.Cache; +} +function validateEventManager(eventManagerImpl: EventManagerImpl): void { debugAssert(!!eventManagerImpl.onListen, 'onListen not set'); debugAssert( !!eventManagerImpl.onRemoteStoreListen, 'onRemoteStoreListen not set' ); + debugAssert(!!eventManagerImpl.onUnlisten, 'onUnlisten not set'); + debugAssert( + !!eventManagerImpl.onRemoteStoreUnlisten, + 'onRemoteStoreUnlisten not set' + ); +} + +export async function eventManagerListen( + eventManager: EventManager, + listener: QueryListener +): Promise { + const eventManagerImpl = debugCast(eventManager, EventManagerImpl); + validateEventManager(eventManagerImpl); const query = listener.query; let firstListen = false; @@ -122,16 +134,17 @@ export async function eventManagerListen( queryInfo = new QueryListenersInfo(); } - const listenToServer = listener.options.source !== ListenSource.Cache; - const firstListenToServer = - listenToServer && queryInfo.getServerListeners().length === 0; + const firstListenToRemoteStore = + listensToRemoteStore(listener) && + queryInfo.getRemoteListeners().length === 0; if (firstListen) { - // When first listening to a query, + // When listening to a query for the first time, it may or may not establish + // watch connection based on the source the query is listening to. try { - queryInfo.viewSnap = await eventManagerImpl.onListen( + queryInfo.viewSnap = await eventManagerImpl.onListen!( query, - firstListenToServer + firstListenToRemoteStore ); } catch (e) { const firestoreError = wrapInUserErrorIfRecoverable( @@ -141,8 +154,21 @@ export async function eventManagerListen( listener.onError(firestoreError); return; } - } else if (firstListenToServer) { - await eventManagerImpl.onRemoteStoreListen(query); + } else if (firstListenToRemoteStore) { + // A query might have listened to previously, but no watch connection is created + // as it has been listening to cache only. + try { + await eventManagerImpl.onRemoteStoreListen!(query); + } catch (e) { + const firestoreError = wrapInUserErrorIfRecoverable( + e as Error, + `Initialization of remote connection for query '${stringifyQuery( + listener.query + )}' failed` + ); + listener.onError(firestoreError); + return; + } } eventManagerImpl.queries.set(query, queryInfo); @@ -170,34 +196,34 @@ export async function eventManagerUnlisten( listener: QueryListener ): Promise { const eventManagerImpl = debugCast(eventManager, EventManagerImpl); - - debugAssert(!!eventManagerImpl.onUnlisten, 'onUnlisten not set'); - debugAssert( - !!eventManagerImpl.onRemoteStoreUnlisten, - 'onRemoteStoreUnlisten not set' - ); + validateEventManager(eventManagerImpl); const query = listener.query; - const listenToServer = listener.options.source !== ListenSource.Cache; let lastListen = false; - let lastListenToServer = false; + let lastListenToRemoteStore = false; const queryInfo = eventManagerImpl.queries.get(query); if (queryInfo) { const i = queryInfo.listeners.indexOf(listener); if (i >= 0) { queryInfo.listeners.splice(i, 1); + lastListen = queryInfo.listeners.length === 0; - lastListenToServer = - listenToServer && queryInfo.getServerListeners().length === 0; + lastListenToRemoteStore = + listensToRemoteStore(listener) && + queryInfo.getRemoteListeners().length === 0; } } if (lastListen) { eventManagerImpl.queries.delete(query); - return eventManagerImpl.onUnlisten(query, lastListenToServer); - } else if (lastListenToServer) { - return eventManagerImpl.onRemoteStoreUnlisten(query); + // When removing the last listener, trigger remote store un-listen based + // on the source it is listening to. If it is cache, watch connection might + // have not been established previously or already been closed. + return eventManagerImpl.onUnlisten!(query, lastListenToRemoteStore); + } else if (lastListenToRemoteStore) { + // Un-listen to the remote store if there are no listeners sourced from watch left. + return eventManagerImpl.onRemoteStoreUnlisten!(query); } } diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index 38d335104db..8b5f1e0630a 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -317,8 +317,10 @@ export async function syncEngineListen( queryToTarget(query) ); - // Multi-tab - const status = shouldListenToRemote + // PORTING NOTE: When the query is listening to cache only, the target ID + // do not need to be registered as watch target with local Firestore client, + // to let primary tab listen to watch on behalf. + const status: QueryTargetState = shouldListenToRemote ? syncEngineImpl.sharedClientState.addLocalQueryTarget( targetData.targetId ) @@ -352,7 +354,8 @@ export async function triggerRemoteStoreListen( queryToTarget(query) ); - // Multi-tab + // PORTING NOTE: Register the target ID with local Firestore client as + // watch target. syncEngineImpl.sharedClientState.addLocalQueryTarget(targetData.targetId); if (syncEngineImpl.isPrimaryClient) { @@ -484,7 +487,8 @@ export async function triggerRemoteStoreUnlisten( ); const queries = syncEngineImpl.queriesByTarget.get(queryView.targetId)!; - // Multi-tab + // PORTING NOTE: Unregister the target ID with local Firestore client as + // watch target. syncEngineImpl.sharedClientState.removeLocalQueryTarget(queryView.targetId); if (queries.length === 1) { diff --git a/packages/firestore/src/local/shared_client_state.ts b/packages/firestore/src/local/shared_client_state.ts index 67a528957b4..0d0e430c12f 100644 --- a/packages/firestore/src/local/shared_client_state.ts +++ b/packages/firestore/src/local/shared_client_state.ts @@ -349,7 +349,7 @@ export class QueryTargetMetadata { /** * Metadata state of a single client denoting the query targets it is actively - * listening to. + * listening to the watch. */ // Visible for testing. export interface ClientState { diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index f4a62ba50ea..7f1d1a56590 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -55,7 +55,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { (persistence.gc === 'lru' ? describe : describe.skip)( 'listen to persistence cache', () => { - it('can listen to source==cache', () => { + it('can raise snapshot from cache and local mutations', () => { const testDocs = { a: { k: 'a', sort: 0 } }; @@ -114,7 +114,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('can attach multiple source==cache listeners', () => { + it('Multiple listeners sourced from cache can work independently', () => { const testDocs = { a: { k: 'a', sort: 0 } }; @@ -161,7 +161,8 @@ apiDescribe('Snapshot Listener source options ', persistence => { toDataArray(snapshots[1]) ); - // Detach one listener, and do a local mutation. The other listener should not be affected. + // Detach one listener, and do a local mutation. The other listener + // should not be affected. unsubscribe1(); await addDoc(coll, { k: 'c', sort: 2 }); @@ -184,7 +185,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { // Since limitToLast() queries are sent to the backend with a modified // orderBy() clause, they can map to the same target representation as // limit() query, even if both queries appear separate to the user. - it('can listen/unlisten/relisten to mirror queries', () => { + it('can listen/un-listen/re-listen to mirror queries', () => { const testDocs = { a: { k: 'a', sort: 0 }, b: { k: 'b', sort: 1 }, @@ -223,15 +224,13 @@ apiDescribe('Snapshot Listener source options ', persistence => { ]); expect(snapshot.metadata.fromCache).to.equal(true); - // Unlisten then relisten limit query. + // Un-listen then re-listen to the limit query. limitUnlisten(); limitUnlisten = onSnapshot( query(coll, orderBy('sort', 'asc'), limit(2)), { source: ListenSource.Cache }, storeLimitEvent.storeEvent ); - - // Verify `limit` query still works. snapshot = await storeLimitEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ { k: 'a', sort: 0 }, @@ -258,7 +257,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { ]); expect(snapshot.metadata.fromCache).to.equal(true); - // Unlisten to limitToLast, update a doc, then relisten limitToLast. + // Un-listen to limitToLast, update a doc, then re-listen limitToLast. limitToLastUnlisten(); await updateDoc(doc(coll, 'a'), { k: 'a', sort: -2 }); @@ -302,7 +301,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); let snapshot = await storeDefaultEvent.awaitRemoteEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); - // No cached result, only raise snapshot from server result expect(snapshot.metadata.fromCache).to.equal(false); // Listen to the same query from cache @@ -355,7 +353,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { // First snapshot will be raised from cache. expect(snapshot.metadata.fromCache).to.equal(true); snapshot = await storeDefaultEvent.awaitEvent(); - // Second snapshot raised from server result + // Second snapshot will be raised from server result expect(snapshot.metadata.fromCache).to.equal(false); await storeDefaultEvent.assertNoAdditionalEvents(); @@ -367,7 +365,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('can unlisten to default source while still listening to cache', () => { + it('can un-listen to default source while still listening to cache', () => { const testDocs = { a: { k: 'a', sort: 0 }, b: { k: 'b', sort: 1 } @@ -388,14 +386,13 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); await storeCacheEvent.awaitEvent(); - // Unlisten to the default listener. + // Un-listen to the default listener. defaultUnlisten(); await storeDefaultEvent.assertNoAdditionalEvents(); - // Add a document that would change the result set. + // Add a document and verify listener to cache works as expected await addDoc(coll, { k: 'c', sort: -1 }); - // Verify listener to cache works as expected const snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ { k: 'c', sort: -1 }, @@ -409,7 +406,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('can unlisten to cache while still listening to server', () => { + it('can un-listen to cache while still listening to server', () => { const testDocs = { a: { k: 'a', sort: 0 }, b: { k: 'b', sort: 1 } @@ -434,10 +431,9 @@ apiDescribe('Snapshot Listener source options ', persistence => { cacheUnlisten(); await storeCacheEvent.assertNoAdditionalEvents(); - // Add a document that would change the result set. + // Add a documentvand verify listener to server works as expected. await addDoc(coll, { k: 'c', sort: -1 }); - // Verify listener to server works as expected const snapshot = await storeDefaultEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ { k: 'c', sort: -1 }, @@ -526,7 +522,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('will not get metadata only updates if only listening to cache', () => { + it('will not get metadata only updates if listening to cache only', () => { const testDocs = { a: { k: 'a', sort: 0 } }; diff --git a/packages/firestore/test/unit/specs/listen_source_spec.test.ts b/packages/firestore/test/unit/specs/listen_source_spec.test.ts index c04d74a5f08..3d968a8d435 100644 --- a/packages/firestore/test/unit/specs/listen_source_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_source_spec.test.ts @@ -119,7 +119,7 @@ describeSpec('Listens source options:', [], () => { .expectEvents(query_, { added: [docA] }) .userUnlistens(query_) .watchRemoves(query_) - + // Listen to cache .userListensToCache(query_) .expectEvents(query_, { added: [docA], fromCache: true }) .userDeletes('collection/a') @@ -152,7 +152,7 @@ describeSpec('Listens source options:', [], () => { .expectEvents(query1, { added: [docAv1] }) .userUnlistens(query1) .watchRemoves(query1) - + // Listen to cache .userListensToCache(query1) .expectEvents(query1, { added: [docAv1], fromCache: true }) .userAddsSnapshotsInSyncListener() @@ -190,7 +190,6 @@ describeSpec('Listens source options:', [], () => { const query1 = query('collection'); return ( spec() - // Disable GC so the cache persists across listens. .ensureManualLruGC() // Populate the cache with the empty query results. .userListens(query1) @@ -213,7 +212,6 @@ describeSpec('Listens source options:', [], () => { const doc1 = doc('collection/a', 1000, { v: 1 }); return ( spec() - // Disable GC so the cache persists across listens. .ensureManualLruGC() // Populate the cache with the empty query results. .userListens(query1) @@ -253,7 +251,7 @@ describeSpec('Listens source options:', [], () => { .expectEvents(query1, { added: [docA] }) .userUnlistens(query1) .watchRemoves(query1) - + // Listen to cache .userListensToCache(query1) .expectEvents(query1, { added: [docA], fromCache: true }) .loadBundle(bundleString) @@ -281,7 +279,7 @@ describeSpec('Listens source options:', [], () => { .expectEvents(query1, { added: [docA] }) .userUnlistens(query1) .watchRemoves(query1) - + // Listen to cache .userListensToCache(query1) .expectEvents(query1, { added: [docA], fromCache: true }) .loadBundle(bundleString) @@ -307,7 +305,7 @@ describeSpec('Listens source options:', [], () => { .expectEvents(query1, { added: [docA] }) .userUnlistens(query1) .watchRemoves(query1) - + // Listen to cache .userListensToCache(query1) .expectEvents(query1, { added: [docA], fromCache: true }) // No events are expected here. @@ -338,7 +336,7 @@ describeSpec('Listens source options:', [], () => { .expectEvents(query1, { added: [docA] }) .userUnlistens(query1) .watchRemoves(query1) - + // Listen to cache .userListensToCache(query1) .expectEvents(query1, { added: [doc('collection/a', 250, { value: 'a' })], @@ -391,7 +389,7 @@ describeSpec('Listens source options:', [], () => { .expectEvents(query1, { added: [docA] }) .userUnlistens(query1) .watchRemoves(query1) - + // Listen to cache .userListensToCache(query1) .expectEvents(query1, { added: [doc('collection/a', 250, { value: 'a' })], @@ -423,27 +421,31 @@ describeSpec('Listens source options:', [], () => { ); specTest( - 'Primary client should not invoke listen to server while all clients are listening to cache', + 'Primary client should not invoke watch request while all clients are listening to cache', ['multi-client'], () => { const query1 = query('collection'); - return client(0) - .becomeVisible() - .client(1) - .userListensToCache(query1) - .expectEvents(query1, { added: [], fromCache: true }) - .client(0) - .expectListenToCache(query1) - .client(1) - .userUnlistensToCache(query1) - .client(0) - .expectUnlistenToCache(query1); + return ( + client(0) + .becomeVisible() + .client(1) + .userListensToCache(query1) + .expectEvents(query1, { added: [], fromCache: true }) + // Primary client should not invoke watch request for cache listeners + .client(0) + .expectListenToCache(query1) + .expectActiveTargets() + .client(1) + .userUnlistensToCache(query1) + .client(0) + .expectUnlistenToCache(query1) + ); } ); specTest( - 'Local mutations notifies listeners that source==cache in all tabs', + 'Local mutations notifies listeners sourced from cache in all tabs', ['multi-client'], () => { const query1 = query('collection'); @@ -470,16 +472,12 @@ describeSpec('Listens source options:', [], () => { hasPendingWrites: true, added: [docA], fromCache: true - }) - .userUnlistensToCache(query1) - .client(0) - .userUnlistensToCache(query1) - .expectUnlistenToCache(query1); + }); } ); specTest( - 'Query is shared between primary and secondary clients', + 'Listeners with different source shares watch changes between primary and secondary clients', ['multi-client'], () => { const query1 = query('collection'); @@ -493,13 +491,15 @@ describeSpec('Listens source options:', [], () => { .userListens(query1) .watchAcksFull(query1, 1000, docA) .expectEvents(query1, { added: [docA] }) + // Listen to cache in secondary clients .client(1) .userListensToCache(query1) .expectEvents(query1, { added: [docA], fromCache: true }) .client(2) .userListensToCache(query1) .expectEvents(query1, { added: [docA], fromCache: true }) - // Updates in the primary client notifies listeners sourcing from cache in secondary clients + // Updates in the primary client notifies listeners sourcing from cache + // in secondary clients. .client(0) .watchSends({ affects: [query1] }, docB) .watchSnapshots(2000) @@ -508,16 +508,17 @@ describeSpec('Listens source options:', [], () => { .expectEvents(query1, { added: [docB] }) .client(2) .expectEvents(query1, { added: [docB] }) + // Un-listen to the server in the primary tab. .client(0) .userUnlistens(query1) - // There should be no active watch targets left + // There should be no active watch targets left. .expectActiveTargets() ); } ); specTest( - 'Tabs can have multiple listeners with different sources', + 'Clients can have multiple listeners with different sources', ['multi-client'], () => { const query1 = query('collection'); @@ -527,13 +528,13 @@ describeSpec('Listens source options:', [], () => { return ( client(0) .becomeVisible() - // Listen to server in the primary client + // Listen to both server and cache in the primary client .userListens(query1) .watchAcksFull(query1, 1000, docA) .expectEvents(query1, { added: [docA] }) .userListensToCache(query1) .expectEvents(query1, { added: [docA] }) - // Listen to server and cache in the secondary client + // Listen to both server and cache in the secondary client .client(1) .userListens(query1) .expectEvents(query1, { added: [docA] }) @@ -552,38 +553,42 @@ describeSpec('Listens source options:', [], () => { } ); - specTest('Query is executed by primary client', ['multi-client'], () => { - const query1 = query('collection'); - const docA = doc('collection/a', 1000, { key: 'a' }); + specTest( + 'Query is executed by primary client even if it only includes listeners sourced from cache', + ['multi-client'], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { key: 'a' }); - return ( - client(0) - .becomeVisible() - // Listen to cache in primary client - .userListensToCache(query1) - .expectEvents(query1, { added: [], fromCache: true }) - // Listen to server in primary client - .client(1) - .userListens(query1) - // The query is executed in the primary client - .client(0) - .expectListen(query1) - // Updates in the primary client notifies both listeners - .watchAcks(query1) - .watchSends({ affects: [query1] }, docA) - .watchSnapshots(1000) - .expectEvents(query1, { added: [docA], fromCache: true }) - .client(1) - .expectEvents(query1, { added: [docA], fromCache: true }) - .client(0) - .watchCurrents(query1, 'resume-token-2000') - .watchSnapshots(2000) - // Listeners in both tabs are in sync - .expectEvents(query1, { fromCache: false }) - .client(1) - .expectEvents(query1, { fromCache: false }) - ); - }); + return ( + client(0) + .becomeVisible() + // Listen to cache in primary client + .userListensToCache(query1) + .expectEvents(query1, { added: [], fromCache: true }) + // Listen to server in secondary client + .client(1) + .userListens(query1) + // The query is executed in the primary client + .client(0) + .expectListen(query1) + // Updates in the primary client notifies both listeners + .watchAcks(query1) + .watchSends({ affects: [query1] }, docA) + .watchSnapshots(1000) + .expectEvents(query1, { added: [docA], fromCache: true }) + .client(1) + .expectEvents(query1, { added: [docA], fromCache: true }) + .client(0) + .watchCurrents(query1, 'resume-token-2000') + .watchSnapshots(2000) + // Listeners in both tabs are in sync + .expectEvents(query1, { fromCache: false }) + .client(1) + .expectEvents(query1, { fromCache: false }) + ); + } + ); specTest( 'Query only raises events in participating clients', @@ -611,58 +616,62 @@ describeSpec('Listens source options:', [], () => { } ); - specTest('Mirror queries in different clients', ['multi-client'], () => { - const fullQuery = query('collection'); - const limit = queryWithLimit( - query('collection', orderBy('sort', 'asc')), - 2, - LimitType.First - ); - const limitToLast = queryWithLimit( - query('collection', orderBy('sort', 'desc')), - 2, - LimitType.Last - ); - const docA = doc('collection/a', 1000, { sort: 0 }); - const docB = doc('collection/b', 1000, { sort: 1 }); - const docC = doc('collection/c', 1000, { sort: 1 }); - const docCV2 = doc('collection/c', 2000, { - sort: -1 - }).setHasLocalMutations(); + specTest( + 'Mirror queries being listened in different clients sourced from cache ', + ['multi-client'], + () => { + const fullQuery = query('collection'); + const limit = queryWithLimit( + query('collection', orderBy('sort', 'asc')), + 2, + LimitType.First + ); + const limitToLast = queryWithLimit( + query('collection', orderBy('sort', 'desc')), + 2, + LimitType.Last + ); + const docA = doc('collection/a', 1000, { sort: 0 }); + const docB = doc('collection/b', 1000, { sort: 1 }); + const docC = doc('collection/c', 1000, { sort: 1 }); + const docCV2 = doc('collection/c', 2000, { + sort: -1 + }).setHasLocalMutations(); - return ( - client(0) - .becomeVisible() - // Populate the cache first - .userListens(fullQuery) - .watchAcksFull(fullQuery, 1000, docA, docB, docC) - .expectEvents(fullQuery, { added: [docA, docB, docC] }) - .userUnlistens(fullQuery) - .watchRemoves(fullQuery) - - // Listen to mirror queries in 2 different tabs - .userListensToCache(limit) - .expectEvents(limit, { added: [docA, docB], fromCache: true }) - .client(1) - .userListensToCache(limitToLast) - .expectEvents(limitToLast, { added: [docB, docA], fromCache: true }) - // Un-listen to the query in primary tab and do a local mutation - .client(0) - .userUnlistensToCache(limit) - .userSets('collection/c', { sort: -1 }) - // Listener in the other tab should work as expected - .client(1) - .expectEvents(limitToLast, { - hasPendingWrites: true, - added: [docCV2], - removed: [docB], - fromCache: true - }) - ); - }); + return ( + client(0) + .becomeVisible() + // Populate the cache first + .userListens(fullQuery) + .watchAcksFull(fullQuery, 1000, docA, docB, docC) + .expectEvents(fullQuery, { added: [docA, docB, docC] }) + .userUnlistens(fullQuery) + .watchRemoves(fullQuery) + + // Listen to mirror queries from cache in 2 different tabs + .userListensToCache(limit) + .expectEvents(limit, { added: [docA, docB], fromCache: true }) + .client(1) + .userListensToCache(limitToLast) + .expectEvents(limitToLast, { added: [docB, docA], fromCache: true }) + // Un-listen to the query in primary tab and do a local mutation + .client(0) + .userUnlistensToCache(limit) + .userSets('collection/c', { sort: -1 }) + // Listener in the other tab should work as expected + .client(1) + .expectEvents(limitToLast, { + hasPendingWrites: true, + added: [docCV2], + removed: [docB], + fromCache: true + }) + ); + } + ); specTest( - 'Mirror queries from same secondary client', + 'Mirror queries being listened in the same secondary client sourced from cache', ['multi-client'], () => { const fullQuery = query('collection'); @@ -714,7 +723,7 @@ describeSpec('Listens source options:', [], () => { ); specTest( - 'Mirror queries from different sources while listening to server in primary tab', + 'Mirror queries being listened from different sources while listening to server in primary tab', ['multi-client'], () => { const limit = queryWithLimit( @@ -751,11 +760,6 @@ describeSpec('Listens source options:', [], () => { // Cache listener gets notified as well. .client(1) .expectEvents(limitToLast, { added: [docC], removed: [docB] }) - // Un-listen to both queries - .client(0) - .userUnlistens(limit) - .client(1) - .userUnlistensToCache(limitToLast) ); } ); @@ -803,4 +807,40 @@ describeSpec('Listens source options:', [], () => { ); } ); + + specTest( + 'Un-listen to listeners from different source', + ['multi-client'], + () => { + const query1 = query('collection'); + const docA = doc('collection/a', 1000, { key: 'a' }); + const docB = doc('collection/b', 1000, { + key: 'b' + }).setHasLocalMutations(); + + return ( + client(0) + .becomeVisible() + // Listen to server in primary client + .userListens(query1) + .watchAcksFull(query1, 1000, docA) + .expectEvents(query1, { added: [docA] }) + // Listen to cache in secondary client + .client(1) + .userListensToCache(query1) + .expectEvents(query1, { added: [docA], fromCache: true }) + .client(0) + .userUnlistens(query1) + .userSets('collection/b', { key: 'b' }) + // The other listener should work as expected + .client(1) + .expectEvents(query1, { + hasPendingWrites: true, + added: [docB], + fromCache: true + }) + .userUnlistensToCache(query1) + ); + } + ); }); From 83ab589d0d823290fbec3ada5a4f0bc81e61be46 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 1 Feb 2024 11:50:12 -0500 Subject: [PATCH 09/33] change getRemoteListeners to hasRemoteListeners --- packages/firestore/src/core/event_manager.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index 4e9acf050b0..bf3c70e5f60 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -34,9 +34,9 @@ class QueryListenersInfo { viewSnap: ViewSnapshot | undefined = undefined; listeners: QueryListener[] = []; - // Helper methods to filter listeners that listens to watch changes. - getRemoteListeners(): QueryListener[] { - return this.listeners.filter(l => l.options.source !== ListenSource.Cache); + // Helper methods that checks if the query has listeners that listening to remote store + hasRemoteListeners(): boolean { + return this.listeners.some(listener => listensToRemoteStore(listener)); } } @@ -135,9 +135,7 @@ export async function eventManagerListen( } const firstListenToRemoteStore = - listensToRemoteStore(listener) && - queryInfo.getRemoteListeners().length === 0; - + !queryInfo.hasRemoteListeners() && listensToRemoteStore(listener); if (firstListen) { // When listening to a query for the first time, it may or may not establish // watch connection based on the source the query is listening to. @@ -209,9 +207,9 @@ export async function eventManagerUnlisten( queryInfo.listeners.splice(i, 1); lastListen = queryInfo.listeners.length === 0; + // Check if the removed listener is the last one that sourced from watch. lastListenToRemoteStore = - listensToRemoteStore(listener) && - queryInfo.getRemoteListeners().length === 0; + !queryInfo.hasRemoteListeners() && listensToRemoteStore(listener); } } From bfee69ca69a06708563cfbed93be90c914686d5f Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:54:51 -0500 Subject: [PATCH 10/33] add a test for document reference --- .../api/snasphot_listener_source.test.ts | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index 7f1d1a56590..b25a99b362b 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -24,7 +24,9 @@ import { deleteDoc, disableNetwork, doc, + DocumentSnapshot, enableNetwork, + getDoc, getDocs, limit, limitToLast, @@ -44,7 +46,8 @@ import { apiDescribe, toDataArray, withTestCollection, - withTestDb + withTestDb, + withTestDocAndInitialData } from '../util/helpers'; import { bundleString, verifySuccessProgress } from './bundle.test'; @@ -55,7 +58,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { (persistence.gc === 'lru' ? describe : describe.skip)( 'listen to persistence cache', () => { - it('can raise snapshot from cache and local mutations', () => { + it('can raise snapshot from cache and local mutations for Query', () => { const testDocs = { a: { k: 'a', sort: 0 } }; @@ -88,6 +91,38 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); + it('can raise snapshot from cache and local mutations for DocumentReference', () => { + const testDocs = { k: 'a', sort: 0 }; + return withTestDocAndInitialData( + persistence, + testDocs, + async docRef => { + await getDoc(docRef); // Populate the cache. + + const storeEvent = new EventsAccumulator(); + const unsubscribe = onSnapshot( + docRef, + { source: ListenSource.Cache }, + storeEvent.storeEvent + ); + + let snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(snapshot.data()).to.deep.equal({ k: 'a', sort: 0 }); + + await setDoc(docRef, { k: 'a', sort: 1 }); + + snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(snapshot.data()).to.deep.equal({ k: 'a', sort: 1 }); + + await storeEvent.assertNoAdditionalEvents(); + unsubscribe(); + } + ); + }); + it('listen to cache would not be affected by online status change', () => { const testDocs = { a: { k: 'a', sort: 0 } From d8d3fc33cfd99ed6aa8e1cd421ef8beb7defd7c3 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 2 Feb 2024 17:06:19 -0500 Subject: [PATCH 11/33] remove excessive tests --- .../api/snasphot_listener_source.test.ts | 370 +++++++----------- 1 file changed, 144 insertions(+), 226 deletions(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index b25a99b362b..cedf7bb4121 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -21,7 +21,6 @@ import { EventsAccumulator } from '../util/events_accumulator'; import { addDoc, collection, - deleteDoc, disableNetwork, doc, DocumentSnapshot, @@ -51,14 +50,13 @@ import { } from '../util/helpers'; import { bundleString, verifySuccessProgress } from './bundle.test'; -import { verifyDocumentChange } from './query.test'; apiDescribe('Snapshot Listener source options ', persistence => { // eslint-disable-next-line no-restricted-properties (persistence.gc === 'lru' ? describe : describe.skip)( 'listen to persistence cache', () => { - it('can raise snapshot from cache and local mutations for Query', () => { + it('can raise snapshot from cache for Query', () => { const testDocs = { a: { k: 'a', sort: 0 } }; @@ -72,26 +70,16 @@ apiDescribe('Snapshot Listener source options ', persistence => { storeEvent.storeEvent ); - let snapshot = await storeEvent.awaitEvent(); + const snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.equal(true); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); - await addDoc(coll, { k: 'b', sort: 1 }); - - snapshot = await storeEvent.awaitEvent(); - expect(snapshot.metadata.fromCache).to.equal(true); - expect(snapshot.metadata.hasPendingWrites).to.equal(true); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); - await storeEvent.assertNoAdditionalEvents(); unsubscribe(); }); }); - it('can raise snapshot from cache and local mutations for DocumentReference', () => { + it('can raise snapshot from cache for DocumentReference', () => { const testDocs = { k: 'a', sort: 0 }; return withTestDocAndInitialData( persistence, @@ -106,17 +94,10 @@ apiDescribe('Snapshot Listener source options ', persistence => { storeEvent.storeEvent ); - let snapshot = await storeEvent.awaitEvent(); + const snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.equal(true); expect(snapshot.data()).to.deep.equal({ k: 'a', sort: 0 }); - await setDoc(docRef, { k: 'a', sort: 1 }); - - snapshot = await storeEvent.awaitEvent(); - expect(snapshot.metadata.fromCache).to.equal(true); - expect(snapshot.metadata.hasPendingWrites).to.equal(true); - expect(snapshot.data()).to.deep.equal({ k: 'a', sort: 1 }); - await storeEvent.assertNoAdditionalEvents(); unsubscribe(); } @@ -149,7 +130,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('Multiple listeners sourced from cache can work independently', () => { + it('multiple listeners sourced from cache can work independently', () => { const testDocs = { a: { k: 'a', sort: 0 } }; @@ -170,13 +151,10 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); let snapshots = await storeEvent.awaitEvents(2); - expect(snapshots[0].metadata.fromCache).to.equal(true); expect(toDataArray(snapshots[0])).to.deep.equal([ { k: 'a', sort: 0 } ]); - expect(snapshots[0].metadata.fromCache).to.equal( - snapshots[1].metadata.fromCache - ); + expect(snapshots[0].metadata).to.deep.equal(snapshots[1].metadata); expect(toDataArray(snapshots[0])).to.deep.equal( toDataArray(snapshots[1]) ); @@ -184,14 +162,11 @@ apiDescribe('Snapshot Listener source options ', persistence => { await addDoc(coll, { k: 'b', sort: 1 }); snapshots = await storeEvent.awaitEvents(2); - expect(snapshots[0].metadata.fromCache).to.equal(true); expect(toDataArray(snapshots[0])).to.deep.equal([ { k: 'a', sort: 0 }, { k: 'b', sort: 1 } ]); - expect(snapshots[0].metadata.fromCache).to.equal( - snapshots[1].metadata.fromCache - ); + expect(snapshots[0].metadata).to.deep.equal(snapshots[1].metadata); expect(toDataArray(snapshots[0])).to.deep.equal( toDataArray(snapshots[1]) ); @@ -220,7 +195,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { // Since limitToLast() queries are sent to the backend with a modified // orderBy() clause, they can map to the same target representation as // limit() query, even if both queries appear separate to the user. - it('can listen/un-listen/re-listen to mirror queries', () => { + it('can listen/un-listen/re-listen to mirror queries from cache', () => { const testDocs = { a: { k: 'a', sort: 0 }, b: { k: 'b', sort: 1 }, @@ -251,13 +226,11 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'a', sort: 0 }, { k: 'b', sort: 1 } ]); - expect(snapshot.metadata.fromCache).to.equal(true); snapshot = await storeLimitToLastEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ { k: 'b', sort: 1 }, { k: 'a', sort: 0 } ]); - expect(snapshot.metadata.fromCache).to.equal(true); // Un-listen then re-listen to the limit query. limitUnlisten(); @@ -283,14 +256,13 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'a', sort: 0 } ]); expect(snapshot.metadata.hasPendingWrites).to.equal(true); - expect(snapshot.metadata.fromCache).to.equal(true); snapshot = await storeLimitToLastEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ { k: 'a', sort: 0 }, { k: 'd', sort: -1 } ]); - expect(snapshot.metadata.fromCache).to.equal(true); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); // Un-listen to limitToLast, update a doc, then re-listen limitToLast. limitToLastUnlisten(); @@ -309,14 +281,14 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'd', sort: -1 } ]); expect(snapshot.metadata.hasPendingWrites).to.equal(true); - expect(snapshot.metadata.fromCache).to.equal(true); snapshot = await storeLimitToLastEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ { k: 'd', sort: -1 }, { k: 'a', sort: -2 } ]); - expect(snapshot.metadata.fromCache).to.equal(true); + // We listened to LimitToLast query after the doc update. + expect(snapshot.metadata.hasPendingWrites).to.equal(false); limitUnlisten(); limitToLastUnlisten(); @@ -347,12 +319,12 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + // The metadata is sync with server due to the default listener expect(snapshot.metadata.fromCache).to.equal(false); await storeDefaultEvent.assertNoAdditionalEvents(); await storeCacheEvent.assertNoAdditionalEvents(); - // Unlisten to the default listener first then cache. defaultUnlisten(); cacheUnlisten(); }); @@ -363,8 +335,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { a: { k: 'a', sort: 0 } }; return withTestCollection(persistence, testDocs, async coll => { - await getDocs(coll); // Populate the cache. - // Listen to the cache const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( @@ -373,10 +343,88 @@ apiDescribe('Snapshot Listener source options ', persistence => { storeCacheEvent.storeEvent ); let snapshot = await storeCacheEvent.awaitEvent(); + // Cache is empty + expect(toDataArray(snapshot)).to.deep.equal([]); + expect(snapshot.metadata.fromCache).to.equal(true); + + // Listen to the same query from server + const storeDefaultEvent = new EventsAccumulator(); + const defaultUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + storeDefaultEvent.storeEvent + ); + snapshot = await storeDefaultEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(snapshot.metadata.fromCache).to.equal(false); + + // Default listener updates the cache, whish triggers cache listener to raise snapshot. + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + // The metadata is sync with server due to the default listener + expect(snapshot.metadata.fromCache).to.equal(false); + + await storeDefaultEvent.assertNoAdditionalEvents(); + await storeCacheEvent.assertNoAdditionalEvents(); + + defaultUnlisten(); + cacheUnlisten(); + }); + }); + + it('will not get metadata only updates if listening to cache only', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + await getDocs(coll); // Populate the cache. + + const storeEvent = new EventsAccumulator(); + const unsubscribe = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { includeMetadataChanges: true, source: ListenSource.Cache }, + storeEvent.storeEvent + ); + + let snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + + await addDoc(coll, { k: 'b', sort: 1 }); + + snapshot = await storeEvent.awaitEvent(); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(snapshot.metadata.fromCache).to.equal(true); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + + // As we are not listening to server, the listener will not get notified + // when local mutation is acknowledged by server. + await storeEvent.assertNoAdditionalEvents(); + unsubscribe(); + }); + }); + + it('will have synced metadata updates when listening to both cache and default source', () => { + const testDocs = { + a: { k: 'a', sort: 0 } + }; + return withTestCollection(persistence, testDocs, async coll => { + await getDocs(coll); // Populate the cache. + + // Listen to the query from cache + const storeCacheEvent = new EventsAccumulator(); + const cacheUnlisten = onSnapshot( + query(coll, orderBy('sort', 'asc')), + { includeMetadataChanges: true, source: ListenSource.Cache }, + storeCacheEvent.storeEvent + ); + let snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); expect(snapshot.metadata.fromCache).to.equal(true); - // Listen to the same query with default options + // Listen to the same query from server const storeDefaultEvent = new EventsAccumulator(); const defaultUnlisten = onSnapshot( query(coll, orderBy('sort', 'asc')), @@ -391,10 +439,39 @@ apiDescribe('Snapshot Listener source options ', persistence => { // Second snapshot will be raised from server result expect(snapshot.metadata.fromCache).to.equal(false); - await storeDefaultEvent.assertNoAdditionalEvents(); - await storeCacheEvent.assertNoAdditionalEvents(); + // As listening to metadata changes, the cache listener also gets triggered and synced + // with default listener. + snapshot = await storeCacheEvent.awaitEvent(); + expect(snapshot.metadata.fromCache).to.equal(false); + + await addDoc(coll, { k: 'b', sort: 1 }); + + // snapshot gets triggered by local mutation + snapshot = await storeDefaultEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(snapshot.metadata.fromCache).to.equal(false); + + snapshot = await storeCacheEvent.awaitEvent(); + expect(toDataArray(snapshot)).to.deep.equal([ + { k: 'a', sort: 0 }, + { k: 'b', sort: 1 } + ]); + expect(snapshot.metadata.hasPendingWrites).to.equal(true); + expect(snapshot.metadata.fromCache).to.equal(false); + + // Local mutation gets acknowledged by the server + snapshot = await storeDefaultEvent.awaitEvent(); + expect(snapshot.metadata.hasPendingWrites).to.equal(false); + expect(snapshot.metadata.fromCache).to.equal(false); + + snapshot = await storeCacheEvent.awaitEvent(); + expect(snapshot.metadata.hasPendingWrites).to.equal(false); + expect(snapshot.metadata.fromCache).to.equal(false); - // Unlisten to the cache listener first then default. cacheUnlisten(); defaultUnlisten(); }); @@ -402,8 +479,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('can un-listen to default source while still listening to cache', () => { const testDocs = { - a: { k: 'a', sort: 0 }, - b: { k: 'b', sort: 1 } + a: { k: 'a', sort: 0 } }; return withTestCollection(persistence, testDocs, async coll => { // Listen to the query with both source options @@ -426,15 +502,13 @@ apiDescribe('Snapshot Listener source options ', persistence => { await storeDefaultEvent.assertNoAdditionalEvents(); // Add a document and verify listener to cache works as expected - await addDoc(coll, { k: 'c', sort: -1 }); + await addDoc(coll, { k: 'b', sort: -1 }); const snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'c', sort: -1 }, - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } + { k: 'b', sort: -1 }, + { k: 'a', sort: 0 } ]); - expect(snapshot.metadata.hasPendingWrites).to.equal(true); await storeCacheEvent.assertNoAdditionalEvents(); cacheUnlisten(); @@ -443,8 +517,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('can un-listen to cache while still listening to server', () => { const testDocs = { - a: { k: 'a', sort: 0 }, - b: { k: 'b', sort: 1 } + a: { k: 'a', sort: 0 } }; return withTestCollection(persistence, testDocs, async coll => { // Listen to the query with both source options @@ -462,21 +535,18 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); await storeCacheEvent.awaitEvent(); - // Unlisten to the cache. + // Un-listen to cache. cacheUnlisten(); await storeCacheEvent.assertNoAdditionalEvents(); - // Add a documentvand verify listener to server works as expected. - await addDoc(coll, { k: 'c', sort: -1 }); + // Add a document and verify listener to server works as expected. + await addDoc(coll, { k: 'b', sort: -1 }); const snapshot = await storeDefaultEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'c', sort: -1 }, - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } + { k: 'b', sort: -1 }, + { k: 'a', sort: 0 } ]); - expect(snapshot.metadata.fromCache).to.equal(false); - expect(snapshot.metadata.hasPendingWrites).to.equal(true); await storeDefaultEvent.assertNoAdditionalEvents(); defaultUnlisten(); @@ -496,7 +566,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); let snapshot = await storeDefaultEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); - expect(snapshot.metadata.fromCache).to.equal(false); // Listen to the same query from cache const storeCacheEvent = new EventsAccumulator(); @@ -507,10 +576,8 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); - // The metadata is sync with server due to the default listener - expect(snapshot.metadata.fromCache).to.equal(false); - // Un-listen to the default listener, add a doc and re-listen.. + // Un-listen to the default listener, add a doc and re-listen. defaultUnlisten(); await addDoc(coll, { k: 'b', sort: 1 }); @@ -530,7 +597,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { { k: 'b', sort: 1 } ]); - // Unlisten to cache, update a doc, then re-listen to cache. + // Un-listen to cache, update a doc, then re-listen to cache. cacheUnlisten(); await updateDoc(doc(coll, 'a'), { k: 'a', sort: 2 }); @@ -557,154 +624,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('will not get metadata only updates if listening to cache only', () => { - const testDocs = { - a: { k: 'a', sort: 0 } - }; - return withTestCollection(persistence, testDocs, async coll => { - await getDocs(coll); // Populate the cache. - - const storeEvent = new EventsAccumulator(); - const unsubscribe = onSnapshot( - query(coll, orderBy('sort', 'asc')), - { includeMetadataChanges: true, source: ListenSource.Cache }, - storeEvent.storeEvent - ); - - let snapshot = await storeEvent.awaitEvent(); - expect(snapshot.metadata.fromCache).to.equal(true); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); - - await addDoc(coll, { k: 'b', sort: 1 }); - - snapshot = await storeEvent.awaitEvent(); - expect(snapshot.metadata.hasPendingWrites).to.equal(true); - expect(snapshot.metadata.fromCache).to.equal(true); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); - - // As we are not listening to server, the listener will not get notified - // when local mutation is acknowledged by server. - await storeEvent.assertNoAdditionalEvents(); - unsubscribe(); - }); - }); - - it('will have synced metadata updates when listening to both cache and default source', () => { - const testDocs = { - a: { k: 'a', sort: 0 } - }; - return withTestCollection(persistence, testDocs, async coll => { - const storeDefaultEvent = new EventsAccumulator(); - const defaultUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), - { includeMetadataChanges: true }, - storeDefaultEvent.storeEvent - ); - let snapshot = await storeDefaultEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); - expect(snapshot.metadata.hasPendingWrites).to.equal(false); - expect(snapshot.metadata.fromCache).to.equal(false); - - // Listen to the same query from cache - const storeCacheEvent = new EventsAccumulator(); - const cacheUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), - { includeMetadataChanges: true, source: ListenSource.Cache }, - storeCacheEvent.storeEvent - ); - snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); - // metadata is synced with the default source listener - expect(snapshot.metadata.hasPendingWrites).to.equal(false); - expect(snapshot.metadata.fromCache).to.equal(false); - - await addDoc(coll, { k: 'b', sort: 1 }); - - // snapshot gets triggered by local mutation - snapshot = await storeDefaultEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); - expect(snapshot.metadata.hasPendingWrites).to.equal(true); - expect(snapshot.metadata.fromCache).to.equal(false); - - snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); - expect(snapshot.metadata.hasPendingWrites).to.equal(true); - expect(snapshot.metadata.fromCache).to.equal(false); - - // Local mutation gets acknowledged by the server - snapshot = await storeDefaultEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); - expect(snapshot.metadata.hasPendingWrites).to.equal(false); - expect(snapshot.metadata.fromCache).to.equal(false); - - snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); - expect(snapshot.metadata.hasPendingWrites).to.equal(false); - expect(snapshot.metadata.fromCache).to.equal(false); - - cacheUnlisten(); - defaultUnlisten(); - }); - }); - - it('maintains correct DocumentChange indexes', async () => { - const testDocs = { - 'a': { order: 1 }, - 'b': { order: 2 }, - 'c': { 'order': 3 } - }; - await withTestCollection(persistence, testDocs, async coll => { - await getDocs(coll); // Populate the cache. - - const accumulator = new EventsAccumulator(); - const unsubscribe = onSnapshot( - query(coll, orderBy('order')), - { source: ListenSource.Cache }, - accumulator.storeEvent - ); - await accumulator - .awaitEvent() - .then(querySnapshot => { - const changes = querySnapshot.docChanges(); - expect(changes.length).to.equal(3); - verifyDocumentChange(changes[0], 'a', -1, 0, 'added'); - verifyDocumentChange(changes[1], 'b', -1, 1, 'added'); - verifyDocumentChange(changes[2], 'c', -1, 2, 'added'); - }) - .then(() => setDoc(doc(coll, 'b'), { order: 4 })) - .then(() => accumulator.awaitEvent()) - .then(querySnapshot => { - const changes = querySnapshot.docChanges(); - expect(changes.length).to.equal(1); - verifyDocumentChange(changes[0], 'b', 1, 2, 'modified'); - }) - .then(() => deleteDoc(doc(coll, 'c'))) - .then(() => accumulator.awaitEvent()) - .then(querySnapshot => { - const changes = querySnapshot.docChanges(); - expect(changes.length).to.equal(1); - verifyDocumentChange(changes[0], 'c', 1, -1, 'removed'); - }); - - unsubscribe(); - }); - }); - it('can listen to composite index queries', () => { const testDocs = { a: { k: 'a', sort: 0 } @@ -725,7 +644,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { ); const snapshot = await storeEvent.awaitEvent(); - expect(snapshot.metadata.fromCache).to.equal(true); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); unsubscribe(); }); @@ -734,10 +652,8 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('can raise initial snapshot from cache, even if it is empty', () => { return withTestCollection(persistence, {}, async coll => { let snapshot = await getDocs(coll); // Populate the cache. - expect(snapshot.metadata.fromCache).to.be.false; expect(toDataArray(snapshot)).to.deep.equal([]); // Precondition check. - // Add a snapshot listener whose first event should be raised from cache. const storeEvent = new EventsAccumulator(); onSnapshot( coll, @@ -775,15 +691,17 @@ apiDescribe('Snapshot Listener source options ', persistence => { // cache can only be tested in spec tests. await accumulator.assertNoAdditionalEvents(); - let snap = await getDocs((await namedQuery(db, 'limit'))!); - expect(toDataArray(snap)).to.deep.equal([{ k: 'b', bar: 0 }]); + let snapshot = await getDocs((await namedQuery(db, 'limit'))!); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'b', bar: 0 }]); + expect(snapshot.metadata.fromCache).to.be.true; - snap = await getDocs((await namedQuery(db, 'limit-to-last'))!); - expect(toDataArray(snap)).to.deep.equal([{ k: 'a', bar: 0 }]); + snapshot = await getDocs((await namedQuery(db, 'limit-to-last'))!); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', bar: 0 }]); + expect(snapshot.metadata.fromCache).to.be.true; }); }); - it('Will not be triggered by transactions', () => { + it('will not be triggered by transactions', () => { return withTestCollection(persistence, {}, async (coll, db) => { const accumulator = new EventsAccumulator(); const unsubscribe = onSnapshot( @@ -799,7 +717,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const docRef = doc(coll); // Use a transaction to perform a write without triggering any local events. await runTransaction(db, async txn => { - txn.set(docRef, { k: 'a', sort: 0 }); + txn.set(docRef, { k: 'a' }); }); await accumulator.assertNoAdditionalEvents(); @@ -807,7 +725,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('triggered by server side updates when listening to both cache and default', () => { + it('gets triggered by server side updates when listening to both cache and default', () => { const testDocs = { a: { k: 'a', sort: 0 } }; From cfed74daf668976bc9a88bf1d4ff3f37c374f23d Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 2 Feb 2024 17:26:33 -0500 Subject: [PATCH 12/33] Update snasphot_listener_source.test.ts --- .../test/integration/api/snasphot_listener_source.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index cedf7bb4121..d53690a157c 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -693,11 +693,9 @@ apiDescribe('Snapshot Listener source options ', persistence => { let snapshot = await getDocs((await namedQuery(db, 'limit'))!); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'b', bar: 0 }]); - expect(snapshot.metadata.fromCache).to.be.true; snapshot = await getDocs((await namedQuery(db, 'limit-to-last'))!); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', bar: 0 }]); - expect(snapshot.metadata.fromCache).to.be.true; }); }); From 0a25598164e8420e329f50ec63af24a8d7f5f162 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 2 Feb 2024 17:51:21 -0500 Subject: [PATCH 13/33] rename test --- .../api/snasphot_listener_source.test.ts | 27 +++++-------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index d53690a157c..b02cc27cd93 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -703,13 +703,12 @@ apiDescribe('Snapshot Listener source options ', persistence => { return withTestCollection(persistence, {}, async (coll, db) => { const accumulator = new EventsAccumulator(); const unsubscribe = onSnapshot( - query(coll, orderBy('sort')), + coll, { source: ListenSource.Cache }, accumulator.storeEvent ); const snapshot = await accumulator.awaitEvent(); - expect(snapshot.metadata.fromCache).to.be.true; expect(toDataArray(snapshot)).to.deep.equal([]); const docRef = doc(coll); @@ -723,7 +722,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('gets triggered by server side updates when listening to both cache and default', () => { + it('shares server side updates when listening to both cache and default', () => { const testDocs = { a: { k: 'a', sort: 0 } }; @@ -747,12 +746,13 @@ apiDescribe('Snapshot Listener source options ', persistence => { snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); - let docRef = doc(coll); - // Use a transaction to perform a write without triggering any local events. + // Use a transaction to mock server side updates + const docRef = doc(coll); await runTransaction(db, async txn => { txn.set(docRef, { k: 'b', sort: 1 }); }); + // Default listener receives the server update snapshot = await storeDefaultEvent.awaitRemoteEvent(); expect(toDataArray(snapshot)).to.deep.equal([ { k: 'a', sort: 0 }, @@ -760,6 +760,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { ]); expect(snapshot.metadata.fromCache).to.be.false; + // Cache listener raises snapshot as well snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ { k: 'a', sort: 0 }, @@ -768,22 +769,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { expect(snapshot.metadata.fromCache).to.be.false; defaultUnlisten(); - docRef = doc(coll); - // Use a transaction to perform a write without triggering any local events. - await runTransaction(db, async txn => { - txn.set(docRef, { k: 'c', sort: 2 }); - }); - - // Add a document that would change the result set. - await addDoc(coll, { k: 'c', sort: -1 }); - - // Verify listener to cache works as expected - snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'c', sort: -1 }, - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); cacheUnlisten(); }); }); From 6dc7abda54dd374268a2bbcecde2017f8d688705 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 2 Feb 2024 18:05:06 -0500 Subject: [PATCH 14/33] rename tests with "cache" --- .../integration/api/snasphot_listener_source.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index b02cc27cd93..5756fcf0f5e 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -624,7 +624,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('can listen to composite index queries', () => { + it('can listen to composite index queries from cache', () => { const testDocs = { a: { k: 'a', sort: 0 } }; @@ -666,7 +666,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('load with documents already pulled from backend', () => { + it('load documents pulled from backend while listening to cache', () => { return withTestDb(persistence, async db => { await setDoc(doc(db, 'coll-1/a'), { k: 'a', bar: 0 }); await setDoc(doc(db, 'coll-1/b'), { k: 'b', bar: 0 }); @@ -699,7 +699,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('will not be triggered by transactions', () => { + it('will not be triggered by transactions while listening to cache', () => { return withTestCollection(persistence, {}, async (coll, db) => { const accumulator = new EventsAccumulator(); const unsubscribe = onSnapshot( @@ -717,12 +717,13 @@ apiDescribe('Snapshot Listener source options ', persistence => { txn.set(docRef, { k: 'a' }); }); + // There should be no events raised await accumulator.assertNoAdditionalEvents(); unsubscribe(); }); }); - it('shares server side updates when listening to both cache and default', () => { + it('share server side updates when listening to both cache and default', () => { const testDocs = { a: { k: 'a', sort: 0 } }; From edf30876c30bb5c762e98ee60c211c88fd28fd9d Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 5 Feb 2024 09:31:45 -0500 Subject: [PATCH 15/33] adjust the spec builder to not create unnecessary field --- packages/firestore/test/unit/specs/spec_builder.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index cca4c5bcc81..a03c2ba85a4 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -273,7 +273,7 @@ export class SpecBuilder { private addUserListenStep( query: Query, resume?: ResumeSpec, - options: ListenOptions = {} + options?: ListenOptions ): void { this.nextStep(); From f914d4b523bc214907e789856ca6d9a54c5ee8c8 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 5 Feb 2024 10:40:09 -0500 Subject: [PATCH 16/33] add changeset --- .changeset/eleven-spoons-occur.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/eleven-spoons-occur.md diff --git a/.changeset/eleven-spoons-occur.md b/.changeset/eleven-spoons-occur.md new file mode 100644 index 00000000000..097d574e8e7 --- /dev/null +++ b/.changeset/eleven-spoons-occur.md @@ -0,0 +1,5 @@ +--- +'@firebase/firestore': patch +'firebase': patch +--- +Enable snapshot listeners to source from local cache. From cf4de20a22734bb91e99fe73d4290fdd8a787369 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 6 Feb 2024 10:17:49 -0500 Subject: [PATCH 17/33] move source identifier function into QueryListener --- packages/firestore/src/core/event_manager.ts | 18 +++++++++--------- .../test/unit/core/event_manager.test.ts | 3 ++- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index bf3c70e5f60..10eaeaaabb3 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -36,7 +36,7 @@ class QueryListenersInfo { // Helper methods that checks if the query has listeners that listening to remote store hasRemoteListeners(): boolean { - return this.listeners.some(listener => listensToRemoteStore(listener)); + return this.listeners.some(listener => listener.listensToRemoteStore()); } } @@ -101,10 +101,6 @@ export class EventManagerImpl implements EventManager { onRemoteStoreUnlisten?: (query: Query) => Promise; } -function listensToRemoteStore(listener: QueryListener): boolean { - return listener.options.source !== ListenSource.Cache; -} - function validateEventManager(eventManagerImpl: EventManagerImpl): void { debugAssert(!!eventManagerImpl.onListen, 'onListen not set'); debugAssert( @@ -135,7 +131,7 @@ export async function eventManagerListen( } const firstListenToRemoteStore = - !queryInfo.hasRemoteListeners() && listensToRemoteStore(listener); + !queryInfo.hasRemoteListeners() && listener.listensToRemoteStore(); if (firstListen) { // When listening to a query for the first time, it may or may not establish // watch connection based on the source the query is listening to. @@ -209,7 +205,7 @@ export async function eventManagerUnlisten( lastListen = queryInfo.listeners.length === 0; // Check if the removed listener is the last one that sourced from watch. lastListenToRemoteStore = - !queryInfo.hasRemoteListeners() && listensToRemoteStore(listener); + !queryInfo.hasRemoteListeners() && listener.listensToRemoteStore(); } } @@ -343,7 +339,7 @@ export class QueryListener { */ private raisedInitialEvent = false; - readonly options: ListenOptions; + private options: ListenOptions; private snap: ViewSnapshot | null = null; @@ -438,7 +434,7 @@ export class QueryListener { } // Always raise event if listening to cache - if (this.options.source === ListenSource.Cache) { + if (!this.listensToRemoteStore()) { return true; } @@ -500,4 +496,8 @@ export class QueryListener { this.raisedInitialEvent = true; this.queryObserver.next(snap); } + + listensToRemoteStore(): boolean { + return this.options.source !== ListenSource.Cache; + } } diff --git a/packages/firestore/test/unit/core/event_manager.test.ts b/packages/firestore/test/unit/core/event_manager.test.ts index 24371b90dc0..d3318c23d4a 100644 --- a/packages/firestore/test/unit/core/event_manager.test.ts +++ b/packages/firestore/test/unit/core/event_manager.test.ts @@ -54,7 +54,8 @@ describe('EventManager', () => { options: {}, onViewSnapshot: () => {}, onError: () => {}, - applyOnlineStateChange: () => {} + applyOnlineStateChange: () => {}, + listensToRemoteStore: () => {} }; } From 5164b313239ab9508c461d3c58d9e2c38b2c0ea4 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 6 Feb 2024 16:23:22 -0500 Subject: [PATCH 18/33] resolve comments --- ...n-spoons-occur.md => smart-games-cheer.md} | 4 +- packages/firestore/src/core/event_manager.ts | 46 +++++++++++-------- .../firestore/src/core/firestore_client.ts | 4 +- .../firestore/src/core/sync_engine_impl.ts | 24 +++++----- .../test/unit/core/event_manager.test.ts | 14 +++--- .../test/unit/specs/spec_test_runner.ts | 8 ++-- 6 files changed, 53 insertions(+), 47 deletions(-) rename .changeset/{eleven-spoons-occur.md => smart-games-cheer.md} (56%) diff --git a/.changeset/eleven-spoons-occur.md b/.changeset/smart-games-cheer.md similarity index 56% rename from .changeset/eleven-spoons-occur.md rename to .changeset/smart-games-cheer.md index 097d574e8e7..d99d378089d 100644 --- a/.changeset/eleven-spoons-occur.md +++ b/.changeset/smart-games-cheer.md @@ -1,5 +1,5 @@ --- -'@firebase/firestore': patch -'firebase': patch +'@firebase/firestore': minor +'firebase': minor --- Enable snapshot listeners to source from local cache. diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index 10eaeaaabb3..b07d6e40ecc 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -63,8 +63,8 @@ export interface EventManager { enableRemoteListen: boolean ) => Promise; onUnlisten?: (query: Query, disableRemoteListen: boolean) => Promise; - onRemoteStoreListen?: (query: Query) => Promise; - onRemoteStoreUnlisten?: (query: Query) => Promise; + onFirstRemoteStoreListen?: (query: Query) => Promise; + onLastRemoteStoreUnlisten?: (query: Query) => Promise; } export function newEventManager(): EventManager { @@ -93,24 +93,24 @@ export class EventManagerImpl implements EventManager { * Callback invoked when a Query starts listening to the remote store, while * already listening to the cache. */ - onRemoteStoreListen?: (query: Query) => Promise; + onFirstRemoteStoreListen?: (query: Query) => Promise; /** * Callback invoked when a Query stops listening to the remote store, while * still listening to the cache. */ - onRemoteStoreUnlisten?: (query: Query) => Promise; + onLastRemoteStoreUnlisten?: (query: Query) => Promise; } function validateEventManager(eventManagerImpl: EventManagerImpl): void { debugAssert(!!eventManagerImpl.onListen, 'onListen not set'); debugAssert( - !!eventManagerImpl.onRemoteStoreListen, - 'onRemoteStoreListen not set' + !!eventManagerImpl.onFirstRemoteStoreListen, + 'onFirstRemoteStoreListen not set' ); debugAssert(!!eventManagerImpl.onUnlisten, 'onUnlisten not set'); debugAssert( - !!eventManagerImpl.onRemoteStoreUnlisten, - 'onRemoteStoreUnlisten not set' + !!eventManagerImpl.onLastRemoteStoreUnlisten, + 'onLastRemoteStoreUnlisten not set' ); } @@ -129,12 +129,18 @@ export async function eventManagerListen( firstListen = true; queryInfo = new QueryListenersInfo(); } - const firstListenToRemoteStore = !queryInfo.hasRemoteListeners() && listener.listensToRemoteStore(); + + /** + * Different scenarios arise based on whether it's the initial listener and its listen source: + * a. First listener with source 'default': Register the query in the local store and establish a watch connection. + * b. First listener with source 'cache': Register the query in the local store. + * c. Previously listened to 'cache', first listener source from 'default': Establish a watch connection. + * d. Already listened to the 'default' source: No action required. + */ + if (firstListen) { - // When listening to a query for the first time, it may or may not establish - // watch connection based on the source the query is listening to. try { queryInfo.viewSnap = await eventManagerImpl.onListen!( query, @@ -149,10 +155,8 @@ export async function eventManagerListen( return; } } else if (firstListenToRemoteStore) { - // A query might have listened to previously, but no watch connection is created - // as it has been listening to cache only. try { - await eventManagerImpl.onRemoteStoreListen!(query); + await eventManagerImpl.onFirstRemoteStoreListen!(query); } catch (e) { const firestoreError = wrapInUserErrorIfRecoverable( e as Error, @@ -208,16 +212,18 @@ export async function eventManagerUnlisten( !queryInfo.hasRemoteListeners() && listener.listensToRemoteStore(); } } - + /** + * Different scenarios arise based on whether it's the last listener and its listen source: + * a. Last listener and source is 'default': Remove query from local store and remove the watch connection. + * b. Last listener and source is 'cache': Remove query from local store. + * c. Last listener source from 'default', but still have listeners listening to "cache": Remove the watch connection. + * d. Neither the last listener, nor the last listener source from 'default': No action required. + */ if (lastListen) { eventManagerImpl.queries.delete(query); - // When removing the last listener, trigger remote store un-listen based - // on the source it is listening to. If it is cache, watch connection might - // have not been established previously or already been closed. return eventManagerImpl.onUnlisten!(query, lastListenToRemoteStore); } else if (lastListenToRemoteStore) { - // Un-listen to the remote store if there are no listeners sourced from watch left. - return eventManagerImpl.onRemoteStoreUnlisten!(query); + return eventManagerImpl.onLastRemoteStoreUnlisten!(query); } } diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index aea7c4b4606..6e21737380b 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -399,11 +399,11 @@ export async function getEventManager( null, onlineComponentProvider.syncEngine ); - eventManager.onRemoteStoreListen = triggerRemoteStoreListen.bind( + eventManager.onFirstRemoteStoreListen = triggerRemoteStoreListen.bind( null, onlineComponentProvider.syncEngine ); - eventManager.onRemoteStoreUnlisten = triggerRemoteStoreUnlisten.bind( + eventManager.onLastRemoteStoreUnlisten = triggerRemoteStoreUnlisten.bind( null, onlineComponentProvider.syncEngine ); diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index 8b5f1e0630a..410ea9892d0 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -317,9 +317,9 @@ export async function syncEngineListen( queryToTarget(query) ); - // PORTING NOTE: When the query is listening to cache only, the target ID - // do not need to be registered as watch target with local Firestore client, - // to let primary tab listen to watch on behalf. + // PORTING NOTE: When the query is listening to cache only, we skip sending it over to Watch by + // not registering it in shared client state, and directly calculate initial snapshots and + // subsequent updates from cache. const status: QueryTargetState = shouldListenToRemote ? syncEngineImpl.sharedClientState.addLocalQueryTarget( targetData.targetId @@ -354,7 +354,7 @@ export async function triggerRemoteStoreListen( queryToTarget(query) ); - // PORTING NOTE: Register the target ID with local Firestore client as + // PORTING NOTE: Register the target ID with local Firestore client as active // watch target. syncEngineImpl.sharedClientState.addLocalQueryTarget(targetData.targetId); @@ -487,11 +487,11 @@ export async function triggerRemoteStoreUnlisten( ); const queries = syncEngineImpl.queriesByTarget.get(queryView.targetId)!; - // PORTING NOTE: Unregister the target ID with local Firestore client as - // watch target. - syncEngineImpl.sharedClientState.removeLocalQueryTarget(queryView.targetId); + if (syncEngineImpl.isPrimaryClient && queries.length === 1) { + // PORTING NOTE: Unregister the target ID with local Firestore client as + // watch target. + syncEngineImpl.sharedClientState.removeLocalQueryTarget(queryView.targetId); - if (queries.length === 1) { remoteStoreUnlisten(syncEngineImpl.remoteStore, queryView.targetId); } } @@ -1555,12 +1555,12 @@ export async function syncEngineApplyActiveTargetsChange( } for (const targetId of added) { - // A target might have been added in a previous attempt, or could have listened to cache, - // but no remote store connection has been established by the primary client. - const targetHasBeenListenedTo = + // A target is already listening to remote store if it is already registered to + // sharedClientState. + const targetAlreadyListeningToRemoteStore = syncEngineImpl.queriesByTarget.has(targetId) && syncEngineImpl.sharedClientState.isActiveQueryTarget(targetId); - if (targetHasBeenListenedTo) { + if (targetAlreadyListeningToRemoteStore) { logDebug(LOG_TAG, 'Adding an already active target ' + targetId); continue; } diff --git a/packages/firestore/test/unit/core/event_manager.test.ts b/packages/firestore/test/unit/core/event_manager.test.ts index d3318c23d4a..e6752437ac5 100644 --- a/packages/firestore/test/unit/core/event_manager.test.ts +++ b/packages/firestore/test/unit/core/event_manager.test.ts @@ -63,22 +63,24 @@ describe('EventManager', () => { /* eslint-disable @typescript-eslint/no-explicit-any */ let onListenSpy: any, onUnlistenSpy: any, - onRemoteStoreListenSpy: any, - onRemoteStoreUnlistenSpy: any; + onFirstRemoteStoreListenSpy: any, + onLastRemoteStoreUnlistenSpy: any; /* eslint-enable @typescript-eslint/no-explicit-any */ beforeEach(() => { onListenSpy = sinon.stub().returns(Promise.resolve(0)); onUnlistenSpy = sinon.spy(); - onRemoteStoreListenSpy = sinon.spy(); - onRemoteStoreUnlistenSpy = sinon.spy(); + onFirstRemoteStoreListenSpy = sinon.spy(); + onLastRemoteStoreUnlistenSpy = sinon.spy(); }); function eventManagerBindSpy(eventManager: EventManager): void { eventManager.onListen = onListenSpy.bind(null); eventManager.onUnlisten = onUnlistenSpy.bind(null); - eventManager.onRemoteStoreListen = onRemoteStoreListenSpy.bind(null); - eventManager.onRemoteStoreUnlisten = onRemoteStoreUnlistenSpy.bind(null); + eventManager.onFirstRemoteStoreListen = + onFirstRemoteStoreListenSpy.bind(null); + eventManager.onLastRemoteStoreUnlisten = + onLastRemoteStoreUnlistenSpy.bind(null); } it('handles many listenables per query', async () => { diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 72dea3b01ff..9a922dc92eb 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -357,14 +357,12 @@ abstract class TestRunner { this.syncEngine ); - this.eventManager.onRemoteStoreListen = triggerRemoteStoreListen.bind( - null, - this.syncEngine - ); - this.eventManager.onRemoteStoreUnlisten = triggerRemoteStoreUnlisten.bind( + this.eventManager.onFirstRemoteStoreListen = triggerRemoteStoreListen.bind( null, this.syncEngine ); + this.eventManager.onLastRemoteStoreUnlisten = + triggerRemoteStoreUnlisten.bind(null, this.syncEngine); await this.persistence.setDatabaseDeletedListener(async () => { await this.shutdown(); From edd734eb3ca166f0d22aa2c44170565f6e124acc Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 7 Feb 2024 10:25:10 -0500 Subject: [PATCH 19/33] remove unnecessary condition check --- packages/firestore/src/core/sync_engine_impl.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index 410ea9892d0..dbdd341b446 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -1598,11 +1598,7 @@ export async function syncEngineApplyActiveTargetsChange( /* keepPersistedTargetData */ false ) .then(() => { - // A target could have listened to cache only and no remote store connection has been - // established by the primary client. - if (syncEngineImpl.sharedClientState.isActiveQueryTarget(targetId)) { - remoteStoreUnlisten(syncEngineImpl.remoteStore, targetId); - } + remoteStoreUnlisten(syncEngineImpl.remoteStore, targetId); removeAndCleanupTarget(syncEngineImpl, targetId); }) .catch(ignoreIfPrimaryLeaseLoss); From 3383fce09c95ca0922428a422ffd74a2f13a4ce3 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 7 Feb 2024 12:18:15 -0500 Subject: [PATCH 20/33] use enum in event-manager --- packages/firestore/src/core/event_manager.ts | 121 ++++++++++--------- 1 file changed, 65 insertions(+), 56 deletions(-) diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index b07d6e40ecc..7809b2fca3d 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -114,6 +114,20 @@ function validateEventManager(eventManagerImpl: EventManagerImpl): void { ); } +const enum ListenStatus { + FirstListenAndRequireWatchConnection, + FirstListenAndNotRequireWatchConnection, + NotFirstListenButRequireWatchConnection, + NoActionRequired +} + +const enum UnlistenStatus { + LastListenAndRequireWatchDisconnection, + LastListenAndNotRequireWatchDisconnection, + NotLastListenButRequireWatchDisconnection, + NoActionRequired +} + export async function eventManagerListen( eventManager: EventManager, listener: QueryListener @@ -121,52 +135,41 @@ export async function eventManagerListen( const eventManagerImpl = debugCast(eventManager, EventManagerImpl); validateEventManager(eventManagerImpl); + let listenerStatus = ListenStatus.NoActionRequired; + const query = listener.query; - let firstListen = false; let queryInfo = eventManagerImpl.queries.get(query); if (!queryInfo) { - firstListen = true; queryInfo = new QueryListenersInfo(); + listenerStatus = listener.listensToRemoteStore() + ? ListenStatus.FirstListenAndRequireWatchConnection + : ListenStatus.FirstListenAndNotRequireWatchConnection; + } else if (listener.listensToRemoteStore()) { + listenerStatus = ListenStatus.NotFirstListenButRequireWatchConnection; } - const firstListenToRemoteStore = - !queryInfo.hasRemoteListeners() && listener.listensToRemoteStore(); - /** - * Different scenarios arise based on whether it's the initial listener and its listen source: - * a. First listener with source 'default': Register the query in the local store and establish a watch connection. - * b. First listener with source 'cache': Register the query in the local store. - * c. Previously listened to 'cache', first listener source from 'default': Establish a watch connection. - * d. Already listened to the 'default' source: No action required. - */ - - if (firstListen) { - try { - queryInfo.viewSnap = await eventManagerImpl.onListen!( - query, - firstListenToRemoteStore - ); - } catch (e) { - const firestoreError = wrapInUserErrorIfRecoverable( - e as Error, - `Initialization of query '${stringifyQuery(listener.query)}' failed` - ); - listener.onError(firestoreError); - return; - } - } else if (firstListenToRemoteStore) { - try { - await eventManagerImpl.onFirstRemoteStoreListen!(query); - } catch (e) { - const firestoreError = wrapInUserErrorIfRecoverable( - e as Error, - `Initialization of remote connection for query '${stringifyQuery( - listener.query - )}' failed` - ); - listener.onError(firestoreError); - return; + try { + switch (listenerStatus) { + case ListenStatus.FirstListenAndRequireWatchConnection: + queryInfo.viewSnap = await eventManagerImpl.onListen!(query, true); + break; + case ListenStatus.FirstListenAndNotRequireWatchConnection: + queryInfo.viewSnap = await eventManagerImpl.onListen!(query, false); + break; + case ListenStatus.NotFirstListenButRequireWatchConnection: + await eventManagerImpl.onFirstRemoteStoreListen!(query); + break; + default: + break; } + } catch (e) { + const firestoreError = wrapInUserErrorIfRecoverable( + e as Error, + `Initialization of query '${stringifyQuery(listener.query)}' failed` + ); + listener.onError(firestoreError); + return; } eventManagerImpl.queries.set(query, queryInfo); @@ -197,8 +200,7 @@ export async function eventManagerUnlisten( validateEventManager(eventManagerImpl); const query = listener.query; - let lastListen = false; - let lastListenToRemoteStore = false; + let unlistenStatus = UnlistenStatus.NoActionRequired; const queryInfo = eventManagerImpl.queries.get(query); if (queryInfo) { @@ -206,24 +208,31 @@ export async function eventManagerUnlisten( if (i >= 0) { queryInfo.listeners.splice(i, 1); - lastListen = queryInfo.listeners.length === 0; - // Check if the removed listener is the last one that sourced from watch. - lastListenToRemoteStore = - !queryInfo.hasRemoteListeners() && listener.listensToRemoteStore(); + if (queryInfo.listeners.length === 0) { + unlistenStatus = listener.listensToRemoteStore() + ? UnlistenStatus.LastListenAndRequireWatchDisconnection + : UnlistenStatus.LastListenAndNotRequireWatchDisconnection; + } else if ( + !queryInfo.hasRemoteListeners() && + listener.listensToRemoteStore() + ) { + // The removed listener is the last one that sourced from watch. + unlistenStatus = + UnlistenStatus.NotLastListenButRequireWatchDisconnection; + } } } - /** - * Different scenarios arise based on whether it's the last listener and its listen source: - * a. Last listener and source is 'default': Remove query from local store and remove the watch connection. - * b. Last listener and source is 'cache': Remove query from local store. - * c. Last listener source from 'default', but still have listeners listening to "cache": Remove the watch connection. - * d. Neither the last listener, nor the last listener source from 'default': No action required. - */ - if (lastListen) { - eventManagerImpl.queries.delete(query); - return eventManagerImpl.onUnlisten!(query, lastListenToRemoteStore); - } else if (lastListenToRemoteStore) { - return eventManagerImpl.onLastRemoteStoreUnlisten!(query); + switch (unlistenStatus) { + case UnlistenStatus.LastListenAndRequireWatchDisconnection: + eventManagerImpl.queries.delete(query); + return eventManagerImpl.onUnlisten!(query, true); + case UnlistenStatus.LastListenAndNotRequireWatchDisconnection: + eventManagerImpl.queries.delete(query); + return eventManagerImpl.onUnlisten!(query, false); + case UnlistenStatus.NotLastListenButRequireWatchDisconnection: + return eventManagerImpl.onLastRemoteStoreUnlisten!(query); + default: + return; } } From fe46d8b8ada9ab43ed4db39f3a7b21cc06f46519 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 8 Feb 2024 10:33:31 -0500 Subject: [PATCH 21/33] extract duplicated code, rename enums --- packages/firestore/src/core/event_manager.ts | 74 +++++++++++-------- .../firestore/src/core/sync_engine_impl.ts | 65 ++++++++-------- 2 files changed, 78 insertions(+), 61 deletions(-) diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index 7809b2fca3d..b58029e8288 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -114,17 +114,17 @@ function validateEventManager(eventManagerImpl: EventManagerImpl): void { ); } -const enum ListenStatus { - FirstListenAndRequireWatchConnection, - FirstListenAndNotRequireWatchConnection, - NotFirstListenButRequireWatchConnection, +const enum ListenerSetupAction { + InitializeLocalListenAndRequireWatchConnection, + InitializeLocalListenOnly, + RequireWatchConnectionOnly, NoActionRequired } -const enum UnlistenStatus { - LastListenAndRequireWatchDisconnection, - LastListenAndNotRequireWatchDisconnection, - NotLastListenButRequireWatchDisconnection, +const enum ListenerRemovalAction { + TerminateLocalListenAndRequireWatchDisconnection, + TerminateLocalListenOnly, + RequireWatchDisconnectionOnly, NoActionRequired } @@ -135,29 +135,36 @@ export async function eventManagerListen( const eventManagerImpl = debugCast(eventManager, EventManagerImpl); validateEventManager(eventManagerImpl); - let listenerStatus = ListenStatus.NoActionRequired; + let listenerAction = ListenerSetupAction.NoActionRequired; const query = listener.query; let queryInfo = eventManagerImpl.queries.get(query); if (!queryInfo) { queryInfo = new QueryListenersInfo(); - listenerStatus = listener.listensToRemoteStore() - ? ListenStatus.FirstListenAndRequireWatchConnection - : ListenStatus.FirstListenAndNotRequireWatchConnection; + listenerAction = listener.listensToRemoteStore() + ? ListenerSetupAction.InitializeLocalListenAndRequireWatchConnection + : ListenerSetupAction.InitializeLocalListenOnly; } else if (listener.listensToRemoteStore()) { - listenerStatus = ListenStatus.NotFirstListenButRequireWatchConnection; + // Query has been listening to local cache, and tries to add a new listener sourced from watch. + listenerAction = ListenerSetupAction.RequireWatchConnectionOnly; } try { - switch (listenerStatus) { - case ListenStatus.FirstListenAndRequireWatchConnection: - queryInfo.viewSnap = await eventManagerImpl.onListen!(query, true); + switch (listenerAction) { + case ListenerSetupAction.InitializeLocalListenAndRequireWatchConnection: + queryInfo.viewSnap = await eventManagerImpl.onListen!( + query, + /** enableRemoteListen= */ true + ); break; - case ListenStatus.FirstListenAndNotRequireWatchConnection: - queryInfo.viewSnap = await eventManagerImpl.onListen!(query, false); + case ListenerSetupAction.InitializeLocalListenOnly: + queryInfo.viewSnap = await eventManagerImpl.onListen!( + query, + /** enableRemoteListen= */ false + ); break; - case ListenStatus.NotFirstListenButRequireWatchConnection: + case ListenerSetupAction.RequireWatchConnectionOnly: await eventManagerImpl.onFirstRemoteStoreListen!(query); break; default: @@ -200,7 +207,7 @@ export async function eventManagerUnlisten( validateEventManager(eventManagerImpl); const query = listener.query; - let unlistenStatus = UnlistenStatus.NoActionRequired; + let listenerAction = ListenerRemovalAction.NoActionRequired; const queryInfo = eventManagerImpl.queries.get(query); if (queryInfo) { @@ -209,27 +216,32 @@ export async function eventManagerUnlisten( queryInfo.listeners.splice(i, 1); if (queryInfo.listeners.length === 0) { - unlistenStatus = listener.listensToRemoteStore() - ? UnlistenStatus.LastListenAndRequireWatchDisconnection - : UnlistenStatus.LastListenAndNotRequireWatchDisconnection; + listenerAction = listener.listensToRemoteStore() + ? ListenerRemovalAction.TerminateLocalListenAndRequireWatchDisconnection + : ListenerRemovalAction.TerminateLocalListenOnly; } else if ( !queryInfo.hasRemoteListeners() && listener.listensToRemoteStore() ) { // The removed listener is the last one that sourced from watch. - unlistenStatus = - UnlistenStatus.NotLastListenButRequireWatchDisconnection; + listenerAction = ListenerRemovalAction.RequireWatchDisconnectionOnly; } } } - switch (unlistenStatus) { - case UnlistenStatus.LastListenAndRequireWatchDisconnection: + switch (listenerAction) { + case ListenerRemovalAction.TerminateLocalListenAndRequireWatchDisconnection: eventManagerImpl.queries.delete(query); - return eventManagerImpl.onUnlisten!(query, true); - case UnlistenStatus.LastListenAndNotRequireWatchDisconnection: + return eventManagerImpl.onUnlisten!( + query, + /** disableRemoteListen= */ true + ); + case ListenerRemovalAction.TerminateLocalListenOnly: eventManagerImpl.queries.delete(query); - return eventManagerImpl.onUnlisten!(query, false); - case UnlistenStatus.NotLastListenButRequireWatchDisconnection: + return eventManagerImpl.onUnlisten!( + query, + /** disableRemoteListen= */ false + ); + case ListenerRemovalAction.RequireWatchDisconnectionOnly: return eventManagerImpl.onLastRemoteStoreUnlisten!(query); default: return; diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index dbdd341b446..9daec1016b1 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -297,7 +297,6 @@ export async function syncEngineListen( ): Promise { const syncEngineImpl = ensureWatchCallbacks(syncEngine); - let targetId; let viewSnapshot; const queryView = syncEngineImpl.queryViewsByQuery.get(query); @@ -308,59 +307,64 @@ export async function syncEngineListen( // behalf of another tab and the user of the primary also starts listening // to the query. EventManager will not have an assigned target ID in this // case and calls `listen` to obtain this ID. - targetId = queryView.targetId; - syncEngineImpl.sharedClientState.addLocalQueryTarget(targetId); + syncEngineImpl.sharedClientState.addLocalQueryTarget(queryView.targetId); viewSnapshot = queryView.view.computeInitialSnapshot(); } else { - const targetData = await localStoreAllocateTarget( - syncEngineImpl.localStore, - queryToTarget(query) - ); - - // PORTING NOTE: When the query is listening to cache only, we skip sending it over to Watch by - // not registering it in shared client state, and directly calculate initial snapshots and - // subsequent updates from cache. - const status: QueryTargetState = shouldListenToRemote - ? syncEngineImpl.sharedClientState.addLocalQueryTarget( - targetData.targetId - ) - : 'not-current'; - - targetId = targetData.targetId; - viewSnapshot = await initializeViewAndComputeSnapshot( + viewSnapshot = await allocateTargetAndMaybeListen( syncEngineImpl, query, - targetId, - status === 'current', - targetData.resumeToken + shouldListenToRemote, + true ); - - if (syncEngineImpl.isPrimaryClient && shouldListenToRemote) { - remoteStoreListen(syncEngineImpl.remoteStore, targetData); - } + debugAssert(!!viewSnapshot, 'viewSnapshot is not initialized'); } return viewSnapshot; } +/** Query has been listening to the cache, and tries to initiate the remote store listen */ export async function triggerRemoteStoreListen( syncEngine: SyncEngine, query: Query ): Promise { const syncEngineImpl = ensureWatchCallbacks(syncEngine); + await allocateTargetAndMaybeListen(syncEngineImpl, query, true, false); +} +async function allocateTargetAndMaybeListen( + syncEngineImpl: SyncEngineImpl, + query: Query, + shouldListenToRemote: boolean, + shouldInitializeView: boolean +): Promise { const targetData = await localStoreAllocateTarget( syncEngineImpl.localStore, queryToTarget(query) ); - // PORTING NOTE: Register the target ID with local Firestore client as active - // watch target. - syncEngineImpl.sharedClientState.addLocalQueryTarget(targetData.targetId); + const targetId = targetData.targetId; - if (syncEngineImpl.isPrimaryClient) { + // PORTING NOTE: When the query is listening to cache only, we skip sending it over to Watch by + // not registering it in shared client state, and directly calculate initial snapshots and + // subsequent updates from cache. Otherwise, register the target ID with local Firestore client + // as active watch target. + const status: QueryTargetState = shouldListenToRemote + ? syncEngineImpl.sharedClientState.addLocalQueryTarget(targetId) + : 'not-current'; + + if (syncEngineImpl.isPrimaryClient && shouldListenToRemote) { remoteStoreListen(syncEngineImpl.remoteStore, targetData); } + + if (shouldInitializeView) { + return initializeViewAndComputeSnapshot( + syncEngineImpl, + query, + targetId, + status === 'current', + targetData.resumeToken + ); + } } /** @@ -475,6 +479,7 @@ export async function syncEngineUnlisten( } } +/** Unlistens to the remote store while still listening to the cache. */ export async function triggerRemoteStoreUnlisten( syncEngine: SyncEngine, query: Query From 232ae385b6f331b38ec57763232f5ab65240e3df Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 8 Feb 2024 12:40:58 -0500 Subject: [PATCH 22/33] change the order of initializeViewAndComputeSnapshot and remoteStoreListen --- packages/firestore/src/core/sync_engine_impl.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index 9daec1016b1..b4cbe893959 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -352,18 +352,21 @@ async function allocateTargetAndMaybeListen( ? syncEngineImpl.sharedClientState.addLocalQueryTarget(targetId) : 'not-current'; - if (syncEngineImpl.isPrimaryClient && shouldListenToRemote) { - remoteStoreListen(syncEngineImpl.remoteStore, targetData); - } - + let viewSnapshot; if (shouldInitializeView) { - return initializeViewAndComputeSnapshot( + viewSnapshot = await initializeViewAndComputeSnapshot( syncEngineImpl, query, targetId, status === 'current', targetData.resumeToken ); + + if (syncEngineImpl.isPrimaryClient && shouldListenToRemote) { + remoteStoreListen(syncEngineImpl.remoteStore, targetData); + } + + return viewSnapshot; } } From 40962f912181abf77c9b09b5f9b30d5f2d61cf8b Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 8 Feb 2024 13:36:29 -0500 Subject: [PATCH 23/33] Update sync_engine_impl.ts --- packages/firestore/src/core/sync_engine_impl.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index b4cbe893959..23076b6ac81 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -361,13 +361,13 @@ async function allocateTargetAndMaybeListen( status === 'current', targetData.resumeToken ); + } - if (syncEngineImpl.isPrimaryClient && shouldListenToRemote) { - remoteStoreListen(syncEngineImpl.remoteStore, targetData); - } - - return viewSnapshot; + if (syncEngineImpl.isPrimaryClient && shouldListenToRemote) { + remoteStoreListen(syncEngineImpl.remoteStore, targetData); } + + return viewSnapshot; } /** From a945668ad1cb884984f46cc8618c3ad7fb824ebe Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 12 Feb 2024 09:52:04 -0500 Subject: [PATCH 24/33] resolve comments --- .../firestore/src/core/sync_engine_impl.ts | 9 +- .../test/integration/api/bundle.test.ts | 58 ++-- .../api/snasphot_listener_source.test.ts | 299 +++++++++--------- 3 files changed, 192 insertions(+), 174 deletions(-) diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index 23076b6ac81..f4db6f4a5bd 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -314,7 +314,7 @@ export async function syncEngineListen( syncEngineImpl, query, shouldListenToRemote, - true + /** shouldInitializeView= */ true ); debugAssert(!!viewSnapshot, 'viewSnapshot is not initialized'); } @@ -328,7 +328,12 @@ export async function triggerRemoteStoreListen( query: Query ): Promise { const syncEngineImpl = ensureWatchCallbacks(syncEngine); - await allocateTargetAndMaybeListen(syncEngineImpl, query, true, false); + await allocateTargetAndMaybeListen( + syncEngineImpl, + query, + /** shouldListenToRemote= */ true, + /** shouldInitializeView= */ false + ); } async function allocateTargetAndMaybeListen( diff --git a/packages/firestore/test/integration/api/bundle.test.ts b/packages/firestore/test/integration/api/bundle.test.ts index f4d6c182de3..f06a8b4c7b8 100644 --- a/packages/firestore/test/integration/api/bundle.test.ts +++ b/packages/firestore/test/integration/api/bundle.test.ts @@ -41,35 +41,7 @@ import { export const encoder = new TextEncoder(); -/** - * Returns a valid bundle string from replacing project id in `BUNDLE_TEMPLATE` with the given - * db project id (also recalculate length prefixes). - */ -export function bundleString(db: Firestore): string { - const projectId: string = db.app.options.projectId!; - - // Extract elements from BUNDLE_TEMPLATE and replace the project ID. - const elements = BUNDLE_TEMPLATE.map(e => - e.replace('{0}', projectId).replace('(default)', db._databaseId.database) - ); - - // Recalculating length prefixes for elements that are not BundleMetadata. - let bundleContent = ''; - for (const element of elements.slice(1)) { - const length = encoder.encode(element).byteLength; - bundleContent += `${length}${element}`; - } - - // Update BundleMetadata with new totalBytes. - const totalBytes = encoder.encode(bundleContent).byteLength; - const metadata = JSON.parse(elements[0]); - metadata.metadata.totalBytes = totalBytes; - const metadataContent = JSON.stringify(metadata); - const metadataLength = encoder.encode(metadataContent).byteLength; - return `${metadataLength}${metadataContent}${bundleContent}`; -} - -export function verifySuccessProgress(p: LoadBundleTaskProgress): void { +function verifySuccessProgress(p: LoadBundleTaskProgress): void { expect(p.taskState).to.equal('Success'); expect(p.bytesLoaded).to.be.equal(p.totalBytes); expect(p.documentsLoaded).to.equal(p.totalDocuments); @@ -105,6 +77,34 @@ apiDescribe('Bundles', persistence => { ]); } + /** + * Returns a valid bundle string from replacing project id in `BUNDLE_TEMPLATE` with the given + * db project id (also recalculate length prefixes). + */ + function bundleString(db: Firestore): string { + const projectId: string = db.app.options.projectId!; + + // Extract elements from BUNDLE_TEMPLATE and replace the project ID. + const elements = BUNDLE_TEMPLATE.map(e => + e.replace('{0}', projectId).replace('(default)', db._databaseId.database) + ); + + // Recalculating length prefixes for elements that are not BundleMetadata. + let bundleContent = ''; + for (const element of elements.slice(1)) { + const length = encoder.encode(element).byteLength; + bundleContent += `${length}${element}`; + } + + // Update BundleMetadata with new totalBytes. + const totalBytes = encoder.encode(bundleContent).byteLength; + const metadata = JSON.parse(elements[0]); + metadata.metadata.totalBytes = totalBytes; + const metadataContent = JSON.stringify(metadata); + const metadataLength = encoder.encode(metadataContent).byteLength; + return `${metadataLength}${metadataContent}${bundleContent}`; + } + it('load with documents only with on progress and promise interface', () => { return withTestDb(persistence, async db => { const progressEvents: LoadBundleTaskProgress[] = []; diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index 5756fcf0f5e..1d9112ba678 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -20,7 +20,6 @@ import { expect } from 'chai'; import { EventsAccumulator } from '../util/events_accumulator'; import { addDoc, - collection, disableNetwork, doc, DocumentSnapshot, @@ -30,14 +29,11 @@ import { limit, limitToLast, ListenSource, - loadBundle, - namedQuery, onSnapshot, orderBy, query, QuerySnapshot, runTransaction, - setDoc, updateDoc, where } from '../util/firebase_export'; @@ -45,12 +41,9 @@ import { apiDescribe, toDataArray, withTestCollection, - withTestDb, withTestDocAndInitialData } from '../util/helpers'; -import { bundleString, verifySuccessProgress } from './bundle.test'; - apiDescribe('Snapshot Listener source options ', persistence => { // eslint-disable-next-line no-restricted-properties (persistence.gc === 'lru' ? describe : describe.skip)( @@ -58,21 +51,21 @@ apiDescribe('Snapshot Listener source options ', persistence => { () => { it('can raise snapshot from cache for Query', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a' } }; return withTestCollection(persistence, testDocs, async coll => { await getDocs(coll); // Populate the cache. const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( - query(coll, orderBy('sort', 'asc')), + coll, { source: ListenSource.Cache }, storeEvent.storeEvent ); const snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.equal(true); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a' }]); await storeEvent.assertNoAdditionalEvents(); unsubscribe(); @@ -80,7 +73,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); it('can raise snapshot from cache for DocumentReference', () => { - const testDocs = { k: 'a', sort: 0 }; + const testDocs = { k: 'a' }; return withTestDocAndInitialData( persistence, testDocs, @@ -96,7 +89,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.equal(true); - expect(snapshot.data()).to.deep.equal({ k: 'a', sort: 0 }); + expect(snapshot.data()).to.deep.equal({ k: 'a' }); await storeEvent.assertNoAdditionalEvents(); unsubscribe(); @@ -106,7 +99,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('listen to cache would not be affected by online status change', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a' } }; return withTestCollection(persistence, testDocs, async (coll, db) => { await getDocs(coll); // Populate the cache. @@ -120,7 +113,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.equal(true); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a' }]); await disableNetwork(db); await enableNetwork(db); @@ -132,39 +125,45 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('multiple listeners sourced from cache can work independently', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async coll => { await getDocs(coll); // Populate the cache. + const testQuery = query( + coll, + where('k', '>', 'a'), + orderBy('sort', 'asc') + ); const storeEvent = new EventsAccumulator(); const unsubscribe1 = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { source: ListenSource.Cache }, storeEvent.storeEvent ); const unsubscribe2 = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { source: ListenSource.Cache }, storeEvent.storeEvent ); let snapshots = await storeEvent.awaitEvents(2); expect(toDataArray(snapshots[0])).to.deep.equal([ - { k: 'a', sort: 0 } + { k: 'b', sort: 1 } ]); expect(snapshots[0].metadata).to.deep.equal(snapshots[1].metadata); expect(toDataArray(snapshots[0])).to.deep.equal( toDataArray(snapshots[1]) ); - await addDoc(coll, { k: 'b', sort: 1 }); + await addDoc(coll, { k: 'c', sort: 2 }); snapshots = await storeEvent.awaitEvents(2); expect(toDataArray(snapshots[0])).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } + { k: 'b', sort: 1 }, + { k: 'c', sort: 2 } ]); expect(snapshots[0].metadata).to.deep.equal(snapshots[1].metadata); expect(toDataArray(snapshots[0])).to.deep.equal( @@ -175,14 +174,14 @@ apiDescribe('Snapshot Listener source options ', persistence => { // should not be affected. unsubscribe1(); - await addDoc(coll, { k: 'c', sort: 2 }); + await addDoc(coll, { k: 'd', sort: 3 }); const snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.equal(true); expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, { k: 'b', sort: 1 }, - { k: 'c', sort: 2 } + { k: 'c', sort: 2 }, + { k: 'd', sort: 3 } ]); await storeEvent.assertNoAdditionalEvents(); unsubscribe2(); @@ -297,28 +296,35 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('can listen to default source first and then cache', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async coll => { // Listen to the query with default options, which will also populates the cache const storeDefaultEvent = new EventsAccumulator(); + const testQuery = query( + coll, + where('k', '>=', 'b'), + orderBy('sort', 'asc') + ); + const defaultUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, storeDefaultEvent.storeEvent ); let snapshot = await storeDefaultEvent.awaitRemoteEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'b', sort: 1 }]); expect(snapshot.metadata.fromCache).to.equal(false); // Listen to the same query from cache const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { source: ListenSource.Cache }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'b', sort: 1 }]); // The metadata is sync with server due to the default listener expect(snapshot.metadata.fromCache).to.equal(false); @@ -332,13 +338,20 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('can listen to cache source first and then default', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async coll => { // Listen to the cache const storeCacheEvent = new EventsAccumulator(); + const testQuery = query( + coll, + where('k', '>=', 'b'), + orderBy('sort', 'asc') + ); + const cacheUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { source: ListenSource.Cache }, storeCacheEvent.storeEvent ); @@ -350,16 +363,17 @@ apiDescribe('Snapshot Listener source options ', persistence => { // Listen to the same query from server const storeDefaultEvent = new EventsAccumulator(); const defaultUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, storeDefaultEvent.storeEvent ); snapshot = await storeDefaultEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + const expectedData = [{ k: 'b', sort: 1 }]; + expect(toDataArray(snapshot)).to.deep.equal(expectedData); expect(snapshot.metadata.fromCache).to.equal(false); - // Default listener updates the cache, whish triggers cache listener to raise snapshot. + // Default listener updates the cache, which triggers cache listener to raise snapshot. snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal(expectedData); // The metadata is sync with server due to the default listener expect(snapshot.metadata.fromCache).to.equal(false); @@ -373,30 +387,36 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('will not get metadata only updates if listening to cache only', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async coll => { await getDocs(coll); // Populate the cache. + const testQuery = query( + coll, + where('k', '!=', 'a'), + orderBy('sort', 'asc') + ); const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { includeMetadataChanges: true, source: ListenSource.Cache }, storeEvent.storeEvent ); let snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.equal(true); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'b', sort: 1 }]); - await addDoc(coll, { k: 'b', sort: 1 }); + await addDoc(coll, { k: 'c', sort: 2 }); snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.hasPendingWrites).to.equal(true); expect(snapshot.metadata.fromCache).to.equal(true); expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } + { k: 'b', sort: 1 }, + { k: 'c', sort: 2 } ]); // As we are not listening to server, the listener will not get notified @@ -408,31 +428,37 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('will have synced metadata updates when listening to both cache and default source', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async coll => { await getDocs(coll); // Populate the cache. + const testQuery = query( + coll, + where('k', '!=', 'a'), + orderBy('sort', 'asc') + ); // Listen to the query from cache const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { includeMetadataChanges: true, source: ListenSource.Cache }, storeCacheEvent.storeEvent ); let snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'b', sort: 1 }]); expect(snapshot.metadata.fromCache).to.equal(true); // Listen to the same query from server const storeDefaultEvent = new EventsAccumulator(); const defaultUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { includeMetadataChanges: true }, storeDefaultEvent.storeEvent ); snapshot = await storeDefaultEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal([{ k: 'b', sort: 1 }]); // First snapshot will be raised from cache. expect(snapshot.metadata.fromCache).to.equal(true); snapshot = await storeDefaultEvent.awaitEvent(); @@ -444,22 +470,20 @@ apiDescribe('Snapshot Listener source options ', persistence => { snapshot = await storeCacheEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.equal(false); - await addDoc(coll, { k: 'b', sort: 1 }); + await addDoc(coll, { k: 'c', sort: 2 }); // snapshot gets triggered by local mutation snapshot = await storeDefaultEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); + const expectedData = [ + { k: 'b', sort: 1 }, + { k: 'c', sort: 2 } + ]; + expect(toDataArray(snapshot)).to.deep.equal(expectedData); expect(snapshot.metadata.hasPendingWrites).to.equal(true); expect(snapshot.metadata.fromCache).to.equal(false); snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); + expect(toDataArray(snapshot)).to.deep.equal(expectedData); expect(snapshot.metadata.hasPendingWrites).to.equal(true); expect(snapshot.metadata.fromCache).to.equal(false); @@ -479,19 +503,26 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('can un-listen to default source while still listening to cache', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async coll => { + const testQuery = query( + coll, + where('k', '>', 'a'), + orderBy('sort', 'asc') + ); + // Listen to the query with both source options const storeDefaultEvent = new EventsAccumulator(); const defaultUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, storeDefaultEvent.storeEvent ); await storeDefaultEvent.awaitEvent(); const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { source: ListenSource.Cache }, storeCacheEvent.storeEvent ); @@ -502,12 +533,12 @@ apiDescribe('Snapshot Listener source options ', persistence => { await storeDefaultEvent.assertNoAdditionalEvents(); // Add a document and verify listener to cache works as expected - await addDoc(coll, { k: 'b', sort: -1 }); + await addDoc(coll, { k: 'c', sort: -1 }); const snapshot = await storeCacheEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'b', sort: -1 }, - { k: 'a', sort: 0 } + { k: 'c', sort: -1 }, + { k: 'b', sort: 1 } ]); await storeCacheEvent.assertNoAdditionalEvents(); @@ -517,19 +548,26 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('can un-listen to cache while still listening to server', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async coll => { + const testQuery = query( + coll, + where('k', '>', 'a'), + orderBy('sort', 'asc') + ); + // Listen to the query with both source options const storeDefaultEvent = new EventsAccumulator(); const defaultUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, storeDefaultEvent.storeEvent ); await storeDefaultEvent.awaitEvent(); const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { source: ListenSource.Cache }, storeCacheEvent.storeEvent ); @@ -540,12 +578,12 @@ apiDescribe('Snapshot Listener source options ', persistence => { await storeCacheEvent.assertNoAdditionalEvents(); // Add a document and verify listener to server works as expected. - await addDoc(coll, { k: 'b', sort: -1 }); + await addDoc(coll, { k: 'c', sort: -1 }); const snapshot = await storeDefaultEvent.awaitEvent(); expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'b', sort: -1 }, - { k: 'a', sort: 0 } + { k: 'c', sort: -1 }, + { k: 'b', sort: 1 } ]); await storeDefaultEvent.assertNoAdditionalEvents(); @@ -555,69 +593,70 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('can listen/un-listen/re-listen to same query with different source options', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async coll => { + const testQuery = query( + coll, + where('k', '>', 'a'), + orderBy('sort', 'asc') + ); + // Listen to the query with default options, which also populates the cache const storeDefaultEvent = new EventsAccumulator(); let defaultUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, storeDefaultEvent.storeEvent ); let snapshot = await storeDefaultEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + let expectedData = [{ k: 'b', sort: 1 }]; + expect(toDataArray(snapshot)).to.deep.equal(expectedData); // Listen to the same query from cache const storeCacheEvent = new EventsAccumulator(); let cacheUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { source: ListenSource.Cache }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal(expectedData); // Un-listen to the default listener, add a doc and re-listen. defaultUnlisten(); - await addDoc(coll, { k: 'b', sort: 1 }); + await addDoc(coll, { k: 'c', sort: 2 }); snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); + expectedData = [ + { k: 'b', sort: 1 }, + { k: 'c', sort: 2 } + ]; + expect(toDataArray(snapshot)).to.deep.equal(expectedData); - defaultUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), - storeDefaultEvent.storeEvent - ); + defaultUnlisten = onSnapshot(testQuery, storeDefaultEvent.storeEvent); snapshot = await storeDefaultEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); + expect(toDataArray(snapshot)).to.deep.equal(expectedData); // Un-listen to cache, update a doc, then re-listen to cache. cacheUnlisten(); - await updateDoc(doc(coll, 'a'), { k: 'a', sort: 2 }); + await updateDoc(doc(coll, 'b'), { k: 'b', sort: 3 }); snapshot = await storeDefaultEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'b', sort: 1 }, - { k: 'a', sort: 2 } - ]); + expectedData = [ + { k: 'c', sort: 2 }, + { k: 'b', sort: 3 } + ]; + expect(toDataArray(snapshot)).to.deep.equal(expectedData); cacheUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { source: ListenSource.Cache }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'b', sort: 1 }, - { k: 'a', sort: 2 } - ]); + expect(toDataArray(snapshot)).to.deep.equal(expectedData); defaultUnlisten(); cacheUnlisten(); @@ -626,7 +665,8 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('can listen to composite index queries from cache', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async coll => { await getDocs(coll); // Populate the cache. @@ -666,39 +706,6 @@ apiDescribe('Snapshot Listener source options ', persistence => { }); }); - it('load documents pulled from backend while listening to cache', () => { - return withTestDb(persistence, async db => { - await setDoc(doc(db, 'coll-1/a'), { k: 'a', bar: 0 }); - await setDoc(doc(db, 'coll-1/b'), { k: 'b', bar: 0 }); - - const accumulator = new EventsAccumulator(); - onSnapshot( - collection(db, 'coll-1'), - { source: ListenSource.Cache }, - accumulator.storeEvent - ); - await accumulator.awaitEvent(); - const encoder = new TextEncoder(); - const progress = await loadBundle( - db, - // Testing passing in non-string bundles. - encoder.encode(bundleString(db)) - ); - - verifySuccessProgress(progress); - // The test bundle is holding ancient documents, so no events are - // generated as a result. The case where a bundle has newer doc than - // cache can only be tested in spec tests. - await accumulator.assertNoAdditionalEvents(); - - let snapshot = await getDocs((await namedQuery(db, 'limit'))!); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'b', bar: 0 }]); - - snapshot = await getDocs((await namedQuery(db, 'limit-to-last'))!); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', bar: 0 }]); - }); - }); - it('will not be triggered by transactions while listening to cache', () => { return withTestCollection(persistence, {}, async (coll, db) => { const accumulator = new EventsAccumulator(); @@ -725,48 +732,54 @@ apiDescribe('Snapshot Listener source options ', persistence => { it('share server side updates when listening to both cache and default', () => { const testDocs = { - a: { k: 'a', sort: 0 } + a: { k: 'a', sort: 0 }, + b: { k: 'b', sort: 1 } }; return withTestCollection(persistence, testDocs, async (coll, db) => { + const testQuery = query( + coll, + where('k', '>', 'a'), + orderBy('sort', 'asc') + ); + // Listen to the query with default options, which will also populates the cache const storeDefaultEvent = new EventsAccumulator(); const defaultUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, storeDefaultEvent.storeEvent ); let snapshot = await storeDefaultEvent.awaitRemoteEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + let expectedData = [{ k: 'b', sort: 1 }]; + expect(toDataArray(snapshot)).to.deep.equal(expectedData); // Listen to the same query from cache const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( - query(coll, orderBy('sort', 'asc')), + testQuery, { source: ListenSource.Cache }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([{ k: 'a', sort: 0 }]); + expect(toDataArray(snapshot)).to.deep.equal(expectedData); // Use a transaction to mock server side updates const docRef = doc(coll); await runTransaction(db, async txn => { - txn.set(docRef, { k: 'b', sort: 1 }); + txn.set(docRef, { k: 'c', sort: 2 }); }); // Default listener receives the server update snapshot = await storeDefaultEvent.awaitRemoteEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); + expectedData = [ + { k: 'b', sort: 1 }, + { k: 'c', sort: 2 } + ]; + expect(toDataArray(snapshot)).to.deep.equal(expectedData); expect(snapshot.metadata.fromCache).to.be.false; // Cache listener raises snapshot as well snapshot = await storeCacheEvent.awaitEvent(); - expect(toDataArray(snapshot)).to.deep.equal([ - { k: 'a', sort: 0 }, - { k: 'b', sort: 1 } - ]); + expect(toDataArray(snapshot)).to.deep.equal(expectedData); expect(snapshot.metadata.fromCache).to.be.false; defaultUnlisten(); From f35d483a2d3b1f9e516b59f66a575218dc6dc4cd Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:20:07 -0500 Subject: [PATCH 25/33] Update snasphot_listener_source.test.ts --- .../api/snasphot_listener_source.test.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index 1d9112ba678..8b151fb31d6 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -44,7 +44,7 @@ import { withTestDocAndInitialData } from '../util/helpers'; -apiDescribe('Snapshot Listener source options ', persistence => { +apiDescribe.only('Snapshot Listener source options ', persistence => { // eslint-disable-next-line no-restricted-properties (persistence.gc === 'lru' ? describe : describe.skip)( 'listen to persistence cache', @@ -132,7 +132,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { await getDocs(coll); // Populate the cache. const testQuery = query( coll, - where('k', '>', 'a'), + where('sort', '>', 0), orderBy('sort', 'asc') ); @@ -304,7 +304,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeDefaultEvent = new EventsAccumulator(); const testQuery = query( coll, - where('k', '>=', 'b'), + where('sort', '>=', 1), orderBy('sort', 'asc') ); @@ -346,7 +346,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const testQuery = query( coll, - where('k', '>=', 'b'), + where('sort', '!=', 0), orderBy('sort', 'asc') ); @@ -394,7 +394,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { await getDocs(coll); // Populate the cache. const testQuery = query( coll, - where('k', '!=', 'a'), + where('sort', '!=', 0), orderBy('sort', 'asc') ); @@ -435,7 +435,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { await getDocs(coll); // Populate the cache. const testQuery = query( coll, - where('k', '!=', 'a'), + where('sort', '!=', 0), orderBy('sort', 'asc') ); @@ -509,7 +509,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { return withTestCollection(persistence, testDocs, async coll => { const testQuery = query( coll, - where('k', '>', 'a'), + where('sort', '!=', 0), orderBy('sort', 'asc') ); @@ -554,7 +554,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { return withTestCollection(persistence, testDocs, async coll => { const testQuery = query( coll, - where('k', '>', 'a'), + where('sort', '!=', 0), orderBy('sort', 'asc') ); @@ -599,7 +599,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { return withTestCollection(persistence, testDocs, async coll => { const testQuery = query( coll, - where('k', '>', 'a'), + where('sort', '>', 0), orderBy('sort', 'asc') ); @@ -738,7 +738,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { return withTestCollection(persistence, testDocs, async (coll, db) => { const testQuery = query( coll, - where('k', '>', 'a'), + where('sort', '>', 0), orderBy('sort', 'asc') ); From d415d4d9c8dfbf435b598d88033704e84e3f1e2e Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 12 Feb 2024 11:43:41 -0500 Subject: [PATCH 26/33] remove .only --- .../test/integration/api/snasphot_listener_source.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index 8b151fb31d6..e75242c5a52 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -44,7 +44,7 @@ import { withTestDocAndInitialData } from '../util/helpers'; -apiDescribe.only('Snapshot Listener source options ', persistence => { +apiDescribe('Snapshot Listener source options ', persistence => { // eslint-disable-next-line no-restricted-properties (persistence.gc === 'lru' ? describe : describe.skip)( 'listen to persistence cache', From c5c558dadfbec5a3ed06d1277917e29e3f1b0c75 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 13 Feb 2024 13:03:29 -0500 Subject: [PATCH 27/33] resolve comments --- .../api/snasphot_listener_source.test.ts | 4 +- .../unit/specs/listen_source_spec.test.ts | 43 +++++++++---------- 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index e75242c5a52..0a92d95a6d9 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -671,14 +671,14 @@ apiDescribe('Snapshot Listener source options ', persistence => { return withTestCollection(persistence, testDocs, async coll => { await getDocs(coll); // Populate the cache. - const query_ = query( + const testQuery = query( coll, where('k', '<=', 'a'), where('sort', '>=', 0) ); const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( - query_, + testQuery, { source: ListenSource.Cache }, storeEvent.storeEvent ); diff --git a/packages/firestore/test/unit/specs/listen_source_spec.test.ts b/packages/firestore/test/unit/specs/listen_source_spec.test.ts index 3d968a8d435..ee2a9cf5944 100644 --- a/packages/firestore/test/unit/specs/listen_source_spec.test.ts +++ b/packages/firestore/test/unit/specs/listen_source_spec.test.ts @@ -16,7 +16,7 @@ */ import { LimitType, queryWithLimit } from '../../../src/core/query'; -import { deletedDoc, doc, filter, orderBy, query } from '../../util/helpers'; +import { doc, filter, orderBy, query } from '../../util/helpers'; import { bundleWithDocumentAndQuery } from './bundle_spec.test'; import { describeSpec, specTest } from './describe_spec'; @@ -28,22 +28,22 @@ describeSpec('Listens source options:', [], () => { ['eager-gc'], 'Explicitly tests eager GC behavior', () => { - const query_ = query('collection'); + const testQuery = query('collection'); const docA = doc('collection/a', 0, { key: 'a' }).setHasLocalMutations(); return ( spec() .userSets('collection/a', { key: 'a' }) - .userListensToCache(query_) - .expectEvents(query_, { + .userListensToCache(testQuery) + .expectEvents(testQuery, { added: [docA], hasPendingWrites: true, fromCache: true }) - .userUnlistensToCache(query_) + .userUnlistensToCache(testQuery) .writeAcks('collection/a', 1000) // Cache is empty as docA is GCed. - .userListensToCache(query_) - .expectEvents(query_, { added: [], fromCache: true }) + .userListensToCache(testQuery) + .expectEvents(testQuery, { added: [], fromCache: true }) ); } ); @@ -51,7 +51,6 @@ describeSpec('Listens source options:', [], () => { specTest( 'Documents are cleared when listen is removed.', ['eager-gc'], - '', () => { const filteredQuery = query('collection', filter('matches', '==', true)); const unfilteredQuery = query('collection'); @@ -91,15 +90,15 @@ describeSpec('Listens source options:', [], () => { ); specTest("Doesn't include unknown documents in cached result", [], () => { - const query_ = query('collection'); + const testQuery = query('collection'); const existingDoc = doc('collection/exists', 0, { key: 'a' }).setHasLocalMutations(); return spec() .userSets('collection/exists', { key: 'a' }) .userPatches('collection/unknown', { key: 'b' }) - .userListensToCache(query_) - .expectEvents(query_, { + .userListensToCache(testQuery) + .expectEvents(testQuery, { added: [existingDoc], fromCache: true, hasPendingWrites: true @@ -107,26 +106,24 @@ describeSpec('Listens source options:', [], () => { }); specTest("Doesn't raise 'hasPendingWrites' for deletes", [], () => { - const query_ = query('collection'); + const testQuery = query('collection'); const docA = doc('collection/a', 1000, { key: 'a' }); return ( spec() .ensureManualLruGC() // Populate the cache first - .userListens(query_) - .watchAcksFull(query_, 1000, docA) - .expectEvents(query_, { added: [docA] }) - .userUnlistens(query_) - .watchRemoves(query_) + .userListens(testQuery) + .watchAcksFull(testQuery, 1000, docA) + .expectEvents(testQuery, { added: [docA] }) + .userUnlistens(testQuery) + .watchRemoves(testQuery) // Listen to cache - .userListensToCache(query_) - .expectEvents(query_, { added: [docA], fromCache: true }) + .userListensToCache(testQuery) + .expectEvents(testQuery, { added: [docA], fromCache: true }) .userDeletes('collection/a') - .expectEvents(query_, { removed: [docA], fromCache: true }) + .expectEvents(testQuery, { removed: [docA], fromCache: true }) .writeAcks('collection/a', 2000) - .watchSends({ affects: [query_] }, deletedDoc('collection/a', 2000)) - .watchSnapshots(2000) ); }); @@ -554,7 +551,7 @@ describeSpec('Listens source options:', [], () => { ); specTest( - 'Query is executed by primary client even if it only includes listeners sourced from cache', + 'Query is executed by primary client even if primary client only has listeners sourced from cache', ['multi-client'], () => { const query1 = query('collection'); From c969f2394ba37dd5a72358fc24b1a5d3760f1bca Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 14 Feb 2024 16:11:51 -0500 Subject: [PATCH 28/33] Update event_manager.ts --- packages/firestore/src/core/event_manager.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index b58029e8288..317c6ac2795 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -145,7 +145,10 @@ export async function eventManagerListen( listenerAction = listener.listensToRemoteStore() ? ListenerSetupAction.InitializeLocalListenAndRequireWatchConnection : ListenerSetupAction.InitializeLocalListenOnly; - } else if (listener.listensToRemoteStore()) { + } else if ( + !queryInfo.hasRemoteListeners() && + listener.listensToRemoteStore() + ) { // Query has been listening to local cache, and tries to add a new listener sourced from watch. listenerAction = ListenerSetupAction.RequireWatchConnectionOnly; } From 533a99aac8c46b70443e14c7d43bfc5a33c6b2fa Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Tue, 20 Feb 2024 14:07:58 -0500 Subject: [PATCH 29/33] update change set --- .changeset/smart-games-cheer.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/smart-games-cheer.md b/.changeset/smart-games-cheer.md index d99d378089d..e073d62c03c 100644 --- a/.changeset/smart-games-cheer.md +++ b/.changeset/smart-games-cheer.md @@ -2,4 +2,4 @@ '@firebase/firestore': minor 'firebase': minor --- -Enable snapshot listeners to source from local cache. +Enable snapshot listener option to retrieve data from local cache only. From ec1f784b4b2ca1d2696e3be1e76280572f675282 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 8 Mar 2024 11:58:21 -0500 Subject: [PATCH 30/33] change ListenSource from Enum to union type --- common/api-review/firestore.api.md | 5 +- docs-devsite/firestore_.md | 37 +++++--------- .../firestore_.snapshotlistenoptions.md | 4 +- packages/firestore/src/api/reference_impl.ts | 21 ++++---- packages/firestore/src/core/event_manager.ts | 13 +++-- .../api/snasphot_listener_source.test.ts | 48 +++++++++---------- .../firestore/test/unit/specs/spec_builder.ts | 12 ++--- .../test/unit/specs/spec_test_runner.ts | 8 ++-- 8 files changed, 68 insertions(+), 80 deletions(-) diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 0bf6e70bce9..f79ef52442e 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -351,10 +351,7 @@ export function limit(limit: number): QueryLimitConstraint; export function limitToLast(limit: number): QueryLimitConstraint; // @public -export const enum ListenSource { - Cache = 1, - Default = 0 -} +export type ListenSource = 'default' | 'cache'; // @public export function loadBundle(firestore: Firestore, bundleData: ReadableStream | ArrayBuffer | string): LoadBundleTask; diff --git a/docs-devsite/firestore_.md b/docs-devsite/firestore_.md index 7e213d118f0..376d393e666 100644 --- a/docs-devsite/firestore_.md +++ b/docs-devsite/firestore_.md @@ -157,12 +157,6 @@ https://github.com/firebase/firebase-js-sdk | [Transaction](./firestore_.transaction.md#transaction_class) | A reference to a transaction.The Transaction object passed to a transaction's updateFunction provides the methods to read and write data within the transaction context. See [runTransaction()](./firestore_.md#runtransaction_6f03ec4). | | [WriteBatch](./firestore_.writebatch.md#writebatch_class) | A write batch, used to perform multiple writes as a single atomic unit.A WriteBatch object can be acquired by calling [writeBatch()](./firestore_.md#writebatch_231a8e0). It provides methods for adding writes to the write batch. None of the writes will be committed (or visible locally) until [WriteBatch.commit()](./firestore_.writebatch.md#writebatchcommit) is called. | -## Enumerations - -| Enumeration | Description | -| --- | --- | -| [ListenSource](./firestore_.md#listensource) | Describe the source a query listens to. | - ## Interfaces | Interface | Description | @@ -210,6 +204,7 @@ https://github.com/firebase/firebase-js-sdk | [DocumentChangeType](./firestore_.md#documentchangetype) | The type of a DocumentChange may be 'added', 'removed', or 'modified'. | | [FirestoreErrorCode](./firestore_.md#firestoreerrorcode) | The set of Firestore status codes. The codes are the same at the ones exposed by gRPC here: https://github.com/grpc/grpc/blob/master/doc/statuscodes.mdPossible values: - 'cancelled': The operation was cancelled (typically by the caller). - 'unknown': Unknown error or an error from a different error domain. - 'invalid-argument': Client specified an invalid argument. Note that this differs from 'failed-precondition'. 'invalid-argument' indicates arguments that are problematic regardless of the state of the system (e.g. an invalid field name). - 'deadline-exceeded': Deadline expired before operation could complete. For operations that change the state of the system, this error may be returned even if the operation has completed successfully. For example, a successful response from a server could have been delayed long enough for the deadline to expire. - 'not-found': Some requested document was not found. - 'already-exists': Some document that we attempted to create already exists. - 'permission-denied': The caller does not have permission to execute the specified operation. - 'resource-exhausted': Some resource has been exhausted, perhaps a per-user quota, or perhaps the entire file system is out of space. - 'failed-precondition': Operation was rejected because the system is not in a state required for the operation's execution. - 'aborted': The operation was aborted, typically due to a concurrency issue like transaction aborts, etc. - 'out-of-range': Operation was attempted past the valid range. - 'unimplemented': Operation is not implemented or not supported/enabled. - 'internal': Internal errors. Means some invariants expected by underlying system has been broken. If you see one of these errors, something is very broken. - 'unavailable': The service is currently unavailable. This is most likely a transient condition and may be corrected by retrying with a backoff. - 'data-loss': Unrecoverable data loss or corruption. - 'unauthenticated': The request does not have valid authentication credentials for the operation. | | [FirestoreLocalCache](./firestore_.md#firestorelocalcache) | Union type from all supported SDK cache layer. | +| [ListenSource](./firestore_.md#listensource) | Describe the source a query listens to.default: listens to both cache and server changes cache: listens to changes in cache only | | [MemoryGarbageCollector](./firestore_.md#memorygarbagecollector) | Union type from all support gabage collectors for memory local cache. | | [NestedUpdateFields](./firestore_.md#nestedupdatefields) | For each field (e.g. 'bar'), find all nested keys (e.g. {'bar.baz': T1, 'bar.qux': T2}). Intersect them together to make a single map containing all possible keys that are all marked as optional | | [OrderByDirection](./firestore_.md#orderbydirection) | The direction of a [orderBy()](./firestore_.md#orderby_006d61f) clause is specified as 'desc' or 'asc' (descending or ascending). | @@ -2557,6 +2552,18 @@ Union type from all supported SDK cache layer. export declare type FirestoreLocalCache = MemoryLocalCache | PersistentLocalCache; ``` +## ListenSource + +Describe the source a query listens to. + +`default`: listens to both cache and server changes `cache`: listens to changes in cache only + +Signature: + +```typescript +export declare type ListenSource = 'default' | 'cache'; +``` + ## MemoryGarbageCollector Union type from all support gabage collectors for memory local cache. @@ -2722,21 +2729,3 @@ export declare type WithFieldValue = T | (T extends Primitive ? T : T extends [K in keyof T]: WithFieldValue | FieldValue; } : never); ``` - -## ListenSource - -Describe the source a query listens to. - -Signature: - -```typescript -export declare const enum ListenSource -``` - -## Enumeration Members - -| Member | Value | Description | -| --- | --- | --- | -| Cache | 1 | Listen to changes in cache only | -| Default | 0 | Listen to both cache and server changes | - diff --git a/docs-devsite/firestore_.snapshotlistenoptions.md b/docs-devsite/firestore_.snapshotlistenoptions.md index 2e11cc35ad1..33d769f4fd9 100644 --- a/docs-devsite/firestore_.snapshotlistenoptions.md +++ b/docs-devsite/firestore_.snapshotlistenoptions.md @@ -23,7 +23,7 @@ export declare interface SnapshotListenOptions | Property | Type | Description | | --- | --- | --- | | [includeMetadataChanges](./firestore_.snapshotlistenoptions.md#snapshotlistenoptionsincludemetadatachanges) | boolean | Include a change even if only the metadata of the query or of a document changed. Default is false. | -| [source](./firestore_.snapshotlistenoptions.md#snapshotlistenoptionssource) | [ListenSource](./firestore_.md#listensource) | Set the source the query listens to. Default to ListenSource.Default, which listens to both cache and server. | +| [source](./firestore_.snapshotlistenoptions.md#snapshotlistenoptionssource) | [ListenSource](./firestore_.md#listensource) | Set the source the query listens to. Default to "default", which listens to both cache and server. | ## SnapshotListenOptions.includeMetadataChanges @@ -37,7 +37,7 @@ readonly includeMetadataChanges?: boolean; ## SnapshotListenOptions.source -Set the source the query listens to. Default to ListenSource.Default, which listens to both cache and server. +Set the source the query listens to. Default to "default", which listens to both cache and server. Signature: diff --git a/packages/firestore/src/api/reference_impl.ts b/packages/firestore/src/api/reference_impl.ts index 90914344f83..750fc07c5c6 100644 --- a/packages/firestore/src/api/reference_impl.ts +++ b/packages/firestore/src/api/reference_impl.ts @@ -24,6 +24,7 @@ import { NextFn, PartialObserver } from '../api/observer'; +import { ListenerDataSource } from '../core/event_manager'; import { firestoreClientAddSnapshotsInSyncListener, firestoreClientGetDocumentFromLocalCache, @@ -80,20 +81,18 @@ export interface SnapshotListenOptions { readonly includeMetadataChanges?: boolean; /** - * Set the source the query listens to. Default to ListenSource.Default, which + * Set the source the query listens to. Default to "default", which * listens to both cache and server. */ readonly source?: ListenSource; } -/** Describe the source a query listens to. */ -export const enum ListenSource { - /** Listen to both cache and server changes */ - Default, - - /** Listen to changes in cache only */ - Cache -} +/** Describe the source a query listens to. + * + * `default`: listens to both cache and server changes + * `cache`: listens to changes in cache only + */ +export type ListenSource = 'default' | 'cache'; /** * Reads the document referred to by this `DocumentReference`. @@ -684,7 +683,7 @@ export function onSnapshot( let options: SnapshotListenOptions = { includeMetadataChanges: false, - source: ListenSource.Default + source: 'default' }; let currArg = 0; if (typeof args[currArg] === 'object' && !isPartialObserver(args[currArg])) { @@ -694,7 +693,7 @@ export function onSnapshot( const internalOptions = { includeMetadataChanges: options.includeMetadataChanges, - source: options.source + source: options.source as ListenerDataSource }; if (isPartialObserver(args[currArg])) { diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index 317c6ac2795..b53c45669cb 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -15,7 +15,6 @@ * limitations under the License. */ -import { ListenSource } from '../api/reference_impl'; import { debugAssert, debugCast } from '../util/assert'; import { wrapInUserErrorIfRecoverable } from '../util/async_queue'; import { FirestoreError } from '../util/error'; @@ -342,6 +341,14 @@ function raiseSnapshotsInSyncEvent(eventManagerImpl: EventManagerImpl): void { }); } +export enum ListenerDataSource { + /** Listen to both cache and server changes */ + Default = 'default', + + /** Listen to changes in cache only */ + Cache = 'cache' +} + export interface ListenOptions { /** Raise events even when only the metadata changes */ readonly includeMetadataChanges?: boolean; @@ -353,7 +360,7 @@ export interface ListenOptions { readonly waitForSyncWhenOnline?: boolean; /** Set the source events raised from. */ - readonly source?: ListenSource; + readonly source?: ListenerDataSource; } /** @@ -528,6 +535,6 @@ export class QueryListener { } listensToRemoteStore(): boolean { - return this.options.source !== ListenSource.Cache; + return this.options.source !== ListenerDataSource.Cache; } } diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index 0a92d95a6d9..cb765f52dd0 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -17,6 +17,7 @@ import { expect } from 'chai'; +import { ListenerDataSource as Source } from '../../../src/core/event_manager'; import { EventsAccumulator } from '../util/events_accumulator'; import { addDoc, @@ -28,7 +29,6 @@ import { getDocs, limit, limitToLast, - ListenSource, onSnapshot, orderBy, query, @@ -59,7 +59,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( coll, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeEvent.storeEvent ); @@ -83,7 +83,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( docRef, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeEvent.storeEvent ); @@ -107,7 +107,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( coll, - { includeMetadataChanges: true, source: ListenSource.Cache }, + { includeMetadataChanges: true, source: Source.Cache }, storeEvent.storeEvent ); @@ -139,13 +139,13 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe1 = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeEvent.storeEvent ); const unsubscribe2 = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeEvent.storeEvent ); @@ -207,7 +207,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeLimitEvent = new EventsAccumulator(); let limitUnlisten = onSnapshot( query(coll, orderBy('sort', 'asc'), limit(2)), - { source: ListenSource.Cache }, + { source: Source.Cache }, storeLimitEvent.storeEvent ); @@ -215,7 +215,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeLimitToLastEvent = new EventsAccumulator(); let limitToLastUnlisten = onSnapshot( query(coll, orderBy('sort', 'desc'), limitToLast(2)), - { source: ListenSource.Cache }, + { source: Source.Cache }, storeLimitToLastEvent.storeEvent ); @@ -235,7 +235,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { limitUnlisten(); limitUnlisten = onSnapshot( query(coll, orderBy('sort', 'asc'), limit(2)), - { source: ListenSource.Cache }, + { source: Source.Cache }, storeLimitEvent.storeEvent ); snapshot = await storeLimitEvent.awaitEvent(); @@ -269,7 +269,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { await updateDoc(doc(coll, 'a'), { k: 'a', sort: -2 }); limitToLastUnlisten = onSnapshot( query(coll, orderBy('sort', 'desc'), limitToLast(2)), - { source: ListenSource.Cache }, + { source: Source.Cache }, storeLimitToLastEvent.storeEvent ); @@ -320,7 +320,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent(); @@ -352,7 +352,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const cacheUnlisten = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeCacheEvent.storeEvent ); let snapshot = await storeCacheEvent.awaitEvent(); @@ -401,7 +401,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( testQuery, - { includeMetadataChanges: true, source: ListenSource.Cache }, + { includeMetadataChanges: true, source: Source.Cache }, storeEvent.storeEvent ); @@ -443,7 +443,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { includeMetadataChanges: true, source: ListenSource.Cache }, + { includeMetadataChanges: true, source: Source.Cache }, storeCacheEvent.storeEvent ); let snapshot = await storeCacheEvent.awaitEvent(); @@ -523,7 +523,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeCacheEvent.storeEvent ); await storeCacheEvent.awaitEvent(); @@ -568,7 +568,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeCacheEvent.storeEvent ); await storeCacheEvent.awaitEvent(); @@ -617,7 +617,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); let cacheUnlisten = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent(); @@ -651,7 +651,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { cacheUnlisten = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeCacheEvent.storeEvent ); @@ -679,7 +679,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeEvent.storeEvent ); @@ -695,11 +695,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { expect(toDataArray(snapshot)).to.deep.equal([]); // Precondition check. const storeEvent = new EventsAccumulator(); - onSnapshot( - coll, - { source: ListenSource.Cache }, - storeEvent.storeEvent - ); + onSnapshot(coll, { source: Source.Cache }, storeEvent.storeEvent); snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.be.true; expect(toDataArray(snapshot)).to.deep.equal([]); @@ -711,7 +707,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const accumulator = new EventsAccumulator(); const unsubscribe = onSnapshot( coll, - { source: ListenSource.Cache }, + { source: Source.Cache }, accumulator.storeEvent ); @@ -756,7 +752,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { source: ListenSource.Cache }, + { source: Source.Cache }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent(); diff --git a/packages/firestore/test/unit/specs/spec_builder.ts b/packages/firestore/test/unit/specs/spec_builder.ts index a03c2ba85a4..d79cca9cd82 100644 --- a/packages/firestore/test/unit/specs/spec_builder.ts +++ b/packages/firestore/test/unit/specs/spec_builder.ts @@ -16,11 +16,11 @@ */ import { IndexConfiguration } from '../../../src/api/index_configuration'; +import { ExpUserDataWriter } from '../../../src/api/reference_impl'; import { - ExpUserDataWriter, - ListenSource -} from '../../../src/api/reference_impl'; -import { ListenOptions } from '../../../src/core/event_manager'; + ListenOptions, + ListenerDataSource as Source +} from '../../../src/core/event_manager'; import { FieldFilter, Filter } from '../../../src/core/filter'; import { LimitType, @@ -294,7 +294,7 @@ export class SpecBuilder { this.queryMapping.set(target, targetId); - if (options?.source !== ListenSource.Cache) { + if (options?.source !== Source.Cache) { // Active targets are created if listener is not sourcing from cache this.addQueryToActiveTargets(targetId, query, resume); } @@ -316,7 +316,7 @@ export class SpecBuilder { } userListensToCache(query: Query, resume?: ResumeSpec): this { - this.addUserListenStep(query, resume, { source: ListenSource.Cache }); + this.addUserListenStep(query, resume, { source: Source.Cache }); return this; } diff --git a/packages/firestore/test/unit/specs/spec_test_runner.ts b/packages/firestore/test/unit/specs/spec_test_runner.ts index 9a922dc92eb..b34421d9e0a 100644 --- a/packages/firestore/test/unit/specs/spec_test_runner.ts +++ b/packages/firestore/test/unit/specs/spec_test_runner.ts @@ -27,7 +27,6 @@ import { IndexConfiguration, parseIndexes } from '../../../src/api/index_configuration'; -import { ListenSource } from '../../../src/api/reference_impl'; import { User } from '../../../src/auth/user'; import { ComponentConfiguration } from '../../../src/core/component_provider'; import { DatabaseInfo } from '../../../src/core/database_info'; @@ -39,7 +38,8 @@ import { QueryListener, removeSnapshotsInSyncListener, addSnapshotsInSyncListener, - ListenOptions + ListenOptions, + ListenerDataSource as Source } from '../../../src/core/event_manager'; import { canonifyQuery, @@ -498,7 +498,7 @@ abstract class TestRunner { includeMetadataChanges: listenSpec.options?.includeMetadataChanges ?? true, waitForSyncWhenOnline: false, - source: listenSpec.options?.source ?? ListenSource.Default + source: listenSpec.options?.source ?? Source.Default }; const queryListener = new QueryListener(query, aggregator, options); @@ -526,7 +526,7 @@ abstract class TestRunner { if ( this.isPrimaryClient && this.networkEnabled && - options.source !== ListenSource.Cache + options.source !== Source.Cache ) { // Unless listened to cache, open always have happened after a listen. await this.connection.waitForWatchOpen(); From 5fb1b8eb5ce31c1c9bf2692ba0b0b1ce712f12a4 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 8 Mar 2024 13:49:42 -0500 Subject: [PATCH 31/33] resolve comments --- docs-devsite/firestore_.md | 4 ++-- packages/firestore/src/api/reference_impl.ts | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs-devsite/firestore_.md b/docs-devsite/firestore_.md index 376d393e666..74a0c356523 100644 --- a/docs-devsite/firestore_.md +++ b/docs-devsite/firestore_.md @@ -204,7 +204,7 @@ https://github.com/firebase/firebase-js-sdk | [DocumentChangeType](./firestore_.md#documentchangetype) | The type of a DocumentChange may be 'added', 'removed', or 'modified'. | | [FirestoreErrorCode](./firestore_.md#firestoreerrorcode) | The set of Firestore status codes. The codes are the same at the ones exposed by gRPC here: https://github.com/grpc/grpc/blob/master/doc/statuscodes.mdPossible values: - 'cancelled': The operation was cancelled (typically by the caller). - 'unknown': Unknown error or an error from a different error domain. - 'invalid-argument': Client specified an invalid argument. Note that this differs from 'failed-precondition'. 'invalid-argument' indicates arguments that are problematic regardless of the state of the system (e.g. an invalid field name). - 'deadline-exceeded': Deadline expired before operation could complete. For operations that change the state of the system, this error may be returned even if the operation has completed successfully. For example, a successful response from a server could have been delayed long enough for the deadline to expire. - 'not-found': Some requested document was not found. - 'already-exists': Some document that we attempted to create already exists. - 'permission-denied': The caller does not have permission to execute the specified operation. - 'resource-exhausted': Some resource has been exhausted, perhaps a per-user quota, or perhaps the entire file system is out of space. - 'failed-precondition': Operation was rejected because the system is not in a state required for the operation's execution. - 'aborted': The operation was aborted, typically due to a concurrency issue like transaction aborts, etc. - 'out-of-range': Operation was attempted past the valid range. - 'unimplemented': Operation is not implemented or not supported/enabled. - 'internal': Internal errors. Means some invariants expected by underlying system has been broken. If you see one of these errors, something is very broken. - 'unavailable': The service is currently unavailable. This is most likely a transient condition and may be corrected by retrying with a backoff. - 'data-loss': Unrecoverable data loss or corruption. - 'unauthenticated': The request does not have valid authentication credentials for the operation. | | [FirestoreLocalCache](./firestore_.md#firestorelocalcache) | Union type from all supported SDK cache layer. | -| [ListenSource](./firestore_.md#listensource) | Describe the source a query listens to.default: listens to both cache and server changes cache: listens to changes in cache only | +| [ListenSource](./firestore_.md#listensource) | Describe the source a query listens to.Set to default to listen to both cache and server changes. Set to cache to listen to changes in cache only. | | [MemoryGarbageCollector](./firestore_.md#memorygarbagecollector) | Union type from all support gabage collectors for memory local cache. | | [NestedUpdateFields](./firestore_.md#nestedupdatefields) | For each field (e.g. 'bar'), find all nested keys (e.g. {'bar.baz': T1, 'bar.qux': T2}). Intersect them together to make a single map containing all possible keys that are all marked as optional | | [OrderByDirection](./firestore_.md#orderbydirection) | The direction of a [orderBy()](./firestore_.md#orderby_006d61f) clause is specified as 'desc' or 'asc' (descending or ascending). | @@ -2556,7 +2556,7 @@ export declare type FirestoreLocalCache = MemoryLocalCache | PersistentLocalCach Describe the source a query listens to. -`default`: listens to both cache and server changes `cache`: listens to changes in cache only +Set to `default` to listen to both cache and server changes. Set to `cache` to listen to changes in cache only. Signature: diff --git a/packages/firestore/src/api/reference_impl.ts b/packages/firestore/src/api/reference_impl.ts index 750fc07c5c6..79fb64b85cb 100644 --- a/packages/firestore/src/api/reference_impl.ts +++ b/packages/firestore/src/api/reference_impl.ts @@ -87,10 +87,11 @@ export interface SnapshotListenOptions { readonly source?: ListenSource; } -/** Describe the source a query listens to. +/** + * Describe the source a query listens to. * - * `default`: listens to both cache and server changes - * `cache`: listens to changes in cache only + * Set to `default` to listen to both cache and server changes. Set to `cache` + * to listen to changes in cache only. */ export type ListenSource = 'default' | 'cache'; From 0172b646102ccdc3e7f6ddc7730319e4798de55e Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Fri, 8 Mar 2024 14:25:21 -0500 Subject: [PATCH 32/33] format --- packages/firestore/src/api/reference_impl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firestore/src/api/reference_impl.ts b/packages/firestore/src/api/reference_impl.ts index 79fb64b85cb..e730fb40da7 100644 --- a/packages/firestore/src/api/reference_impl.ts +++ b/packages/firestore/src/api/reference_impl.ts @@ -87,7 +87,7 @@ export interface SnapshotListenOptions { readonly source?: ListenSource; } -/** +/** * Describe the source a query listens to. * * Set to `default` to listen to both cache and server changes. Set to `cache` From a340dd67317fbece152e4e23bc71c6c2393d5fa6 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Mon, 11 Mar 2024 14:36:06 -0400 Subject: [PATCH 33/33] update test --- .../api/snasphot_listener_source.test.ts | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts index cb765f52dd0..39a93d61912 100644 --- a/packages/firestore/test/integration/api/snasphot_listener_source.test.ts +++ b/packages/firestore/test/integration/api/snasphot_listener_source.test.ts @@ -17,7 +17,6 @@ import { expect } from 'chai'; -import { ListenerDataSource as Source } from '../../../src/core/event_manager'; import { EventsAccumulator } from '../util/events_accumulator'; import { addDoc, @@ -59,7 +58,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( coll, - { source: Source.Cache }, + { source: 'cache' }, storeEvent.storeEvent ); @@ -83,7 +82,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( docRef, - { source: Source.Cache }, + { source: 'cache' }, storeEvent.storeEvent ); @@ -107,7 +106,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( coll, - { includeMetadataChanges: true, source: Source.Cache }, + { includeMetadataChanges: true, source: 'cache' }, storeEvent.storeEvent ); @@ -139,13 +138,13 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe1 = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeEvent.storeEvent ); const unsubscribe2 = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeEvent.storeEvent ); @@ -207,7 +206,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeLimitEvent = new EventsAccumulator(); let limitUnlisten = onSnapshot( query(coll, orderBy('sort', 'asc'), limit(2)), - { source: Source.Cache }, + { source: 'cache' }, storeLimitEvent.storeEvent ); @@ -215,7 +214,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeLimitToLastEvent = new EventsAccumulator(); let limitToLastUnlisten = onSnapshot( query(coll, orderBy('sort', 'desc'), limitToLast(2)), - { source: Source.Cache }, + { source: 'cache' }, storeLimitToLastEvent.storeEvent ); @@ -235,7 +234,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { limitUnlisten(); limitUnlisten = onSnapshot( query(coll, orderBy('sort', 'asc'), limit(2)), - { source: Source.Cache }, + { source: 'cache' }, storeLimitEvent.storeEvent ); snapshot = await storeLimitEvent.awaitEvent(); @@ -269,7 +268,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { await updateDoc(doc(coll, 'a'), { k: 'a', sort: -2 }); limitToLastUnlisten = onSnapshot( query(coll, orderBy('sort', 'desc'), limitToLast(2)), - { source: Source.Cache }, + { source: 'cache' }, storeLimitToLastEvent.storeEvent ); @@ -320,7 +319,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent(); @@ -352,7 +351,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const cacheUnlisten = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeCacheEvent.storeEvent ); let snapshot = await storeCacheEvent.awaitEvent(); @@ -401,7 +400,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( testQuery, - { includeMetadataChanges: true, source: Source.Cache }, + { includeMetadataChanges: true, source: 'cache' }, storeEvent.storeEvent ); @@ -443,7 +442,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { includeMetadataChanges: true, source: Source.Cache }, + { includeMetadataChanges: true, source: 'cache' }, storeCacheEvent.storeEvent ); let snapshot = await storeCacheEvent.awaitEvent(); @@ -523,7 +522,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeCacheEvent.storeEvent ); await storeCacheEvent.awaitEvent(); @@ -568,7 +567,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeCacheEvent.storeEvent ); await storeCacheEvent.awaitEvent(); @@ -617,7 +616,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); let cacheUnlisten = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent(); @@ -651,7 +650,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { cacheUnlisten = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeCacheEvent.storeEvent ); @@ -679,7 +678,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeEvent = new EventsAccumulator(); const unsubscribe = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeEvent.storeEvent ); @@ -695,7 +694,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { expect(toDataArray(snapshot)).to.deep.equal([]); // Precondition check. const storeEvent = new EventsAccumulator(); - onSnapshot(coll, { source: Source.Cache }, storeEvent.storeEvent); + onSnapshot(coll, { source: 'cache' }, storeEvent.storeEvent); snapshot = await storeEvent.awaitEvent(); expect(snapshot.metadata.fromCache).to.be.true; expect(toDataArray(snapshot)).to.deep.equal([]); @@ -707,7 +706,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const accumulator = new EventsAccumulator(); const unsubscribe = onSnapshot( coll, - { source: Source.Cache }, + { source: 'cache' }, accumulator.storeEvent ); @@ -752,7 +751,7 @@ apiDescribe('Snapshot Listener source options ', persistence => { const storeCacheEvent = new EventsAccumulator(); const cacheUnlisten = onSnapshot( testQuery, - { source: Source.Cache }, + { source: 'cache' }, storeCacheEvent.storeEvent ); snapshot = await storeCacheEvent.awaitEvent();