Fix Firestore failing to return empty results from the local cache

.changeset/cool-grapes-attend.md

@@ -0,0 +1,5 @@
+---
+'@firebase/firestore': patch
+---
+
+Fix Firestore failing to raise initial snapshot from empty local cache result

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,13 @@ 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 any documents or resume token,
+    // or we are offline.
+    return (
+      !snap.docs.isEmpty() ||
+      snap.resumeToken.approximateByteSize() > 0 ||
+      onlineState === OnlineState.Offline
+    );
   }
 
   private shouldRaiseEvent(snap: ViewSnapshot): boolean {
@@ -405,7 +411,8 @@ export class QueryListener {
       snap.query,
       snap.docs,
       snap.mutatedKeys,
-      snap.fromCache
+      snap.fromCache,
+      snap.resumeToken
     );
     this.raisedInitialEvent = true;
     this.queryObserver.next(snap);

packages/firestore/src/core/sync_engine_impl.ts

@@ -64,6 +64,7 @@ import {
 import { debugAssert, debugCast, fail, hardAssert } from '../util/assert';
 import { wrapInUserErrorIfRecoverable } from '../util/async_queue';
 import { BundleReader } from '../util/bundle_reader';
+import { ByteString } from '../util/byte_string';
 import { Code, FirestoreError } from '../util/error';
 import { logDebug, logWarn } from '../util/log';
 import { primitiveComparator } from '../util/misc';
@@ -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<ViewSnapshot> {
   // 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,7 +363,8 @@ async function initializeViewAndComputeSnapshot(
   const synthesizedTargetChange =
     TargetChange.createSynthesizedTargetChangeForCurrentChange(
       targetId,
-      current && syncEngineImpl.onlineState !== OnlineState.Offline
+      current && syncEngineImpl.onlineState !== OnlineState.Offline,
+      resumeToken
     );
   const viewChange = view.applyChanges(
     viewDocChanges,
@@ -1385,7 +1389,8 @@ async function synchronizeQueryViewsAndRaiseSnapshots(
         syncEngineImpl,
         synthesizeTargetToQuery(target!),
         targetId,
-        /*current=*/ false
+        /*current=*/ false,
+        targetData.resumeToken
       );
     }
 
@@ -1457,7 +1462,8 @@ export async function syncEngineApplyTargetState(
         const synthesizedRemoteEvent =
           RemoteEvent.createSynthesizedRemoteEventForCurrentChange(
             targetId,
-            state === 'current'
+            state === 'current',
+            ByteString.EMPTY_BYTE_STRING
           );
         await syncEngineEmitNewSnapsAndNotifyLocalStore(
           syncEngineImpl,
@@ -1512,7 +1518,8 @@ export async function syncEngineApplyActiveTargetsChange(
       syncEngineImpl,
       synthesizeTargetToQuery(target),
       targetData.targetId,
-      /*current=*/ false
+      /*current=*/ false,
+      targetData.resumeToken
     );
     remoteStoreListen(syncEngineImpl.remoteStore, targetData);
   }

packages/firestore/src/core/view.ts

@@ -26,6 +26,7 @@ import { DocumentKey } from '../model/document_key';
 import { DocumentSet } from '../model/document_set';
 import { TargetChange } from '../remote/remote_event';
 import { debugAssert, fail } from '../util/assert';
+import { ByteString } from '../util/byte_string';
 
 import { LimitType, newQueryComparator, Query, queryMatches } from './query';
 import { OnlineState } from './types';
@@ -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
@@ -319,7 +321,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 +471,8 @@ export class View {
       this.query,
       this.documentSet,
       this.mutatedKeys,
-      this.syncState === SyncState.Local
+      this.syncState === SyncState.Local,
+      this.resumeToken ?? ByteString.EMPTY_BYTE_STRING
     );
   }
 }

packages/firestore/src/core/view_snapshot.ts

@@ -20,6 +20,7 @@ import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { DocumentSet } from '../model/document_set';
 import { fail } from '../util/assert';
+import { ByteString } from '../util/byte_string';
 import { SortedMap } from '../util/sorted_map';
 
 import { Query, queryEquals } from './query';
@@ -146,15 +147,17 @@ export class ViewSnapshot {
     readonly mutatedKeys: DocumentKeySet,
     readonly fromCache: boolean,
     readonly syncStateChanged: boolean,
-    readonly excludesMetadataChanges: boolean
+    readonly excludesMetadataChanges: boolean,
+    readonly resumeToken: ByteString
   ) {}
 
   /** 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 +172,8 @@ export class ViewSnapshot {
       mutatedKeys,
       fromCache,
       /* syncStateChanged= */ true,
-      /* excludesMetadataChanges= */ false
+      /* excludesMetadataChanges= */ false,
+      resumeToken
     );
   }
 

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<TargetId, TargetChange>();
     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(),

packages/firestore/test/integration/api/query.test.ts

@@ -1281,13 +1281,52 @@ apiDescribe('Queries', (persistence: boolean) => {
     };
 
     return withTestCollection(persistence, testDocs, async coll => {
-      await getDocs(query(coll)); // Populate the cache
+      await getDocs(query(coll)); // Populate the cache.
       const snapshot = await getDocs(
         query(coll, where('map.nested', '==', 'foo'))
       );
       expect(toDataArray(snapshot)).to.deep.equal([{ map: { nested: 'foo' } }]);
     });
   });
+
+  // Reproduces https://github.com/firebase/firebase-js-sdk/issues/5873
+  // eslint-disable-next-line no-restricted-properties
+  (persistence ? describe : describe.skip)('Caching empty results', () => {
+    it('can raise initial snapshot from cache, even if it is empty', () => {
+      return withTestCollection(persistence, {}, async coll => {
+        const snapshot1 = await getDocs(coll); // 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<QuerySnapshot>();
+        onSnapshot(coll, storeEvent.storeEvent);
+        const snapshot2 = await storeEvent.awaitEvent();
+        expect(snapshot2.metadata.fromCache).to.be.true;
+        expect(toDataArray(snapshot2)).to.deep.equal([]);
+      });
+    });
+
+    it('can raise initial snapshot from cache, even if it has become empty', () => {
+      const testDocs = {
+        a: { key: 'a' }
+      };
+      return withTestCollection(persistence, testDocs, async coll => {
+        // Populate the cache.
+        const snapshot1 = await getDocs(coll);
+        expect(snapshot1.metadata.fromCache).to.be.false;
+        expect(toDataArray(snapshot1)).to.deep.equal([{ key: 'a' }]);
+        // Empty the collection.
+        void deleteDoc(doc(coll, 'a'));
+
+        const storeEvent = new EventsAccumulator<QuerySnapshot>();
+        onSnapshot(coll, storeEvent.storeEvent);
+        const snapshot2 = await storeEvent.awaitEvent();
+        expect(snapshot2.metadata.fromCache).to.be.true;
+        expect(toDataArray(snapshot2)).to.deep.equal([]);
+      });
+    });
+  });
 });
 
 function verifyDocumentChange<T>(

packages/firestore/test/unit/api/database.test.ts

@@ -25,6 +25,8 @@ import {
 } from '../../../src';
 import { EmulatorAuthCredentialsProvider } from '../../../src/api/credentials';
 import { User } from '../../../src/auth/user';
+import { encodeBase64 } from '../../../src/platform/base64';
+import { ByteString } from '../../../src/util/byte_string';
 import {
   collectionReference,
   documentReference,
@@ -179,6 +181,48 @@ describe('QuerySnapshot', () => {
     ).to.be.false;
   });
 
+  it('resume token should not effect querySnapshot equality', () => {
+    const resumeToken1 = ByteString.fromBase64String(
+      encodeBase64('ResumeToken1')
+    );
+    const resumeToken1Copy = ByteString.fromBase64String(
+      encodeBase64('ResumeToken1')
+    );
+    const resumeToken2 = ByteString.fromBase64String(
+      encodeBase64('ResumeToken2')
+    );
+
+    const snapshot1 = querySnapshot(
+      'foo',
+      {},
+      { a: { a: 1 } },
+      keys(),
+      false,
+      false,
+      resumeToken1
+    );
+    const snapshot1Copy = querySnapshot(
+      'foo',
+      {},
+      { a: { a: 1 } },
+      keys(),
+      false,
+      false,
+      resumeToken1Copy
+    );
+    const snapshot2 = querySnapshot(
+      'foo',
+      {},
+      { a: { a: 1 } },
+      keys(),
+      false,
+      false,
+      resumeToken2
+    );
+    expect(snapshotEqual(snapshot1, snapshot1Copy)).to.be.true;
+    expect(snapshotEqual(snapshot1, snapshot2)).to.be.true;
+  });
+
   it('JSON.stringify() does not throw', () => {
     JSON.stringify(
       querySnapshot('foo', {}, { a: { a: 1 } }, keys(), false, false)

packages/firestore/test/unit/core/event_manager.test.ts

@@ -224,7 +224,8 @@ describe('QueryListener', () => {
       docChanges: [change1, change4],
       fromCache: snap2.fromCache,
       syncStateChanged: true,
-      mutatedKeys: keys()
+      mutatedKeys: keys(),
+      resumeToken: snap2.resumeToken
     };
     expect(otherEvents).to.deep.equal([expectedSnap2]);
   });
@@ -396,7 +397,8 @@ describe('QueryListener', () => {
         docChanges: [change3],
         fromCache: snap2.fromCache,
         syncStateChanged: snap2.syncStateChanged,
-        mutatedKeys: snap2.mutatedKeys
+        mutatedKeys: snap2.mutatedKeys,
+        resumeToken: snap2.resumeToken
       };
       expect(filteredEvents).to.deep.equal([snap1, expectedSnap2]);
     }
@@ -482,7 +484,8 @@ describe('QueryListener', () => {
       ],
       fromCache: false,
       syncStateChanged: true,
-      mutatedKeys: keys()
+      mutatedKeys: keys(),
+      resumeToken: snap3.resumeToken
     };
     expect(events).to.deep.equal([expectedSnap]);
   });
@@ -517,7 +520,8 @@ describe('QueryListener', () => {
       docChanges: [{ type: ChangeType.Added, doc: doc1 }],
       fromCache: true,
       syncStateChanged: true,
-      mutatedKeys: keys()
+      mutatedKeys: keys(),
+      resumeToken: snap1.resumeToken
     };
     const expectedSnap2 = {
       query: query1,
@@ -526,7 +530,8 @@ describe('QueryListener', () => {
       docChanges: [{ type: ChangeType.Added, doc: doc2 }],
       fromCache: true,
       syncStateChanged: false,
-      mutatedKeys: keys()
+      mutatedKeys: keys(),
+      resumeToken: snap2.resumeToken
     };
     expect(events).to.deep.equal([expectedSnap1, expectedSnap2]);
   });
@@ -552,7 +557,8 @@ describe('QueryListener', () => {
       docChanges: [],
       fromCache: true,
       syncStateChanged: true,
-      mutatedKeys: keys()
+      mutatedKeys: keys(),
+      resumeToken: snap1.resumeToken
     };
     expect(events).to.deep.equal([expectedSnap]);
   });
@@ -577,7 +583,8 @@ describe('QueryListener', () => {
       docChanges: [],
       fromCache: true,
       syncStateChanged: true,
-      mutatedKeys: keys()
+      mutatedKeys: keys(),
+      resumeToken: snap1.resumeToken
     };
     expect(events).to.deep.equal([expectedSnap]);
   });