Adding a dirty bit

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.remoteVersion;
+    let docVersion = doc.version;
     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

@@ -448,12 +448,12 @@ export class DbRemoteDocument {
      */
     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`).
+     * Documents that were written to the remote document store based on
+     * a write acknowledgment are marked with `hasCommittedMutations`. These
+     * documents are potentially inconsistent with the document's version on
+     * the backend.
      */
-    public commitVersion?: DbTimestamp
+    public hasCommittedMutations?: boolean
   ) {}
 }
 

packages/firestore/src/local/local_serializer.ts

@@ -43,8 +43,10 @@ export class LocalSerializer {
   /** Decodes a remote document from storage locally to a Document. */
   fromDbRemoteDocument(remoteDoc: DbRemoteDocument): MaybeDocument {
     if (remoteDoc.document) {
-      const commitTime = this.fromDbTimestamp(remoteDoc.commitVersion);
-      return this.remoteSerializer.fromDocument(remoteDoc.document, commitTime);
+      return this.remoteSerializer.fromDocument(
+        remoteDoc.document,
+        !!remoteDoc.hasCommittedMutations
+      );
     } else if (remoteDoc.noDocument) {
       const key = DocumentKey.fromSegments(remoteDoc.noDocument.path);
       const version = this.fromDbTimestamp(remoteDoc.noDocument.readTime);
@@ -57,12 +59,12 @@ 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, commitVersion);
+      const hasCommittedMutations = maybeDoc.hasCommittedMutations;
+      return new DbRemoteDocument(null, doc, hasCommittedMutations);
     } else {
       const path = maybeDoc.key.path.toArray();
-      const readTime = this.toDbTimestamp(maybeDoc.remoteVersion);
+      const readTime = this.toDbTimestamp(maybeDoc.version);
       return new DbRemoteDocument(new DbNoDocument(path, readTime), null);
     }
   }
@@ -72,16 +74,12 @@ export class LocalSerializer {
     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;
-    }
+  private fromDbTimestamp(dbTimestamp: DbTimestamp): SnapshotVersion {
+    const timestamp = new Timestamp(
+      dbTimestamp.seconds,
+      dbTimestamp.nanoseconds
+    );
+    return SnapshotVersion.fromTimestamp(timestamp);
   }
 
   /** Encodes a batch of mutations into a DbMutationBatch for local storage. */

packages/firestore/src/local/local_store.ts

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

packages/firestore/src/model/document.ts

@@ -23,18 +23,16 @@ import { FieldValue, JsonObject, ObjectValue } from './field_value';
 import { FieldPath } from './path';
 
 export interface DocumentOptions {
-  hasLocalMutations: boolean;
+  hasLocalMutations?: boolean;
+  hasCommittedMutations?: boolean;
 }
 
 /**
  * The result of a lookup for a given path may be an existing document or a
  * marker that this document does not exist at a given version.
  */
 export abstract class MaybeDocument {
-  constructor(
-    readonly key: DocumentKey,
-    readonly remoteVersion: SnapshotVersion
-  ) {}
+  constructor(readonly key: DocumentKey, readonly version: SnapshotVersion) {}
 
   static compareByKey(d1: MaybeDocument, d2: MaybeDocument): number {
     return DocumentKey.comparator(d1.key, d2.key);
@@ -47,16 +45,17 @@ export abstract class MaybeDocument {
  */
 export class Document extends MaybeDocument {
   readonly hasLocalMutations: boolean;
+  readonly hasCommittedMutations: boolean;
 
   constructor(
     key: DocumentKey,
-    remoteVersion: SnapshotVersion,
-    readonly commitVersion: SnapshotVersion,
+    version: SnapshotVersion,
     readonly data: ObjectValue,
     options: DocumentOptions
   ) {
-    super(key, remoteVersion);
-    this.hasLocalMutations = options.hasLocalMutations;
+    super(key, version);
+    this.hasLocalMutations = !!options.hasLocalMutations;
+    this.hasCommittedMutations = !!options.hasCommittedMutations;
   }
 
   field(path: FieldPath): FieldValue | undefined {
@@ -76,17 +75,18 @@ export class Document extends MaybeDocument {
     return (
       other instanceof Document &&
       this.key.isEqual(other.key) &&
-      this.remoteVersion.isEqual(other.remoteVersion) &&
-      this.commitVersion.isEqual(other.commitVersion) &&
+      this.version.isEqual(other.version) &&
       this.data.isEqual(other.data) &&
-      this.hasLocalMutations === other.hasLocalMutations
+      this.hasLocalMutations === other.hasLocalMutations &&
+      this.hasCommittedMutations === other.hasCommittedMutations
     );
   }
 
   toString(): string {
     return (
-      `Document(${this.key}, ${this.remoteVersion}, ${this.data.toString()}, ` +
-      `{hasLocalMutations: ${this.hasLocalMutations}})`
+      `Document(${this.key}, ${this.version}, ${this.data.toString()}, ` +
+      `{hasLocalMutations: ${this.hasLocalMutations}}), ` +
+      `{hasCommittedMutations: ${this.hasCommittedMutations}})`
     );
   }
 
@@ -107,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, remoteVersion: SnapshotVersion) {
-    super(key, remoteVersion);
+  constructor(key: DocumentKey, version: SnapshotVersion) {
+    super(key, version);
   }
 
   toString(): string {
-    return `NoDocument(${this.key}, ${this.remoteVersion})`;
+    return `NoDocument(${this.key}, ${this.version})`;
   }
 
   isEqual(other: NoDocument): boolean {
     return (
       other &&
-      other.remoteVersion.isEqual(this.remoteVersion) &&
+      other.version.isEqual(this.version) &&
       other.key.isEqual(this.key)
     );
   }

packages/firestore/src/model/mutation.ts

@@ -142,7 +142,7 @@ export class Precondition {
     if (this.updateTime !== undefined) {
       return (
         maybeDoc instanceof Document &&
-        maybeDoc.remoteVersion.isEqual(this.updateTime)
+        maybeDoc.version.isEqual(this.updateTime)
       );
     } else if (this.exists !== undefined) {
       if (this.exists) {
@@ -265,7 +265,7 @@ export abstract class Mutation {
     maybeDoc: MaybeDocument | null
   ): SnapshotVersion {
     if (maybeDoc instanceof Document) {
-      return maybeDoc.remoteVersion;
+      return maybeDoc.version;
     } else {
       return SnapshotVersion.MIN;
     }
@@ -302,10 +302,9 @@ export class SetMutation extends Mutation {
     // document the server has accepted the mutation so the precondition must
     // have held.
 
-    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
-    const commitVersion = mutationResult.version;
-    return new Document(this.key, remoteVersion, commitVersion, this.value, {
-      hasLocalMutations: false
+    const version = mutationResult.version;
+    return new Document(this.key, version, this.value, {
+      hasCommittedMutations: true
     });
   }
 
@@ -320,16 +319,10 @@ export class SetMutation extends Mutation {
       return maybeDoc;
     }
 
-    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
-    return new Document(
-      this.key,
-      remoteVersion,
-      /*commitVersion=*/ SnapshotVersion.MIN,
-      this.value,
-      {
-        hasLocalMutations: true
-      }
-    );
+    const version = Mutation.getPostMutationVersion(maybeDoc);
+    return new Document(this.key, version, this.value, {
+      hasLocalMutations: true
+    });
   }
 
   isEqual(other: Mutation): boolean {
@@ -388,11 +381,10 @@ export class PatchMutation extends Mutation {
       return maybeDoc;
     }
 
-    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
-    const commitVersion = mutationResult.version;
+    const version = mutationResult.version;
     const newData = this.patchDocument(maybeDoc);
-    return new Document(this.key, remoteVersion, commitVersion, newData, {
-      hasLocalMutations: false
+    return new Document(this.key, version, newData, {
+      hasCommittedMutations: true
     });
   }
 
@@ -407,17 +399,11 @@ export class PatchMutation extends Mutation {
       return maybeDoc;
     }
 
-    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
+    const version = Mutation.getPostMutationVersion(maybeDoc);
     const newData = this.patchDocument(maybeDoc);
-    return new Document(
-      this.key,
-      remoteVersion,
-      /*commitVersion=*/ SnapshotVersion.MIN,
-      newData,
-      {
-        hasLocalMutations: true
-      }
-    );
+    return new Document(this.key, version, newData, {
+      hasLocalMutations: true
+    });
   }
 
   isEqual(other: Mutation): boolean {
@@ -507,11 +493,10 @@ export class TransformMutation extends Mutation {
       maybeDoc,
       mutationResult.transformResults!
     );
-    const remoteVersion = Mutation.getPostMutationVersion(maybeDoc);
-    const commitVersion = mutationResult.version;
+    const version = mutationResult.version;
     const newData = this.transformObject(doc.data, transformResults);
-    return new Document(this.key, remoteVersion, commitVersion, newData, {
-      hasLocalMutations: false
+    return new Document(this.key, version, newData, {
+      hasCommittedMutations: true
     });
   }
 
@@ -532,15 +517,9 @@ export class TransformMutation extends Mutation {
       baseDoc
     );
     const newData = this.transformObject(doc.data, transformResults);
-    return new Document(
-      this.key,
-      doc.remoteVersion,
-      SnapshotVersion.MIN,
-      newData,
-      {
-        hasLocalMutations: true
-      }
-    );
+    return new Document(this.key, doc.version, newData, {
+      hasLocalMutations: true
+    });
   }
 
   isEqual(other: Mutation): boolean {