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;
@@ -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);

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

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

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

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

@@ -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<firestore.QuerySnapshot>();
+      coll.onSnapshot(storeEvent.storeEvent);
+      const snapshot2 = await storeEvent.awaitEvent();
+
+      expect(snapshot2.metadata.fromCache).to.be.true;
+      expect(toDataArray(snapshot2)).to.deep.equal([]);
+    });
+  });
+
 });
 
 function verifyDocumentChange<T>(