Merge UnknownDocument

Author: schmidt-sebastian

packages/firestore/src/local/indexeddb_schema.ts

@@ -425,6 +425,14 @@ export class DbNoDocument {
   constructor(public path: string[], public readTime: DbTimestamp) {}
 }
 
+/**
+ * Represents a document that is know to exist but whose data is unknwon.
+ * Stored in IndexedDb as part of a DbRemoteDocument object.
+ */
+export class DbUnknownDocument {
+  constructor(public path: string[], public readTime: DbTimestamp) {}
+}
+
 /**
  * An object to be stored in the 'remoteDocuments' store in IndexedDb. It
  * represents either a cached document (if it exists) or a cached "no-document"
@@ -437,6 +445,12 @@ export class DbRemoteDocument {
   static store = 'remoteDocuments';
 
   constructor(
+    /**
+     * Set to an instance of DbUnknownDocument if the data for a document is
+     * not known, but yet we no the document exist (e.g. it had a successful
+     * update applied to it)
+     */
+    public unknownDocument: DbUnknownDocument | null,
     /**
      * Set to an instance of a DbNoDocument if it is known that no document
      * exists.

packages/firestore/src/local/local_serializer.ts

@@ -18,7 +18,12 @@ import * as api from '../protos/firestore_proto_api';
 import { Timestamp } from '../api/timestamp';
 import { Query } from '../core/query';
 import { SnapshotVersion } from '../core/snapshot_version';
-import { Document, MaybeDocument, NoDocument } from '../model/document';
+import {
+  Document,
+  MaybeDocument,
+  NoDocument,
+  UnknownDocument
+} from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { MutationBatch } from '../model/mutation_batch';
 import { JsonProtoSerializer } from '../remote/serializer';
@@ -30,7 +35,8 @@ import {
   DbQuery,
   DbRemoteDocument,
   DbTarget,
-  DbTimestamp
+  DbTimestamp,
+  DbUnknownDocument
 } from './indexeddb_schema';
 import { QueryData, QueryPurpose } from './query_data';
 import { decode, encode, EncodedResourcePath } from './encoded_resource_path';
@@ -51,6 +57,10 @@ export class LocalSerializer {
       const key = DocumentKey.fromSegments(remoteDoc.noDocument.path);
       const version = this.fromDbTimestamp(remoteDoc.noDocument.readTime);
       return new NoDocument(key, version);
+    } else if (remoteDoc.unknownDocument) {
+      const key = DocumentKey.fromSegments(remoteDoc.unknownDocument.path);
+      const version = this.fromDbTimestamp(remoteDoc.unknownDocument.readTime);
+      return new UnknownDocument(key, version);
     } else {
       return fail('Unexpected DbRemoteDocument');
     }
@@ -61,11 +71,27 @@ export class LocalSerializer {
     if (maybeDoc instanceof Document) {
       const doc = this.remoteSerializer.toDocument(maybeDoc);
       const hasCommittedMutations = maybeDoc.hasCommittedMutations;
-      return new DbRemoteDocument(null, doc, hasCommittedMutations);
-    } else {
+      return new DbRemoteDocument(null, null, doc, hasCommittedMutations);
+    } else if (maybeDoc instanceof NoDocument) {
       const path = maybeDoc.key.path.toArray();
       const readTime = this.toDbTimestamp(maybeDoc.version);
-      return new DbRemoteDocument(new DbNoDocument(path, readTime), null);
+      return new DbRemoteDocument(
+        null,
+        new DbNoDocument(path, readTime),
+        null,
+        /* hasCommittedMutations= */ false
+      );
+    } else if (maybeDoc instanceof UnknownDocument) {
+      const path = maybeDoc.key.path.toArray();
+      const readTime = this.toDbTimestamp(maybeDoc.version);
+      return new DbRemoteDocument(
+        new DbUnknownDocument(path, readTime),
+        null,
+        null,
+        /* hasCommittedMutations= */ false
+      );
+    } else {
+      return fail('Unexpected MaybeDocumment');
     }
   }
 

packages/firestore/src/local/local_store.ts

@@ -25,7 +25,7 @@ import {
   DocumentMap,
   MaybeDocumentMap
 } from '../model/collections';
-import { MaybeDocument } from '../model/document';
+import { MaybeDocument, UnknownDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { Mutation } from '../model/mutation';
 import {
@@ -498,30 +498,13 @@ export class LocalStore {
     const documentBuffer = new RemoteDocumentChangeBuffer(this.remoteDocuments);
     return this.persistence.runTransaction('Apply remote event', true, txn => {
       const promises = [] as Array<PersistencePromise<void>>;
-      let authoritativeUpdates = documentKeySet();
       objUtils.forEachNumber(
         remoteEvent.targetChanges,
         (targetId: TargetId, change: TargetChange) => {
           // Do not ref/unref unassigned targetIds - it may lead to leaks.
           let queryData = this.queryDataByTarget[targetId];
           if (!queryData) return;
 
-          // When a global snapshot contains updates (either add or modify) we
-          // can completely trust these updates as authoritative and blindly
-          // apply them to our cache (as a defensive measure to promote
-          // self-healing in the unfortunate case that our cache is ever somehow
-          // corrupted / out-of-sync).
-          //
-          // If the document is only updated while removing it from a target
-          // then watch isn't obligated to send the absolute latest version: it
-          // can send the first version that caused the document not to match.
-          change.addedDocuments.forEach(key => {
-            authoritativeUpdates = authoritativeUpdates.add(key);
-          });
-          change.modifiedDocuments.forEach(key => {
-            authoritativeUpdates = authoritativeUpdates.add(key);
-          });
-
           promises.push(
             this.queryCache
               .removeMatchingKeys(txn, change.removedDocuments, targetId)
@@ -559,15 +542,9 @@ export class LocalStore {
         changedDocKeys = changedDocKeys.add(key);
         promises.push(
           documentBuffer.getEntry(txn, key).next(existingDoc => {
-            // If a document update isn't authoritative, make sure we don't
-            // apply an old document version to the remote cache. We make an
-            // exception for SnapshotVersion.MIN which can happen for
-            // manufactured events (e.g. in the case of a limbo document
-            // resolution failing).
             if (
               existingDoc == null ||
               doc.version.isEqual(SnapshotVersion.MIN) ||
-              authoritativeUpdates.has(doc.key) ||
               doc.version.compareTo(existingDoc.version) >= 0
             ) {
               documentBuffer.addEntry(doc);

packages/firestore/src/model/document.ts

@@ -37,6 +37,8 @@ export abstract class MaybeDocument {
   static compareByKey(d1: MaybeDocument, d2: MaybeDocument): number {
     return DocumentKey.comparator(d1.key, d2.key);
   }
+
+  abstract isEqual(other: MaybeDocument | null | undefined): boolean;
 }
 
 /**
@@ -71,7 +73,7 @@ export class Document extends MaybeDocument {
     return this.data.value();
   }
 
-  isEqual(other: Document | null | undefined): boolean {
+  isEqual(other: MaybeDocument | null | undefined): boolean {
     return (
       other instanceof Document &&
       this.key.isEqual(other.key) &&
@@ -115,9 +117,31 @@ export class NoDocument extends MaybeDocument {
     return `NoDocument(${this.key}, ${this.version})`;
   }
 
-  isEqual(other: NoDocument): boolean {
+  isEqual(other: MaybeDocument | null | undefined): boolean {
+    return (
+      other instanceof NoDocument &&
+      other.version.isEqual(this.version) &&
+      other.key.isEqual(this.key)
+    );
+  }
+}
+
+/**
+ * A class representing a document whose data is unknown (e.g. a document that
+ * was updated without a known base version).
+ */
+export class UnknownDocument extends MaybeDocument {
+  constructor(key: DocumentKey, version: SnapshotVersion) {
+    super(key, version);
+  }
+
+  toString(): string {
+    return `UnknownDocument(${this.key}, ${this.version})`;
+  }
+
+  isEqual(other: MaybeDocument | null | undefined): boolean {
     return (
-      other &&
+      other instanceof UnknownDocument &&
       other.version.isEqual(this.version) &&
       other.key.isEqual(this.key)
     );

packages/firestore/src/model/mutation.ts

@@ -19,7 +19,12 @@ import { SnapshotVersion } from '../core/snapshot_version';
 import { assert } from '../util/assert';
 import * as misc from '../util/misc';
 
-import { Document, MaybeDocument, NoDocument } from './document';
+import {
+  Document,
+  MaybeDocument,
+  NoDocument,
+  UnknownDocument
+} from './document';
 import { DocumentKey } from './document_key';
 import { FieldValue, ObjectValue } from './field_value';
 import { FieldPath } from './path';
@@ -221,7 +226,7 @@ export abstract class Mutation {
   abstract applyToRemoteDocument(
     maybeDoc: MaybeDocument | null,
     mutationResult: MutationResult
-  ): MaybeDocument | null;
+  ): MaybeDocument;
 
   /**
    * Applies this mutation to the given MaybeDocument or null for the purposes
@@ -290,7 +295,7 @@ export class SetMutation extends Mutation {
   applyToRemoteDocument(
     maybeDoc: MaybeDocument | null,
     mutationResult: MutationResult
-  ): MaybeDocument | null {
+  ): MaybeDocument {
     this.verifyKeyMatches(maybeDoc);
 
     assert(
@@ -363,25 +368,26 @@ export class PatchMutation extends Mutation {
   applyToRemoteDocument(
     maybeDoc: MaybeDocument | null,
     mutationResult: MutationResult
-  ): MaybeDocument | null {
+  ): MaybeDocument {
     this.verifyKeyMatches(maybeDoc);
 
     assert(
       mutationResult.transformResults == null,
       'Transform results received by PatchMutation.'
     );
 
+    const version = mutationResult.version;
+
     // TODO(mcg): Relax enforcement of this precondition
     //
     // We shouldn't actually enforce the precondition since it already passed on
     // the backend, but we may not have a local version of the document to
     // patch, so we use the precondition to prevent incorrectly putting a
     // partial document into our cache.
     if (!this.precondition.isValidFor(maybeDoc)) {
-      return maybeDoc;
+      return new UnknownDocument(this.key, version);
     }
 
-    const version = mutationResult.version;
     const newData = this.patchDocument(maybeDoc);
     return new Document(this.key, version, newData, {
       hasCommittedMutations: true
@@ -470,7 +476,7 @@ export class TransformMutation extends Mutation {
   applyToRemoteDocument(
     maybeDoc: MaybeDocument | null,
     mutationResult: MutationResult
-  ): MaybeDocument | null {
+  ): MaybeDocument {
     this.verifyKeyMatches(maybeDoc);
 
     assert(
@@ -485,7 +491,7 @@ export class TransformMutation extends Mutation {
     // patch, so we use the precondition to prevent incorrectly putting a
     // partial document into our cache.
     if (!this.precondition.isValidFor(maybeDoc)) {
-      return maybeDoc;
+      return new UnknownDocument(this.key, mutationResult.version);
     }
 
     const doc = this.requireDocument(maybeDoc);
@@ -646,7 +652,7 @@ export class DeleteMutation extends Mutation {
   applyToRemoteDocument(
     maybeDoc: MaybeDocument | null,
     mutationResult: MutationResult
-  ): MaybeDocument | null {
+  ): MaybeDocument {
     this.verifyKeyMatches(maybeDoc);
 
     assert(
@@ -658,7 +664,7 @@ export class DeleteMutation extends Mutation {
     // document the server has accepted the mutation so the precondition must
     // have held.
 
-    return new NoDocument(this.key, SnapshotVersion.MIN);
+    return new NoDocument(this.key, mutationResult.version);
   }
 
   applyToLocalView(

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

@@ -24,10 +24,13 @@ import {
   apiDescribe,
   withTestCollection,
   withTestDb,
+  withTestDbs,
   withTestDoc,
   withTestDocAndInitialData
 } from '../util/helpers';
 import { query } from '../../util/api_helpers';
+import { fail } from '../../../src/util/assert';
+import { Code } from '../../../src/util/error';
 
 const Timestamp = firebase.firestore!.Timestamp;
 const FieldValue = firebase.firestore!.FieldValue;
@@ -108,6 +111,30 @@ apiDescribe('Database', persistence => {
     });
   });
 
+  (persistence ? it : it.skip)('can update an unknown document', () => {
+    return withTestDbs(persistence, 2, async ([reader, writer]) => {
+      const writerRef = writer.collection('collection').doc();
+      const readerRef = reader.collection('collection').doc(writerRef.id);
+      await writerRef.set({ a: 'a' });
+      await readerRef.update({ b: 'b' });
+      await writerRef
+        .get({ source: 'cache' })
+        .then(doc => expect(doc.exists).to.be.true);
+      await readerRef
+        .get({ source: 'cache' })
+        .then(
+          () => fail('Expected cache miss'),
+          err => expect(err.code).to.be.equal(Code.UNAVAILABLE)
+        );
+      await writerRef
+        .get()
+        .then(doc => expect(doc.data()).to.deep.equal({ a: 'a', b: 'b' }));
+      await readerRef
+        .get()
+        .then(doc => expect(doc.data()).to.deep.equal({ a: 'a', b: 'b' }));
+    });
+  });
+
   it('can merge data with an existing document using set', () => {
     return withTestDoc(persistence, doc => {
       const initialData = {