diff --git a/packages/firestore/src/core/event_manager.ts b/packages/firestore/src/core/event_manager.ts index ecce1109150..376d25b4aec 100644 --- a/packages/firestore/src/core/event_manager.ts +++ b/packages/firestore/src/core/event_manager.ts @@ -307,7 +307,8 @@ export class QueryListener { snap.mutatedKeys, snap.fromCache, snap.syncStateChanged, - /* excludesMetadataChanges= */ true + /* excludesMetadataChanges= */ true, + snap.resumeToken ); } let raisedEvent = false; @@ -371,8 +372,9 @@ export class QueryListener { return false; } - // Raise data from cache if we have any documents or we are offline - return !snap.docs.isEmpty() || onlineState === OnlineState.Offline; + // Raise data from cache if we have previously executed the query and have + // cached data or we are offline + return snap.resumeToken.approximateByteSize() > 0 || onlineState === OnlineState.Offline; } private shouldRaiseEvent(snap: ViewSnapshot): boolean { @@ -405,7 +407,8 @@ export class QueryListener { snap.query, snap.docs, snap.mutatedKeys, - snap.fromCache + snap.fromCache, + snap.resumeToken ); this.raisedInitialEvent = true; this.queryObserver.next(snap); diff --git a/packages/firestore/src/core/sync_engine_impl.ts b/packages/firestore/src/core/sync_engine_impl.ts index 339865549a7..2c4d7558686 100644 --- a/packages/firestore/src/core/sync_engine_impl.ts +++ b/packages/firestore/src/core/sync_engine_impl.ts @@ -115,6 +115,7 @@ import { ViewChange } from './view'; import { ViewSnapshot } from './view_snapshot'; +import {ByteString} from '../util/byte_string'; const LOG_TAG = 'SyncEngine'; @@ -327,7 +328,8 @@ export async function syncEngineListen( syncEngineImpl, query, targetId, - status === 'current' + status === 'current', + targetData.resumeToken ); } @@ -342,7 +344,8 @@ async function initializeViewAndComputeSnapshot( syncEngineImpl: SyncEngineImpl, query: Query, targetId: TargetId, - current: boolean + current: boolean, + resumeToken: ByteString ): Promise { // PORTING NOTE: On Web only, we inject the code that registers new Limbo // targets based on view changes. This allows us to only depend on Limbo @@ -360,12 +363,13 @@ async function initializeViewAndComputeSnapshot( const synthesizedTargetChange = TargetChange.createSynthesizedTargetChangeForCurrentChange( targetId, - current && syncEngineImpl.onlineState !== OnlineState.Offline + current && syncEngineImpl.onlineState !== OnlineState.Offline, + resumeToken ); const viewChange = view.applyChanges( viewDocChanges, /* updateLimboDocuments= */ syncEngineImpl.isPrimaryClient, - synthesizedTargetChange + synthesizedTargetChange, ); updateTrackedLimbos(syncEngineImpl, targetId, viewChange.limboChanges); @@ -1025,7 +1029,7 @@ export async function syncEngineEmitNewSnapsAndNotifyLocalStore( if (syncEngineImpl.isPrimaryClient) { syncEngineImpl.sharedClientState.updateQueryState( queryView.targetId, - viewSnapshot.fromCache ? 'not-current' : 'current' + viewSnapshot.fromCache ? 'not-current' : 'current', ); } newSnaps.push(viewSnapshot); @@ -1379,7 +1383,8 @@ async function synchronizeQueryViewsAndRaiseSnapshots( syncEngineImpl, synthesizeTargetToQuery(target!), targetId, - /*current=*/ false + /*current=*/ false, + targetData.resumeToken ); } @@ -1451,7 +1456,8 @@ export async function syncEngineApplyTargetState( const synthesizedRemoteEvent = RemoteEvent.createSynthesizedRemoteEventForCurrentChange( targetId, - state === 'current' + state === 'current', + ByteString.EMPTY_BYTE_STRING ); await syncEngineEmitNewSnapsAndNotifyLocalStore( syncEngineImpl, @@ -1506,7 +1512,8 @@ export async function syncEngineApplyActiveTargetsChange( syncEngineImpl, synthesizeTargetToQuery(target), targetData.targetId, - /*current=*/ false + /*current=*/ false, + targetData.resumeToken ); remoteStoreListen(syncEngineImpl.remoteStore, targetData); } diff --git a/packages/firestore/src/core/view.ts b/packages/firestore/src/core/view.ts index 21ab049b9af..49cf7461cf0 100644 --- a/packages/firestore/src/core/view.ts +++ b/packages/firestore/src/core/view.ts @@ -35,6 +35,7 @@ import { SyncState, ViewSnapshot } from './view_snapshot'; +import {ByteString} from '../util/byte_string'; export type LimboDocumentChange = AddedLimboDocument | RemovedLimboDocument; export class AddedLimboDocument { @@ -72,6 +73,7 @@ export interface ViewChange { */ export class View { private syncState: SyncState | null = null; + private resumeToken: ByteString | null = null; /** * A flag whether the view is current with the backend. A view is considered * current after it has seen the current flag from the backend and did not @@ -306,11 +308,13 @@ export class View { const newSyncState = synced ? SyncState.Synced : SyncState.Local; const syncStateChanged = newSyncState !== this.syncState; this.syncState = newSyncState; + this.resumeToken = targetChange?.resumeToken ?? null; if (changes.length === 0 && !syncStateChanged) { // no changes return { limboChanges }; } else { + const snap: ViewSnapshot = new ViewSnapshot( this.query, docChanges.documentSet, @@ -319,7 +323,8 @@ export class View { docChanges.mutatedKeys, newSyncState === SyncState.Local, syncStateChanged, - /* excludesMetadataChanges= */ false + /* excludesMetadataChanges= */ false, + targetChange?.resumeToken ?? ByteString.EMPTY_BYTE_STRING ); return { snapshot: snap, @@ -468,7 +473,8 @@ export class View { this.query, this.documentSet, this.mutatedKeys, - this.syncState === SyncState.Local + this.syncState === SyncState.Local, + this.resumeToken ?? ByteString.EMPTY_BYTE_STRING ); } } diff --git a/packages/firestore/src/core/view_snapshot.ts b/packages/firestore/src/core/view_snapshot.ts index e631a3dcac0..02c400e2bbc 100644 --- a/packages/firestore/src/core/view_snapshot.ts +++ b/packages/firestore/src/core/view_snapshot.ts @@ -23,6 +23,7 @@ import { fail } from '../util/assert'; import { SortedMap } from '../util/sorted_map'; import { Query, queryEquals } from './query'; +import {ByteString} from '../util/byte_string'; export const enum ChangeType { Added, @@ -146,15 +147,19 @@ export class ViewSnapshot { readonly mutatedKeys: DocumentKeySet, readonly fromCache: boolean, readonly syncStateChanged: boolean, - readonly excludesMetadataChanges: boolean - ) {} + readonly excludesMetadataChanges: boolean, + readonly resumeToken: ByteString + ) { + 1 == 1; + } /** Returns a view snapshot as if all documents in the snapshot were added. */ static fromInitialDocuments( query: Query, documents: DocumentSet, mutatedKeys: DocumentKeySet, - fromCache: boolean + fromCache: boolean, + resumeToken: ByteString ): ViewSnapshot { const changes: DocumentViewChange[] = []; documents.forEach(doc => { @@ -169,7 +174,8 @@ export class ViewSnapshot { mutatedKeys, fromCache, /* syncStateChanged= */ true, - /* excludesMetadataChanges= */ false + /* excludesMetadataChanges= */ false, + resumeToken ); } @@ -184,7 +190,8 @@ export class ViewSnapshot { !this.mutatedKeys.isEqual(other.mutatedKeys) || !queryEquals(this.query, other.query) || !this.docs.isEqual(other.docs) || - !this.oldDocs.isEqual(other.oldDocs) + !this.oldDocs.isEqual(other.oldDocs) || + this.resumeToken !== other.resumeToken ) { return false; } diff --git a/packages/firestore/src/remote/remote_event.ts b/packages/firestore/src/remote/remote_event.ts index e35e1dbae12..5d48d32bb3f 100644 --- a/packages/firestore/src/remote/remote_event.ts +++ b/packages/firestore/src/remote/remote_event.ts @@ -67,14 +67,16 @@ export class RemoteEvent { // PORTING NOTE: Multi-tab only static createSynthesizedRemoteEventForCurrentChange( targetId: TargetId, - current: boolean + current: boolean, + resumeToken: ByteString ): RemoteEvent { const targetChanges = new Map(); targetChanges.set( targetId, TargetChange.createSynthesizedTargetChangeForCurrentChange( targetId, - current + current, + resumeToken ) ); return new RemoteEvent( @@ -134,10 +136,11 @@ export class TargetChange { */ static createSynthesizedTargetChangeForCurrentChange( targetId: TargetId, - current: boolean + current: boolean, + resumeToken: ByteString ): TargetChange { return new TargetChange( - ByteString.EMPTY_BYTE_STRING, + resumeToken, current, documentKeySet(), documentKeySet(), diff --git a/packages/firestore/test/integration/api/query.test.ts b/packages/firestore/test/integration/api/query.test.ts index 66397ed179c..20ab2162260 100644 --- a/packages/firestore/test/integration/api/query.test.ts +++ b/packages/firestore/test/integration/api/query.test.ts @@ -1288,6 +1288,25 @@ apiDescribe('Queries', (persistence: boolean) => { expect(toDataArray(snapshot)).to.deep.equal([{ map: { nested: 'foo' } }]); }); }); + + // eslint-disable-next-line no-restricted-properties + (persistence ? it : it.skip)('empty query results are cached', () => { + // Reproduces https://github.com/firebase/firebase-js-sdk/issues/5873 + return withTestCollection(persistence, {}, async coll => { + const snapshot1 = await coll.get(); // Populate the cache + expect(snapshot1.metadata.fromCache).to.be.false; + expect(toDataArray(snapshot1)).to.deep.equal([]); // Precondition check + + // Add a snapshot listener whose first event should be raised from cache. + const storeEvent = new EventsAccumulator(); + coll.onSnapshot(storeEvent.storeEvent); + const snapshot2 = await storeEvent.awaitEvent(); + + expect(snapshot2.metadata.fromCache).to.be.true; + expect(toDataArray(snapshot2)).to.deep.equal([]); + }); + }); + }); function verifyDocumentChange( diff --git a/packages/firestore/test/unit/api/database.test.ts b/packages/firestore/test/unit/api/database.test.ts index 71b798282f5..d0a4cd12c5a 100644 --- a/packages/firestore/test/unit/api/database.test.ts +++ b/packages/firestore/test/unit/api/database.test.ts @@ -33,7 +33,9 @@ import { query, querySnapshot } from '../../util/api_helpers'; -import { keys } from '../../util/helpers'; +import { expectEqual, expectNotEqual, keys } from '../../util/helpers'; +import {ByteString} from '../../../src/util/byte_string'; +import {encodeBase64} from '../../../src/platform/base64'; describe('CollectionReference', () => { it('support equality checking with isEqual()', () => { @@ -122,66 +124,64 @@ describe('Query', () => { describe('QuerySnapshot', () => { it('support equality checking with isEqual()', () => { - expect( - snapshotEqual( - querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false), - querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false) - ) - ).to.be.true; - expect( - snapshotEqual( - querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false), - querySnapshot('bar', {}, { a: { a: 1 } }, keys(), false, false) - ) - ).to.be.false; - expect( - snapshotEqual( - querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false), - querySnapshot( - 'foo', - { b: { b: 1 } }, - { a: { a: 1 } }, - keys(), - false, - false - ) - ) - ).to.be.false; - expect( - snapshotEqual( - querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false), - querySnapshot('foo', {}, { a: { b: 1 } }, keys(), false, false) - ) - ).to.be.false; - expect( - snapshotEqual( - querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false), - querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false) - ) - ).to.be.false; - expect( - snapshotEqual( - querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false), - querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/b'), false, false) - ) - ).to.be.false; - expect( - snapshotEqual( - querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false), - querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), true, false) - ) - ).to.be.false; - expect( - snapshotEqual( - querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false), - querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, true) + const resumeToken1 = ByteString.fromBase64String(encodeBase64("ResumeToken1")); + const resumeToken1Copy = ByteString.fromBase64String(encodeBase64("ResumeToken1")); + const resumeToken2 = ByteString.fromBase64String(encodeBase64("ResumeToken2")); + + expectEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false, resumeToken1), + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false, resumeToken1) + ); + expectEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false, resumeToken1), + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false, resumeToken1Copy) + ); + expectNotEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false, resumeToken1), + querySnapshot('bar', {}, { a: { a: 1 } }, keys(), false, false, resumeToken1) + ); + expectNotEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false, resumeToken1), + querySnapshot( + 'foo', + { b: { b: 1 } }, + { a: { a: 1 } }, + keys(), + false, + false, + resumeToken1 ) - ).to.be.false; + ); + expectNotEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false, resumeToken1), + querySnapshot('foo', {}, { a: { b: 1 } }, keys(), false, false, resumeToken1) + ); + expectNotEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false, resumeToken1), + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false, resumeToken1) + ); + expectNotEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false, resumeToken1), + querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/b'), false, false, resumeToken1) + ); + expectNotEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false, resumeToken1), + querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), true, false, resumeToken1) + ); + expectNotEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false, resumeToken1), + querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, true, resumeToken1) + ); + expectNotEqual( + querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false, resumeToken1), + querySnapshot('foo', {}, { a: { a: 1 } }, keys('foo/a'), false, false, resumeToken2), + ); }); it('JSON.stringify() does not throw', () => { + const resumeToken = ByteString.fromBase64String(encodeBase64("ResumeToken")); JSON.stringify( - querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false) + querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false, resumeToken) ); }); }); @@ -197,25 +197,23 @@ describe('SnapshotMetadata', () => { }); it('from QuerySnapshot support equality checking with isEqual()', () => { - expect( - querySnapshot('foo', {}, {}, keys('foo/a'), true, false).metadata - ).to.deep.equal( - querySnapshot('foo', {}, {}, keys('foo/a'), true, false).metadata + const resumeToken = ByteString.fromBase64String(encodeBase64("ResumeToken")); + + expectEqual( + querySnapshot('foo', {}, {}, keys('foo/a'), true, false, resumeToken).metadata, + querySnapshot('foo', {}, {}, keys('foo/a'), true, false, resumeToken).metadata ); - expect( - querySnapshot('foo', {}, {}, keys('foo/a'), true, false).metadata - ).to.not.deep.equal( - querySnapshot('foo', {}, {}, keys(), true, false).metadata + expectNotEqual( + querySnapshot('foo', {}, {}, keys('foo/a'), true, false, resumeToken).metadata, + querySnapshot('foo', {}, {}, keys(), true, false, resumeToken).metadata ); - expect( - querySnapshot('foo', {}, {}, keys('foo/a'), true, false).metadata - ).to.not.deep.equal( - querySnapshot('foo', {}, {}, keys('foo/a'), false, false).metadata + expectNotEqual( + querySnapshot('foo', {}, {}, keys('foo/a'), true, false, resumeToken).metadata, + querySnapshot('foo', {}, {}, keys('foo/a'), false, false, resumeToken).metadata ); - expect( - querySnapshot('foo', {}, {}, keys('foo/a'), true, false).metadata - ).to.not.deep.equal( - querySnapshot('foo', {}, {}, keys(), false, false).metadata + expectNotEqual( + querySnapshot('foo', {}, {}, keys('foo/a'), true, false, resumeToken).metadata, + querySnapshot('foo', {}, {}, keys(), false, false, resumeToken).metadata ); }); }); diff --git a/packages/firestore/test/util/api_helpers.ts b/packages/firestore/test/util/api_helpers.ts index 81d01132087..68621f33b60 100644 --- a/packages/firestore/test/util/api_helpers.ts +++ b/packages/firestore/test/util/api_helpers.ts @@ -46,6 +46,7 @@ import { JsonObject } from '../../src/model/object_value'; import { TEST_PROJECT } from '../unit/local/persistence_test_helpers'; import { doc, key, path as pathFrom } from './helpers'; +import {ByteString} from '../../src/util/byte_string'; /** * A mock Firestore. Will not work for integration test. @@ -122,6 +123,7 @@ export function query(path: string): Query { * @param mutatedKeys - The list of document with pending writes. * @param fromCache - Whether the query snapshot is cache result. * @param syncStateChanged - Whether the sync state has changed. + * @param resumeToken - The resume token associated with the query. * @returns A query snapshot that consists of both sets of documents. */ export function querySnapshot( @@ -130,7 +132,8 @@ export function querySnapshot( docsToAdd: { [key: string]: JsonObject }, mutatedKeys: DocumentKeySet, fromCache: boolean, - syncStateChanged: boolean + syncStateChanged: boolean, + resumeToken: ByteString ): QuerySnapshot { const query: InternalQuery = newQueryForPath(pathFrom(path)); let oldDocuments: DocumentSet = new DocumentSet(); @@ -152,7 +155,8 @@ export function querySnapshot( mutatedKeys, fromCache, syncStateChanged, - false + false, + resumeToken ); const db = firestore(); return new QuerySnapshot(