Adding CommitVersion to RemoteDocumentCache

Author: schmidt-sebastian

packages/firestore/src/core/transaction.ts

@@ -37,7 +37,7 @@ export class Transaction {
   constructor(private datastore: Datastore) {}
 
   private recordVersion(doc: MaybeDocument): void {
-    let docVersion = doc.version;
+    let docVersion = doc.remoteVersion;
     if (doc instanceof NoDocument) {
       // For deleted docs, we must use baseVersion 0 when we overwrite them.
       docVersion = SnapshotVersion.forDeletedDoc();

packages/firestore/src/local/indexeddb_schema.ts

@@ -446,7 +446,14 @@ export class DbRemoteDocument {
      * Set to an instance of a Document if there's a cached version of the
      * document.
      */
-    public document: api.Document | null
+    public document: api.Document | null,
+    /**
+     * For documents that were written to the remote document store based on
+     * a write acknowledgment, we store the commit version of the mutation. This
+     * is used to determine if the document is 'dirty' (the commit version is
+     * higher than `document.updateTime`).
+     */
+    public commitVersion?: DbTimestamp
   ) {}
 }
 

packages/firestore/src/local/local_serializer.ts

@@ -43,12 +43,12 @@ export class LocalSerializer {
   /** Decodes a remote document from storage locally to a Document. */
   fromDbRemoteDocument(remoteDoc: DbRemoteDocument): MaybeDocument {
     if (remoteDoc.document) {
-      return this.remoteSerializer.fromDocument(remoteDoc.document);
+      const commitTime = this.fromDbTimestamp(remoteDoc.commitVersion);
+      return this.remoteSerializer.fromDocument(remoteDoc.document, commitTime);
     } else if (remoteDoc.noDocument) {
       const key = DocumentKey.fromSegments(remoteDoc.noDocument.path);
-      const readTime = remoteDoc.noDocument.readTime;
-      const timestamp = new Timestamp(readTime.seconds, readTime.nanoseconds);
-      return new NoDocument(key, SnapshotVersion.fromTimestamp(timestamp));
+      const version = this.fromDbTimestamp(remoteDoc.noDocument.readTime);
+      return new NoDocument(key, version);
     } else {
       return fail('Unexpected DbRemoteDocument');
     }
@@ -57,19 +57,33 @@ export class LocalSerializer {
   /** Encodes a document for storage locally. */
   toDbRemoteDocument(maybeDoc: MaybeDocument): DbRemoteDocument {
     if (maybeDoc instanceof Document) {
+      const commitVersion = this.toDbTimestamp(maybeDoc.commitVersion);
       const doc = this.remoteSerializer.toDocument(maybeDoc);
-      return new DbRemoteDocument(null, doc);
+      return new DbRemoteDocument(null, doc, commitVersion);
     } else {
       const path = maybeDoc.key.path.toArray();
-      const timestamp = maybeDoc.version.toTimestamp();
-      const readTime = new DbTimestamp(
-        timestamp.seconds,
-        timestamp.nanoseconds
-      );
+      const readTime = this.toDbTimestamp(maybeDoc.remoteVersion);
       return new DbRemoteDocument(new DbNoDocument(path, readTime), null);
     }
   }
 
+  private toDbTimestamp(snapshotVersion: SnapshotVersion): DbTimestamp {
+    const timestamp = snapshotVersion.toTimestamp();
+    return new DbTimestamp(timestamp.seconds, timestamp.nanoseconds);
+  }
+
+  private fromDbTimestamp(dbTimestamp?: DbTimestamp): SnapshotVersion {
+    if (dbTimestamp) {
+      const timestamp = new Timestamp(
+        dbTimestamp.seconds,
+        dbTimestamp.nanoseconds
+      );
+      return SnapshotVersion.fromTimestamp(timestamp);
+    } else {
+      return SnapshotVersion.MIN;
+    }
+  }
+
   /** Encodes a batch of mutations into a DbMutationBatch for local storage. */
   toDbMutationBatch(userId: string, batch: MutationBatch): DbMutationBatch {
     const serializedMutations = batch.mutations.map(m =>
@@ -118,11 +132,7 @@ export class LocalSerializer {
 
   /** Decodes a DbTarget into QueryData */
   fromDbTarget(dbTarget: DbTarget): QueryData {
-    const readTime = new Timestamp(
-      dbTarget.readTime.seconds,
-      dbTarget.readTime.nanoseconds
-    );
-    const version = SnapshotVersion.fromTimestamp(readTime);
+    const version = this.fromDbTimestamp(dbTarget.readTime);
     let query: Query;
     if (isDocumentQuery(dbTarget.query)) {
       query = this.remoteSerializer.fromDocumentsTarget(dbTarget.query);
@@ -147,11 +157,7 @@ export class LocalSerializer {
         ' may be stored, got ' +
         queryData.purpose
     );
-    const timestamp = queryData.snapshotVersion.toTimestamp();
-    const dbTimestamp = new DbTimestamp(
-      timestamp.seconds,
-      timestamp.nanoseconds
-    );
+    const dbTimestamp = this.toDbTimestamp(queryData.snapshotVersion);
     let queryProto: DbQuery;
     if (queryData.query.isDocumentQuery()) {
       queryProto = this.remoteSerializer.toDocumentsTarget(queryData.query);

packages/firestore/src/local/local_store.ts

@@ -563,9 +563,9 @@ export class LocalStore {
             // resolution failing).
             if (
               existingDoc == null ||
-              doc.version.isEqual(SnapshotVersion.MIN) ||
+              doc.remoteVersion.isEqual(SnapshotVersion.MIN) ||
               authoritativeUpdates.has(doc.key) ||
-              doc.version.compareTo(existingDoc.version) >= 0
+              doc.remoteVersion.compareTo(existingDoc.remoteVersion) >= 0
             ) {
               documentBuffer.addEntry(doc);
             } else {
@@ -574,9 +574,9 @@ export class LocalStore {
                 'Ignoring outdated watch update for ',
                 key,
                 '. Current version:',
-                existingDoc.version,
+                existingDoc.remoteVersion,
                 ' Watch version:',
-                doc.version
+                doc.remoteVersion
               );
             }
 
@@ -1008,7 +1008,7 @@ export class LocalStore {
             ackVersion !== null,
             'ackVersions should contain every doc in the write.'
           );
-          if (!doc || doc.version.compareTo(ackVersion!) < 0) {
+          if (!doc || doc.remoteVersion.compareTo(ackVersion!) < 0) {
             doc = batch.applyToRemoteDocument(docKey, doc, batchResult);
             if (!doc) {
               assert(

packages/firestore/src/model/document.ts

@@ -31,7 +31,10 @@ export interface DocumentOptions {
  * marker that this document does not exist at a given version.
  */
 export abstract class MaybeDocument {
-  constructor(readonly key: DocumentKey, readonly version: SnapshotVersion) {}
+  constructor(
+    readonly key: DocumentKey,
+    readonly remoteVersion: SnapshotVersion
+  ) {}
 
   static compareByKey(d1: MaybeDocument, d2: MaybeDocument): number {
     return DocumentKey.comparator(d1.key, d2.key);
@@ -47,11 +50,12 @@ export class Document extends MaybeDocument {
 
   constructor(
     key: DocumentKey,
-    version: SnapshotVersion,
+    remoteVersion: SnapshotVersion,
+    readonly commitVersion: SnapshotVersion,
     readonly data: ObjectValue,
     options: DocumentOptions
   ) {
-    super(key, version);
+    super(key, remoteVersion);
     this.hasLocalMutations = options.hasLocalMutations;
   }
 
@@ -72,15 +76,16 @@ export class Document extends MaybeDocument {
     return (
       other instanceof Document &&
       this.key.isEqual(other.key) &&
-      this.version.isEqual(other.version) &&
+      this.remoteVersion.isEqual(other.remoteVersion) &&
+      this.commitVersion.isEqual(other.commitVersion) &&
       this.data.isEqual(other.data) &&
       this.hasLocalMutations === other.hasLocalMutations
     );
   }
 
   toString(): string {
     return (
-      `Document(${this.key}, ${this.version}, ${this.data.toString()}, ` +
+      `Document(${this.key}, ${this.remoteVersion}, ${this.data.toString()}, ` +
       `{hasLocalMutations: ${this.hasLocalMutations}})`
     );
   }
@@ -102,18 +107,18 @@ export class Document extends MaybeDocument {
  * denotes time we know it didn't exist at.
  */
 export class NoDocument extends MaybeDocument {
-  constructor(key: DocumentKey, version: SnapshotVersion) {
-    super(key, version);
+  constructor(key: DocumentKey, remoteVersion: SnapshotVersion) {
+    super(key, remoteVersion);
   }
 
   toString(): string {
-    return `NoDocument(${this.key}, ${this.version})`;
+    return `NoDocument(${this.key}, ${this.remoteVersion})`;
   }
 
   isEqual(other: NoDocument): boolean {
     return (
       other &&
-      other.version.isEqual(this.version) &&
+      other.remoteVersion.isEqual(this.remoteVersion) &&
       other.key.isEqual(this.key)
     );
   }

packages/firestore/src/model/mutation.ts

@@ -79,9 +79,11 @@ export class FieldTransform {
 export class MutationResult {
   constructor(
     /**
-     * The version at which the mutation was committed or null for a delete.
+     * The version at which the mutation was committed, either extracted from
+     * the mutation's updateTime or from the responses commit time (for
+     * deletes).
      */
-    readonly version: SnapshotVersion | null,
+    readonly version: SnapshotVersion,
     /**
      * The resulting fields returned from the backend after a
      * TransformMutation has been committed. Contains one FieldValue for each
@@ -141,7 +143,7 @@ export class Precondition {
     if (this.updateTime !== undefined) {
       return (
         maybeDoc instanceof Document &&
-        maybeDoc.version.isEqual(this.updateTime)
+        maybeDoc.remoteVersion.isEqual(this.updateTime)
       );
     } else if (this.exists !== undefined) {
       if (this.exists) {
@@ -264,7 +266,7 @@ export abstract class Mutation {
     maybeDoc: MaybeDocument | null
   ): SnapshotVersion {
     if (maybeDoc instanceof Document) {
-      return maybeDoc.version;
+      return maybeDoc.remoteVersion;
     } else {
       return SnapshotVersion.MIN;
     }
@@ -301,8 +303,9 @@ export class SetMutation extends Mutation {
     // document the server has accepted the mutation so the precondition must
     // have held.
 
-    const version = Mutation.getPostMutationVersion(maybeDoc);
-    return new Document(this.key, version, this.value, {
+    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
+    const commitVersion = mutationResult.version;
+    return new Document(this.key, remoteVersion, commitVersion, this.value, {
       hasLocalMutations: false
     });
   }
@@ -318,10 +321,16 @@ export class SetMutation extends Mutation {
       return maybeDoc;
     }
 
-    const version = Mutation.getPostMutationVersion(maybeDoc);
-    return new Document(this.key, version, this.value, {
-      hasLocalMutations: true
-    });
+    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
+    return new Document(
+      this.key,
+      remoteVersion,
+      /*commitVersion=*/ SnapshotVersion.MIN,
+      this.value,
+      {
+        hasLocalMutations: true
+      }
+    );
   }
 
   isEqual(other: Mutation): boolean {
@@ -380,9 +389,10 @@ export class PatchMutation extends Mutation {
       return maybeDoc;
     }
 
-    const version = Mutation.getPostMutationVersion(maybeDoc);
+    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
+    const commitVersion = mutationResult.version;
     const newData = this.patchDocument(maybeDoc);
-    return new Document(this.key, version, newData, {
+    return new Document(this.key, remoteVersion, commitVersion, newData, {
       hasLocalMutations: false
     });
   }
@@ -398,11 +408,17 @@ export class PatchMutation extends Mutation {
       return maybeDoc;
     }
 
-    const version = Mutation.getPostMutationVersion(maybeDoc);
+    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
     const newData = this.patchDocument(maybeDoc);
-    return new Document(this.key, version, newData, {
-      hasLocalMutations: true
-    });
+    return new Document(
+      this.key,
+      remoteVersion,
+      /*commitVersion=*/ SnapshotVersion.MIN,
+      newData,
+      {
+        hasLocalMutations: true
+      }
+    );
   }
 
   isEqual(other: Mutation): boolean {
@@ -492,8 +508,10 @@ export class TransformMutation extends Mutation {
       maybeDoc,
       mutationResult.transformResults!
     );
+    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
+    const commitVersion = mutationResult.version;
     const newData = this.transformObject(doc.data, transformResults);
-    return new Document(this.key, doc.version, newData, {
+    return new Document(this.key, remoteVersion, commitVersion, newData, {
       hasLocalMutations: false
     });
   }
@@ -515,9 +533,15 @@ export class TransformMutation extends Mutation {
       baseDoc
     );
     const newData = this.transformObject(doc.data, transformResults);
-    return new Document(this.key, doc.version, newData, {
-      hasLocalMutations: true
-    });
+    return new Document(
+      this.key,
+      doc.remoteVersion,
+      SnapshotVersion.MIN,
+      newData,
+      {
+        hasLocalMutations: true
+      }
+    );
   }
 
   isEqual(other: Mutation): boolean {

packages/firestore/src/model/mutation_batch.ts

@@ -184,14 +184,7 @@ export class MutationBatchResult {
     let versionMap = documentVersionMap();
     const mutations = batch.mutations;
     for (let i = 0; i < mutations.length; i++) {
-      let version = results[i].version;
-      if (version === null) {
-        // deletes don't have a version, so we substitute the commitVersion
-        // of the entire batch.
-        version = commitVersion;
-      }
-
-      versionMap = versionMap.insert(mutations[i].key, version);
+      versionMap = versionMap.insert(mutations[i].key, results[i].version);
     }
 
     return new MutationBatchResult(

packages/firestore/src/remote/datastore.ts

@@ -87,7 +87,10 @@ export class Datastore {
       'Commit',
       params
     ).then(response => {
-      return this.serializer.fromWriteResults(response.writeResults);
+      return this.serializer.fromWriteResults(
+        response.writeResults,
+        response.commitTime
+      );
     });
   }
 

packages/firestore/src/remote/persistent_stream.ts

@@ -730,7 +730,8 @@ export class PersistentWriteStream extends PersistentStream<
       this.backoff.reset();
 
       const results = this.serializer.fromWriteResults(
-        responseProto.writeResults
+        responseProto.writeResults,
+        responseProto.commitTime
       );
       const commitVersion = this.serializer.fromVersion(
         responseProto.commitTime!