Remove Held Write Acks (#1156)

Author: schmidt-sebastian

packages/firestore/src/api/database.ts

@@ -593,11 +593,27 @@ export class Transaction implements firestore.Transaction {
         }
         const doc = docs[0];
         if (doc instanceof NoDocument) {
-          return new DocumentSnapshot(this._firestore, ref._key, null, false);
+          return new DocumentSnapshot(
+            this._firestore,
+            ref._key,
+            null,
+            /* fromCache= */ false,
+            /* hasPendingWrites= */ false
+          );
         } else if (doc instanceof Document) {
-          return new DocumentSnapshot(this._firestore, ref._key, doc, false);
+          return new DocumentSnapshot(
+            this._firestore,
+            ref._key,
+            doc,
+            /* fromCache= */ false,
+            /* hasPendingWrites= */ false
+          );
         } else {
-          throw fail('MaybeDocument is neither Document nor NoDocument');
+          throw fail(
+            `BatchGetDocumentsRequest returned unexpected document type: ${
+              doc.constructor.name
+            }`
+          );
         }
       });
   }
@@ -1040,7 +1056,8 @@ export class DocumentReference implements firestore.DocumentReference {
               this.firestore,
               this._key,
               doc,
-              snapshot.fromCache
+              snapshot.fromCache,
+              snapshot.hasPendingWrites
             )
           );
         }
@@ -1082,7 +1099,8 @@ export class DocumentReference implements firestore.DocumentReference {
                   this.firestore,
                   this._key,
                   doc,
-                  /*fromCache=*/ true
+                  /*fromCache=*/ true,
+                  doc instanceof Document ? doc.hasLocalMutations : false
                 )
               );
             }, reject);
@@ -1173,7 +1191,8 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
     private _firestore: Firestore,
     private _key: DocumentKey,
     public _document: Document | null,
-    private _fromCache: boolean
+    private _fromCache: boolean,
+    private _hasPendingWrites: boolean
   ) {}
 
   data(
@@ -1228,10 +1247,7 @@ export class DocumentSnapshot implements firestore.DocumentSnapshot {
   }
 
   get metadata(): firestore.SnapshotMetadata {
-    return new SnapshotMetadata(
-      this._document !== null && this._document.hasLocalMutations,
-      this._fromCache
-    );
+    return new SnapshotMetadata(this._hasPendingWrites, this._fromCache);
   }
 
   isEqual(other: firestore.DocumentSnapshot): boolean {
@@ -1299,9 +1315,10 @@ export class QueryDocumentSnapshot extends DocumentSnapshot
     firestore: Firestore,
     key: DocumentKey,
     document: Document,
-    fromCache: boolean
+    fromCache: boolean,
+    hasPendingWrites: boolean
   ) {
-    super(firestore, key, document, fromCache);
+    super(firestore, key, document, fromCache, hasPendingWrites);
   }
 
   data(options?: SnapshotOptions): firestore.DocumentData {
@@ -1945,7 +1962,8 @@ export class QuerySnapshot implements firestore.QuerySnapshot {
       this._firestore,
       doc.key,
       doc,
-      this.metadata.fromCache
+      this.metadata.fromCache,
+      this._snapshot.mutatedKeys.has(doc.key)
     );
   }
 }
@@ -2133,7 +2151,8 @@ export function changesFromSnapshot(
         firestore,
         change.doc.key,
         change.doc,
-        snapshot.fromCache
+        snapshot.fromCache,
+        snapshot.mutatedKeys.has(change.doc.key)
       );
       assert(
         change.type === ChangeType.Added,
@@ -2164,7 +2183,8 @@ export function changesFromSnapshot(
           firestore,
           change.doc.key,
           change.doc,
-          snapshot.fromCache
+          snapshot.fromCache,
+          snapshot.mutatedKeys.has(change.doc.key)
         );
         let oldIndex = -1;
         let newIndex = -1;

packages/firestore/src/core/event_manager.ts

@@ -195,8 +195,8 @@ export class QueryListener {
         snap.docs,
         snap.oldDocs,
         docChanges,
+        snap.mutatedKeys,
         snap.fromCache,
-        snap.hasPendingWrites,
         snap.syncStateChanged,
         /* excludesMetadataChanges= */ true
       );
@@ -288,8 +288,8 @@ export class QueryListener {
     snap = ViewSnapshot.fromInitialDocuments(
       snap.query,
       snap.docs,
-      snap.fromCache,
-      snap.hasPendingWrites
+      snap.mutatedKeys,
+      snap.fromCache
     );
     this.raisedInitialEvent = true;
     this.queryObserver.next(snap);

packages/firestore/src/core/firestore_client.ts

@@ -366,8 +366,7 @@ export class FirestoreClient {
         this.localStore = new LocalStore(
           this.persistence,
           user,
-          this.garbageCollector,
-          this.sharedClientState
+          this.garbageCollector
         );
         const serializer = this.platform.newSerializer(
           this.databaseInfo.databaseId

packages/firestore/src/core/sync_engine.ts

@@ -245,7 +245,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
         .remoteDocumentKeys(queryData.targetId)
         .then(remoteKeys => {
           const view = new View(query, remoteKeys);
-          const viewDocChanges = view.computeDocChanges(docs);
+          const viewDocChanges = view.computeInitialChanges(docs);
           const synthesizedTargetChange = TargetChange.createSynthesizedTargetChangeForCurrentChange(
             queryData.targetId,
             current && this.onlineState !== OnlineState.Offline
@@ -606,6 +606,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
     return this.localStore
       .acknowledgeBatch(mutationBatchResult)
       .then(changes => {
+        this.sharedClientState.updateMutationState(batchId, 'acknowledged');
         return this.emitNewSnapsAndNotifyLocalStore(changes);
       })
       .catch(err => this.ignoreIfPrimaryLeaseLoss(err));

packages/firestore/src/core/transaction.ts

@@ -17,12 +17,13 @@
 import { ParsedSetData, ParsedUpdateData } from '../api/user_data_converter';
 import { SnapshotVersion } from './snapshot_version';
 import { documentVersionMap } from '../model/collections';
-import { NoDocument } from '../model/document';
+import { NoDocument, Document } from '../model/document';
 import { MaybeDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { DeleteMutation, Mutation, Precondition } from '../model/mutation';
 import { Datastore } from '../remote/datastore';
 import { Code, FirestoreError } from '../util/error';
+import { fail } from '../util/assert';
 
 /**
  * Internal transaction object responsible for accumulating the mutations to
@@ -37,11 +38,17 @@ export class Transaction {
   constructor(private datastore: Datastore) {}
 
   private recordVersion(doc: MaybeDocument): void {
-    let docVersion = doc.version;
-    if (doc instanceof NoDocument) {
+    let docVersion: SnapshotVersion;
+
+    if (doc instanceof Document) {
+      docVersion = doc.version;
+    } else if (doc instanceof NoDocument) {
       // For deleted docs, we must use baseVersion 0 when we overwrite them.
       docVersion = SnapshotVersion.forDeletedDoc();
+    } else {
+      throw fail('Document in a transaction was a ' + doc.constructor.name);
     }
+
     const existingVersion = this.readVersions.get(doc.key);
     if (existingVersion !== null) {
       if (!docVersion.isEqual(existingVersion)) {
@@ -68,7 +75,13 @@ export class Transaction {
       );
     }
     return this.datastore.lookup(keys).then(docs => {
-      docs.forEach(doc => this.recordVersion(doc));
+      docs.forEach(doc => {
+        if (doc instanceof NoDocument || doc instanceof Document) {
+          this.recordVersion(doc);
+        } else {
+          fail('Document in a transaction was a ' + doc.constructor.name);
+        }
+      });
       return docs;
     });
   }