Index-Free: Persist the lastLimboFreeSnapshot version

Author: schmidt-sebastian

packages/firestore/src/core/view_snapshot.ts

@@ -168,8 +168,8 @@ export class ViewSnapshot {
       changes,
       mutatedKeys,
       fromCache,
-      true,
-      false
+      /* syncStateChanged= */ true,
+      /* excludesMetadataChanges= */ false
     );
   }
 

packages/firestore/src/local/indexeddb_schema.ts

@@ -786,6 +786,12 @@ export class DbTarget {
      * listened to.
      */
     public lastListenSequenceNumber: number,
+    /**
+     * Denotes the maximum snapshot version at which the associated query view
+     * contained no limbo documents.  Undefined for data written prior to
+     * schema version 9.
+     */
+    public lastLimboFreeSnapshotVersion: DbTimestamp | undefined,
     /**
      * The query for this target.
      *

packages/firestore/src/local/local_serializer.ts

@@ -202,6 +202,13 @@ export class LocalSerializer {
   /** Decodes a DbTarget into QueryData */
   fromDbTarget(dbTarget: DbTarget): QueryData {
     const version = this.fromDbTimestamp(dbTarget.readTime);
+    const lastLimboFreeSnapshotVersion =
+      dbTarget.lastLimboFreeSnapshotVersion !== undefined
+        ? this.fromDbTimestamp(dbTarget.lastLimboFreeSnapshotVersion)
+        : SnapshotVersion.MIN;
+    // TODO(b/140573486): Convert to platform representation
+    const resumeToken = dbTarget.resumeToken;
+
     let query: Query;
     if (isDocumentQuery(dbTarget.query)) {
       query = this.remoteSerializer.fromDocumentsTarget(dbTarget.query);
@@ -214,7 +221,8 @@ export class LocalSerializer {
       QueryPurpose.Listen,
       dbTarget.lastListenSequenceNumber,
       version,
-      dbTarget.resumeToken
+      lastLimboFreeSnapshotVersion,
+      resumeToken
     );
   }
 
@@ -228,6 +236,9 @@ export class LocalSerializer {
         queryData.purpose
     );
     const dbTimestamp = this.toDbTimestamp(queryData.snapshotVersion);
+    const dbLastLimboFreeTimestamp = this.toDbTimestamp(
+      queryData.lastLimboFreeSnapshotVersion
+    );
     let queryProto: DbQuery;
     if (queryData.query.isDocumentQuery()) {
       queryProto = this.remoteSerializer.toDocumentsTarget(queryData.query);
@@ -255,6 +266,7 @@ export class LocalSerializer {
       dbTimestamp,
       resumeToken,
       queryData.sequenceNumber,
+      dbLastLimboFreeTimestamp,
       queryProto
     );
   }

packages/firestore/src/local/local_store.ts

@@ -658,6 +658,8 @@ export class LocalStore {
         return PersistencePromise.forEach(
           viewChanges,
           (viewChange: LocalViewChanges) => {
+            const targetId = viewChange.targetId;
+
             this.localViewReferences.addReferences(
               viewChange.addedKeys,
               viewChange.targetId
@@ -666,6 +668,21 @@ export class LocalStore {
               viewChange.removedKeys,
               viewChange.targetId
             );
+
+            if (viewChange.fromCache) {
+              const queryData = this.queryDataByTarget[targetId];
+              assert(
+                queryData !== undefined,
+                `Can't set limbo-free snapshot version for unknown target: ${targetId}`
+              );
+
+              // Advance the last limbo free snapshot version
+              const lastLimboFreeSnapshotVersion = queryData.snapshotVersion;
+              const updatedQueryData = queryData.withLastLimboFreeSnapshotVersion(
+                lastLimboFreeSnapshotVersion
+              );
+              this.queryDataByTarget[targetId] = updatedQueryData;
+            }
             return PersistencePromise.forEach(
               viewChange.removedKeys,
               (key: DocumentKey) =>

packages/firestore/src/local/local_view_changes.ts

@@ -27,6 +27,7 @@ import { documentKeySet, DocumentKeySet } from '../model/collections';
 export class LocalViewChanges {
   constructor(
     readonly targetId: TargetId,
+    readonly fromCache: boolean,
     readonly addedKeys: DocumentKeySet,
     readonly removedKeys: DocumentKeySet
   ) {}
@@ -51,6 +52,11 @@ export class LocalViewChanges {
       }
     }
 
-    return new LocalViewChanges(targetId, addedKeys, removedKeys);
+    return new LocalViewChanges(
+      targetId,
+      viewSnapshot.fromCache,
+      addedKeys,
+      removedKeys
+    );
   }
 }

packages/firestore/src/local/query_data.ts

@@ -48,10 +48,18 @@ export class QueryData {
     readonly targetId: TargetId,
     /** The purpose of the query. */
     readonly purpose: QueryPurpose,
-    /** The sequence number of the last transaction during which this query data was modified */
+    /**
+     * The sequence number of the last transaction during which this query data
+     * was modified.
+     */
     readonly sequenceNumber: ListenSequenceNumber,
     /** The latest snapshot version seen for this target. */
     readonly snapshotVersion: SnapshotVersion = SnapshotVersion.MIN,
+    /**
+     * The maximum snapshot version at which the associated query view
+     * contained no limbo documents.
+     */
+    readonly lastLimboFreeSnapshotVersion: SnapshotVersion = SnapshotVersion.MIN,
     /**
      * An opaque, server-assigned token that allows watching a query to be
      * resumed after disconnecting without retransmitting all the data that
@@ -69,6 +77,7 @@ export class QueryData {
       this.purpose,
       sequenceNumber,
       this.snapshotVersion,
+      this.lastLimboFreeSnapshotVersion,
       this.resumeToken
     );
   }
@@ -87,16 +96,38 @@ export class QueryData {
       this.purpose,
       this.sequenceNumber,
       snapshotVersion,
+      this.lastLimboFreeSnapshotVersion,
       resumeToken
     );
   }
 
+  /**
+   * Creates a new query data instance with an updated last limbo free
+   * snapshot version number.
+   */
+  withLastLimboFreeSnapshotVersion(
+    lastLimboFreeSnapshotVersion: SnapshotVersion
+  ): QueryData {
+    return new QueryData(
+      this.query,
+      this.targetId,
+      this.purpose,
+      this.sequenceNumber,
+      this.snapshotVersion,
+      lastLimboFreeSnapshotVersion,
+      this.resumeToken
+    );
+  }
+
   isEqual(other: QueryData): boolean {
     return (
       this.targetId === other.targetId &&
       this.purpose === other.purpose &&
       this.sequenceNumber === other.sequenceNumber &&
       this.snapshotVersion.isEqual(other.snapshotVersion) &&
+      this.lastLimboFreeSnapshotVersion.isEqual(
+        other.lastLimboFreeSnapshotVersion
+      ) &&
       this.resumeToken === other.resumeToken &&
       this.query.isEqual(other.query)
     );

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

@@ -290,6 +290,44 @@ describe('View', () => {
     );
   });
 
+  it('is marked from cache with limbo documents', () => {
+    const query = Query.atPath(path('rooms/eros/msgs'));
+    const view = new View(query, documentKeySet());
+    const doc1 = doc('rooms/eros/msgs/0', 0, {});
+    const doc2 = doc('rooms/eros/msgs/1', 0, {});
+
+    // Doc1 is contained in the local view, but we are not yet CURRENT so it is
+    // expected that the backend hasn't told us about all documents yet.
+    let changes = view.computeDocChanges(documentUpdates(doc1));
+    let viewChange = view.applyChanges(
+      changes,
+      /* updateLimboDocuments= */ true
+    );
+    expect(viewChange.snapshot!.fromCache).to.be.true;
+
+    // Add doc2 to generate a snapshot. Doc1 is still missing.
+    changes = view.computeDocChanges(documentUpdates(doc2));
+    viewChange = view.applyChanges(changes, /* updateLimboDocuments= */ true);
+    expect(viewChange.snapshot!.fromCache).to.be.true;
+
+    // Add doc2 to the backend's result set.
+    viewChange = view.applyChanges(
+      changes,
+      /* updateLimboDocuments= */ true,
+      updateMapping(version(0), [doc2], [], [], /* current= */ true)
+    );
+    // We are CURRENT but doc1 is in limbo.
+    expect(viewChange.snapshot!.fromCache).to.be.true;
+
+    // Add doc1 to the backend's result set.
+    viewChange = view.applyChanges(
+      changes,
+      /* updateLimboDocuments= */ true,
+      updateMapping(version(0), [doc1], [], [], /* current= */ true)
+    );
+    expect(viewChange.snapshot!.fromCache).to.be.false;
+  });
+
   it('resumes queries without creating limbo documents', () => {
     const query = Query.atPath(path('rooms/eros/msgs'));
     const doc1 = doc('rooms/eros/msgs/0', 0, {});

packages/firestore/test/unit/local/local_store.test.ts

@@ -844,13 +844,21 @@ function genericLocalStoreTests(
       .after(setMutation('foo/baz', { foo: 'baz' }))
       .toContain(doc('foo/bar', 1, { foo: 'bar' }))
       .toContain(doc('foo/baz', 0, { foo: 'baz' }, { hasLocalMutations: true }))
-      .after(localViewChanges(2, { added: ['foo/bar', 'foo/baz'] }))
+      .after(
+        localViewChanges(2, /* fromCache= */ false, {
+          added: ['foo/bar', 'foo/baz']
+        })
+      )
       .after(docUpdateRemoteEvent(doc('foo/bar', 1, { foo: 'bar' }), [], [2]))
       .after(docUpdateRemoteEvent(doc('foo/baz', 2, { foo: 'baz' }), [2]))
       .afterAcknowledgingMutation({ documentVersion: 2 })
       .toContain(doc('foo/bar', 1, { foo: 'bar' }))
       .toContain(doc('foo/baz', 2, { foo: 'baz' }))
-      .after(localViewChanges(2, { removed: ['foo/bar', 'foo/baz'] }))
+      .after(
+        localViewChanges(2, /* fromCache= */ false, {
+          removed: ['foo/bar', 'foo/baz']
+        })
+      )
       .afterReleasingQuery(query)
       .toNotContain('foo/bar')
       .toNotContain('foo/baz')

packages/firestore/test/unit/local/query_cache.test.ts

@@ -62,16 +62,20 @@ describe('IndexedDbQueryCache', () => {
     const originalSequenceNumber = 1234;
     const targetId = 5;
     const snapshotVersion = SnapshotVersion.fromTimestamp(new Timestamp(1, 2));
+    const lastLimboFreeSnapshotVersion = SnapshotVersion.fromTimestamp(
+      new Timestamp(3, 4)
+    );
     const query = Query.atPath(path('rooms'));
-    await queryCache1.addQueryData(
-      new QueryData(
-        query,
-        targetId,
-        QueryPurpose.Listen,
-        originalSequenceNumber,
-        snapshotVersion
-      )
+    const queryData = new QueryData(
+      query,
+      targetId,
+      QueryPurpose.Listen,
+      originalSequenceNumber,
+      snapshotVersion,
+      lastLimboFreeSnapshotVersion
     );
+
+    await queryCache1.addQueryData(queryData);
     // Snapshot version needs to be set separately
     await queryCache1.setTargetsMetadata(
       originalSequenceNumber,
@@ -86,6 +90,14 @@ describe('IndexedDbQueryCache', () => {
     expect(await queryCache2.getHighestSequenceNumber()).to.equal(
       originalSequenceNumber
     );
+    const actualQueryData = await queryCache2.getQueryData(query);
+
+    if (process.env.USE_MOCK_PERSISTENCE !== 'YES') {
+      // TODO(b/140573486): This fails on Node with persistence since the
+      // resume token is read back as a string.
+      expect(queryData.isEqual(actualQueryData!)).to.be.true;
+    }
+
     const actualSnapshotVersion = await queryCache2.getLastRemoteSnapshotVersion();
     expect(snapshotVersion.isEqual(actualSnapshotVersion)).to.be.true;
     await db2.shutdown();
@@ -127,6 +139,7 @@ function genericQueryCacheTests(
       QueryPurpose.Listen,
       ++previousSequenceNumber,
       snapshotVersion,
+      /* lastLimboFreeSnapshotVersion= */ SnapshotVersion.MIN,
       resumeToken
     );
   }

packages/firestore/test/unit/remote/node/serializer.test.ts

@@ -105,14 +105,7 @@ describe('Serializer', () => {
    * variations on Query.
    */
   function wrapQueryData(query: Query): QueryData {
-    return new QueryData(
-      query,
-      1,
-      QueryPurpose.Listen,
-      2,
-      SnapshotVersion.MIN,
-      emptyResumeToken
-    );
+    return new QueryData(query, 1, QueryPurpose.Listen, 2);
   }
 
   describe('converts value', () => {
@@ -1196,6 +1189,7 @@ describe('Serializer', () => {
           QueryPurpose.Listen,
           4,
           SnapshotVersion.MIN,
+          SnapshotVersion.MIN,
           new Uint8Array([1, 2, 3])
         )
       );

packages/firestore/test/unit/specs/spec_test_runner.ts

@@ -1072,6 +1072,7 @@ abstract class TestRunner {
           QueryPurpose.Listen,
           ARBITRARY_SEQUENCE_NUMBER,
           SnapshotVersion.MIN,
+          SnapshotVersion.MIN,
           expected.resumeToken
         )
       );

packages/firestore/test/util/helpers.ts

@@ -435,6 +435,7 @@ export function limboChanges(changes: {
 
 export function localViewChanges(
   targetId: TargetId,
+  fromCache: boolean,
   changes: { added?: string[]; removed?: string[] }
 ): LocalViewChanges {
   if (!changes.added) {
@@ -453,7 +454,7 @@ export function localViewChanges(
     keyStr => (removedKeys = removedKeys.add(key(keyStr)))
   );
 
-  return new LocalViewChanges(targetId, addedKeys, removedKeys);
+  return new LocalViewChanges(targetId, fromCache, addedKeys, removedKeys);
 }
 
 /** Creates a resume token to match the given snapshot version. */