Fix Firestore failing to return empty results from the local cache

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;
@@ -372,7 +373,11 @@ export class QueryListener {
     }
 
     // Raise data from cache if we have any documents or we are offline
-    return !snap.docs.isEmpty() || onlineState === OnlineState.Offline;
+    return (
+      !snap.docs.isEmpty() ||
+      snap.resumeToken.approximateByteSize() > 0 ||
+      onlineState === OnlineState.Offline
+    );
   }
 
   private shouldRaiseEvent(snap: ViewSnapshot): boolean {
@@ -405,7 +410,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
     );
   }
 
@@ -184,7 +188,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;
     }

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 ?? ByteString.EMPTY_BYTE_STRING,
       current,
       documentKeySet(),
       documentKeySet(),

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

@@ -1288,6 +1288,93 @@ apiDescribe('Queries', (persistence: boolean) => {
       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 cache empty query results', () => {
+      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 empty cached collection and raise snapshot from it', () => {
+      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'));
+
+        // 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 snapshot from a cached collection which was emptied offline', () => {
+      const testDocs = {
+        a: { key: 'a' }
+      };
+      return withTestCollection(
+        persistence,
+        testDocs,
+        async (coll, firestore) => {
+          await getDocs(coll); // Populate the cache
+          const storeEvent = new EventsAccumulator<QuerySnapshot>();
+          onSnapshot(coll, storeEvent.storeEvent);
+          await storeEvent.awaitEvent();
+
+          await disableNetwork(firestore);
+          void deleteDoc(doc(coll, 'a'));
+          await enableNetwork(firestore);
+
+          const snapshot = await storeEvent.awaitEvent();
+          expect(snapshot.metadata.fromCache).to.be.true;
+          expect(toDataArray(snapshot)).to.deep.equal([]);
+        }
+      );
+    });
+
+    it('can register a listener and empty cache offline, and raise snaoshot from it when came back online', () => {
+      const testDocs = {
+        a: { key: 'a' }
+      };
+      return withTestCollection(
+        persistence,
+        testDocs,
+        async (coll, firestore) => {
+          await getDocs(coll); // Populate the cache
+          await disableNetwork(firestore);
+          const storeEvent = new EventsAccumulator<QuerySnapshot>();
+          onSnapshot(coll, storeEvent.storeEvent);
+          await storeEvent.awaitEvent();
+          void deleteDoc(doc(coll, 'a'));
+          await enableNetwork(firestore);
+
+          const snapshot = await storeEvent.awaitEvent();
+          expect(snapshot.metadata.fromCache).to.be.true;
+          expect(toDataArray(snapshot)).to.deep.equal([]);
+        }
+      );
+    });
+  });
 });
 
 function verifyDocumentChange<T>(

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]);
   });