resolve comments

Author: milaGGL

packages/firestore/src/core/event_manager.ts

@@ -372,7 +372,7 @@ export class QueryListener {
       return false;
     }
 
-    // Raise data from cache if we have any documents or we are 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 ||

packages/firestore/src/core/view_snapshot.ts

@@ -188,8 +188,7 @@ export class ViewSnapshot {
       !this.mutatedKeys.isEqual(other.mutatedKeys) ||
       !queryEquals(this.query, other.query) ||
       !this.docs.isEqual(other.docs) ||
-      !this.oldDocs.isEqual(other.oldDocs) ||
-      this.resumeToken !== other.resumeToken
+      !this.oldDocs.isEqual(other.oldDocs)
     ) {
       return false;
     }

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

@@ -1281,7 +1281,7 @@ 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'))
       );
@@ -1291,12 +1291,12 @@ apiDescribe('Queries', (persistence: boolean) => {
 
   // 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', () => {
+  (persistence ? describe : describe.skip)('Caching empty results', () => {
+    it('can raises initial snapshot from cache, even if it is empty', () => {
       return withTestCollection(persistence, {}, async coll => {
-        const snapshot1 = await getDocs(coll); // Populate the cache
+        const snapshot1 = await getDocs(coll); // Populate the cache.
         expect(snapshot1.metadata.fromCache).to.be.false;
-        expect(toDataArray(snapshot1)).to.deep.equal([]); // Precondition check
+        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>();
@@ -1307,73 +1307,25 @@ apiDescribe('Queries', (persistence: boolean) => {
       });
     });
 
-    it('can empty cached collection and raise snapshot from it', () => {
+    it('can raises initial snapshot from cache, even if it has become empty', () => {
       const testDocs = {
         a: { key: 'a' }
       };
       return withTestCollection(persistence, testDocs, async coll => {
-        // Populate the cache
+        // 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
+        // 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([]);
-        }
-      );
-    });
   });
 });
 

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/util/api_helpers.ts

@@ -131,7 +131,8 @@ export function querySnapshot(
   docsToAdd: { [key: string]: JsonObject<unknown> },
   mutatedKeys: DocumentKeySet,
   fromCache: boolean,
-  syncStateChanged: boolean
+  syncStateChanged: boolean,
+  resumeToken?: ByteString
 ): QuerySnapshot {
   const query: InternalQuery = newQueryForPath(pathFrom(path));
   let oldDocuments: DocumentSet = new DocumentSet();
@@ -145,7 +146,6 @@ export function querySnapshot(
     newDocuments = newDocuments.add(docToAdd);
     documentChanges.push({ type: ChangeType.Added, doc: docToAdd });
   });
-  const resumeToken = ByteString.EMPTY_BYTE_STRING;
   const viewSnapshot: ViewSnapshot = new ViewSnapshot(
     query,
     newDocuments,
@@ -155,7 +155,7 @@ export function querySnapshot(
     fromCache,
     syncStateChanged,
     false,
-    resumeToken
+    resumeToken ?? ByteString.EMPTY_BYTE_STRING
   );
   const db = firestore();
   return new QuerySnapshot(