Fix overlay bug when offline (Port Android SDK PR #3537).

Author: ehsannas

packages/firestore/src/local/local_documents_view.ts

@@ -132,7 +132,7 @@ export class LocalDocumentsView {
   getLocalViewOfDocuments(
     transaction: PersistenceTransaction,
     docs: MutableDocumentMap,
-    existenceStateChanged: DocumentKeySet
+    existenceStateChanged: DocumentKeySet = documentKeySet()
   ): PersistencePromise<DocumentMap> {
     const overlays = newOverlayMap();
     return this.populateOverlays(transaction, overlays, docs).next(() => {

packages/firestore/src/local/local_store_impl.ts

@@ -314,11 +314,32 @@ export function localStoreWriteLocally(
 
   return localStoreImpl.persistence
     .runTransaction('Locally write mutations', 'readwrite', txn => {
-      // Load and apply all existing mutations. This lets us compute the
-      // current base state for all non-idempotent transforms before applying
-      // any additional user-provided writes.
-      return localStoreImpl.localDocuments
-        .getDocuments(txn, keys)
+      // Figure out which keys do not have a remote version in the cache, this
+      // is needed to create the right overlay mutation: if no remote version
+      // presents, we do not need to create overlays as patch mutations.
+      // TODO(Overlay): Is there a better way to determine this? Document
+      //  version does not work because local mutations set them back to 0.
+      let remoteDocs = mutableDocumentMap();
+      let docsWithoutRemoteVersion = documentKeySet();
+      return localStoreImpl.remoteDocuments
+        .getEntries(txn, keys)
+        .next(docs => {
+          remoteDocs = docs;
+          remoteDocs.forEach((key, doc) => {
+            if (!doc.isValidDocument()) {
+              docsWithoutRemoteVersion = docsWithoutRemoteVersion.add(key);
+            }
+          });
+        })
+        .next(() => {
+          // Load and apply all existing mutations. This lets us compute the
+          // current base state for all non-idempotent transforms before applying
+          // any additional user-provided writes.
+          return localStoreImpl.localDocuments.getLocalViewOfDocuments(
+            txn,
+            remoteDocs
+          );
+        })
         .next(docs => {
           existingDocs = docs;
 
@@ -358,7 +379,10 @@ export function localStoreWriteLocally(
         })
         .next(batch => {
           mutationBatch = batch;
-          const overlays = batch.applyToLocalDocumentSet(existingDocs);
+          const overlays = batch.applyToLocalDocumentSet(
+            existingDocs,
+            docsWithoutRemoteVersion
+          );
           return localStoreImpl.documentOverlayCache.saveOverlays(
             txn,
             batch.batchId,

packages/firestore/src/model/mutation_batch.ts

@@ -138,7 +138,10 @@ export class MutationBatch {
    * this batch. Returns a `DocumentKey` to `Mutation` map which can be used to
    * replace all the mutation applications.
    */
-  applyToLocalDocumentSet(documentMap: DocumentMap): MutationMap {
+  applyToLocalDocumentSet(
+    documentMap: DocumentMap,
+    documentsWithoutRemoteVersion: DocumentKeySet
+  ): MutationMap {
     // TODO(mrschmidt): This implementation is O(n^2). If we apply the mutations
     // directly (as done in `applyToLocalView()`), we can reduce the complexity
     // to O(n).
@@ -148,7 +151,13 @@ export class MutationBatch {
       // TODO(mutabledocuments): This method should take a MutableDocumentMap
       // and we should remove this cast.
       const mutableDocument = document as MutableDocument;
-      const mutatedFields = this.applyToLocalView(mutableDocument);
+      let mutatedFields = this.applyToLocalView(mutableDocument);
+      // Set mutatedFields to null if the document is only from local mutations.
+      // This creates a Set or Delete mutation, instead of trying to create a
+      // patch mutation as the overlay.
+      mutatedFields = documentsWithoutRemoteVersion.has(m.key)
+        ? null
+        : mutatedFields;
       const overlay = calculateOverlayMutation(mutableDocument, mutatedFields);
       if (overlay !== null) {
         overlays.set(m.key, overlay);

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

@@ -949,7 +949,7 @@ function genericLocalStoreTests(
       .toContain(deletedDoc('foo/bar', 0).setHasLocalMutations())
       .afterAcknowledgingMutation({ documentVersion: 2 }) // delete mutation
       .toReturnRemoved('foo/bar')
-      .toContain(deletedDoc('foo/bar', 2).setHasCommittedMutations())
+      .toContain(deletedDoc('foo/bar', 2).setHasLocalMutations())
       .afterAcknowledgingMutation({ documentVersion: 3 }) // patch mutation
       .toReturnChanged(unknownDoc('foo/bar', 3))
       .toNotContainIfEager(unknownDoc('foo/bar', 3))
@@ -1656,6 +1656,27 @@ function genericLocalStoreTests(
       .finish();
   });
 
+  it('add then update while offline', () => {
+    return expectLocalStore()
+      .afterMutations([
+        setMutation('foo/bar', { 'foo': 'foo-value', 'bar': 1 })
+      ])
+      .toContain(
+        doc('foo/bar', 0, {
+          'foo': 'foo-value',
+          'bar': 1
+        }).setHasLocalMutations()
+      )
+      .afterMutations([patchMutation('foo/bar', { 'bar': 2 })])
+      .toContain(
+        doc('foo/bar', 0, {
+          'foo': 'foo-value',
+          'bar': 2
+        }).setHasLocalMutations()
+      )
+      .finish();
+  });
+
   it('handles saving and loading named queries', async () => {
     return expectLocalStore()
       .after(