From e2fddb05e54cfaf78b00f58174a20e165cff1a4f Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Wed, 6 Apr 2022 15:07:48 -0400 Subject: [PATCH 01/10] Fix overlay bug with multiple field patches --- .../google/firebase/firestore/QueryTest.java | 52 +++++++++++++++++++ .../firestore/local/LocalDocumentsResult.java | 13 +++++ .../firestore/local/LocalDocumentsView.java | 35 +++++++++---- .../firebase/firestore/local/LocalStore.java | 10 ++-- .../model/mutation/MutationBatch.java | 8 +-- .../firestore/model/mutation/Overlay.java | 6 +++ .../firestore/local/LocalStoreTestCase.java | 43 +++++++++++++++ 7 files changed, 150 insertions(+), 17 deletions(-) diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java index 1f4ed810eaa..a46436e35d8 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java @@ -920,4 +920,56 @@ public void testMultipleUpdatesWhileOffline() { QuerySnapshot snapshot2 = waitFor(collection.get(Source.CACHE)); assertEquals(asList(map("foo", "zzyzx", "bar", "2")), querySnapshotToValues(snapshot2)); } + + @Test + public void testMultipleUpdatesWhileOfflineOnRemoteDoc() { + CollectionReference collection = testCollection(); + DocumentReference doc = collection.document(); + doc.set(map("likes", 0, "stars", 0)); + + collection.getFirestore().disableNetwork(); + + doc.update("likes", 1); + QuerySnapshot snapshot1 = waitFor(collection.get(Source.CACHE)); + List result1 = querySnapshotToValues(snapshot1); + + doc.update("stars", 1); + snapshot1 = waitFor(collection.get(Source.CACHE)); + List result2 = querySnapshotToValues(snapshot1); + + doc.update("stars", 2); + snapshot1 = waitFor(collection.get(Source.CACHE)); + List result3 = querySnapshotToValues(snapshot1); + + result3.contains(0); + } + + @Test + public void testListenToMultipleUpdatesWhileOffline() { + CollectionReference collection = testCollection(); + collection.addSnapshotListener( + (snap, e) -> { + List result = querySnapshotToValues(snap); + result.contains(0); + }); + DocumentReference doc = collection.document(); + doc.set(map("likes", 0, "stars", 0)); + waitFor(doc.set(map("likes", 0, "stars", 0))); + + collection.getFirestore().disableNetwork(); + + doc.update("likes", 1); + QuerySnapshot snapshot1 = waitFor(collection.get(Source.CACHE)); + List result1 = querySnapshotToValues(snapshot1); + + doc.update("stars", 1); + snapshot1 = waitFor(collection.get(Source.CACHE)); + List result2 = querySnapshotToValues(snapshot1); + + doc.update("stars", 2); + snapshot1 = waitFor(collection.get(Source.CACHE)); + List result3 = querySnapshotToValues(snapshot1); + + result3.contains(0); + } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsResult.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsResult.java index f3b91589516..a1742c0908f 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsResult.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsResult.java @@ -14,9 +14,12 @@ package com.google.firebase.firestore.local; +import static com.google.firebase.firestore.model.DocumentCollections.emptyDocumentMap; + import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.DocumentKey; +import java.util.Map; /** * Represents a set of document along with their mutation batch ID. @@ -33,6 +36,16 @@ public final class LocalDocumentsResult { this.documents = documents; } + public static LocalDocumentsResult fromOverlayedDocuments( + int batchId, ImmutableSortedMap overlays) { + ImmutableSortedMap documents = emptyDocumentMap(); + for (Map.Entry entry : overlays) { + documents = documents.insert(entry.getKey(), entry.getValue().getOverlay()); + } + + return new LocalDocumentsResult(batchId, documents); + } + public int getBatchId() { return batchId; } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java index 8ab5aecf6cd..9a7f91a0bf4 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java @@ -119,7 +119,13 @@ ImmutableSortedMap getLocalViewOfDocuments( Map docs, Set existenceStateChanged) { Map overlays = new HashMap<>(); populateOverlays(overlays, docs.keySet()); - return computeViews(docs, overlays, existenceStateChanged); + ImmutableSortedMap result = emptyDocumentMap(); + for (Map.Entry entry : + computeViews(docs, overlays, existenceStateChanged)) { + result = result.insert(entry.getKey(), entry.getValue().getOverlay()); + } + + return result; } /** @@ -128,7 +134,7 @@ ImmutableSortedMap getLocalViewOfDocuments( * * @param docs The documents to apply local mutations to get the local views. */ - ImmutableSortedMap getLocalViewOfDocuments( + ImmutableSortedMap getLocalViewOfDocuments( Map docs) { Map overlays = new HashMap<>(); populateOverlays(overlays, docs.keySet()); @@ -136,12 +142,14 @@ ImmutableSortedMap getLocalViewOfDocuments( } /*Computes the local view for doc */ - private ImmutableSortedMap computeViews( + private ImmutableSortedMap computeViews( Map docs, Map overlays, Set existenceStateChanged) { - ImmutableSortedMap results = emptyDocumentMap(); + ImmutableSortedMap results = + ImmutableSortedMap.Builder.emptyMap(DocumentKey.comparator()); Map recalculateDocuments = new HashMap<>(); + Map mutatedFields = new HashMap<>(); for (MutableDocument doc : docs.values()) { Overlay overlay = overlays.get(doc.getKey()); // Recalculate an overlay if the document's existence state is changed due to a remote @@ -154,19 +162,26 @@ private ImmutableSortedMap computeViews( && (overlay == null || overlay.getMutation() instanceof PatchMutation)) { recalculateDocuments.put(doc.getKey(), doc); } else if (overlay != null) { + mutatedFields.put(doc.getKey(), overlay.getFieldMask()); overlay.getMutation().applyToLocalView(doc, null, Timestamp.now()); } } - recalculateAndSaveOverlays(recalculateDocuments); + Map recalculatedFields = + recalculateAndSaveOverlays(recalculateDocuments); + mutatedFields.putAll(recalculatedFields); for (Map.Entry entry : docs.entrySet()) { - results = results.insert(entry.getKey(), entry.getValue()); + results = + results.insert( + entry.getKey(), + new OverlayedDocument(entry.getValue(), mutatedFields.get(entry.getKey()))); } return results; } - private void recalculateAndSaveOverlays(Map docs) { + private Map recalculateAndSaveOverlays( + Map docs) { List batches = mutationQueue.getAllMutationBatchesAffectingDocumentKeys(docs.keySet()); @@ -211,6 +226,8 @@ private void recalculateAndSaveOverlays(Map docs) } documentOverlayCache.saveOverlays(entry.getKey(), overlays); } + + return masks; } /** @@ -311,9 +328,9 @@ LocalDocumentsResult getNextDocuments(String collectionGroup, IndexOffset offset } populateOverlays(overlays, docs.keySet()); - ImmutableSortedMap localDocs = + ImmutableSortedMap localDocs = computeViews(docs, overlays, Collections.emptySet()); - return new LocalDocumentsResult(largestBatchId, localDocs); + return LocalDocumentsResult.fromOverlayedDocuments(largestBatchId, localDocs); } /** diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java index babed062ecb..b5cad63cf84 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java @@ -258,7 +258,7 @@ public LocalDocumentsResult writeLocally(List mutations) { } // 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. - ImmutableSortedMap documents = + ImmutableSortedMap overlayedDocuments = localDocuments.getLocalViewOfDocuments(remoteDocs); // For non-idempotent mutations (such as `FieldValue.increment()`), we record the base @@ -268,7 +268,8 @@ public LocalDocumentsResult writeLocally(List mutations) { List baseMutations = new ArrayList<>(); for (Mutation mutation : mutations) { ObjectValue baseValue = - mutation.extractTransformBaseValue(documents.get(mutation.getKey())); + mutation.extractTransformBaseValue( + overlayedDocuments.get(mutation.getKey()).getOverlay()); if (baseValue != null) { // NOTE: The base state should only be applied if there's some existing // document to override, so use a Precondition of exists=true @@ -284,9 +285,10 @@ public LocalDocumentsResult writeLocally(List mutations) { MutationBatch batch = mutationQueue.addMutationBatch(localWriteTime, baseMutations, mutations); Map overlays = - batch.applyToLocalDocumentSet(documents, docsWithoutRemoteVersion); + batch.applyToLocalDocumentSet(overlayedDocuments, docsWithoutRemoteVersion); documentOverlayCache.saveOverlays(batch.getBatchId(), overlays); - return new LocalDocumentsResult(batch.getBatchId(), documents); + return LocalDocumentsResult.fromOverlayedDocuments( + batch.getBatchId(), overlayedDocuments); }); } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java index 886036d5346..0111f0324ff 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java @@ -19,7 +19,7 @@ import androidx.annotation.Nullable; import com.google.firebase.Timestamp; import com.google.firebase.database.collection.ImmutableSortedMap; -import com.google.firebase.firestore.model.Document; +import com.google.firebase.firestore.local.OverlayedDocument; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.MutableDocument; import com.google.firebase.firestore.model.SnapshotVersion; @@ -136,7 +136,7 @@ public FieldMask applyToLocalView(MutableDocument document, @Nullable FieldMask * applications. */ public Map applyToLocalDocumentSet( - ImmutableSortedMap documentMap, + ImmutableSortedMap documentMap, Set documentsWithoutRemoteVersion) { // TODO(mrschmidt): This implementation is O(n^2). If we iterate through the mutations first // (as done in `applyToLocalView(MutableDocument d)`), we can reduce the complexity to @@ -145,8 +145,8 @@ public Map applyToLocalDocumentSet( for (DocumentKey key : getKeys()) { // TODO(mutabledocuments): This method should take a map of MutableDocuments and we should // remove this cast. - MutableDocument document = (MutableDocument) documentMap.get(key); - FieldMask mutatedFields = applyToLocalView(document); + MutableDocument document = (MutableDocument) documentMap.get(key).getOverlay(); + FieldMask mutatedFields = applyToLocalView(document, documentMap.get(key).getMutatedFields()); // Set mutationFields 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.contains(key) ? null : mutatedFields; diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Overlay.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Overlay.java index c88648e1bda..c83ebe4ae67 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Overlay.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Overlay.java @@ -36,4 +36,10 @@ public static Overlay create(int largestBatchId, Mutation mutation) { public DocumentKey getKey() { return getMutation().getKey(); } + + public FieldMask getFieldMask() { + return getMutation() instanceof PatchMutation + ? ((PatchMutation) getMutation()).getMask() + : null; + } } diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java index 87c8b9a1aac..219d81bd2d3 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java @@ -1616,4 +1616,47 @@ public void testCanHandleBatchAckWhenPendingBatchesHaveOtherDocs() { assertContains(doc("foo/bar", 0, map("foo", "bar-set")).setHasLocalMutations()); assertContains(doc("foo/another", 0, map("foo", "another")).setHasLocalMutations()); } + + @Test + public void testMultipleFieldPatchesOnRemoteDocs() { + Query query = query("foo"); + allocateQuery(query); + assertTargetId(2); + + applyRemoteEvent( + addedRemoteEvent(doc("foo/bar", 1, map("likes", 0, "stars", 0)), asList(2), emptyList())); + assertChanged(doc("foo/bar", 1, map("likes", 0, "stars", 0))); + assertContains(doc("foo/bar", 1, map("likes", 0, "stars", 0))); + + writeMutation(patchMutation("foo/bar", map("likes", 1))); + assertChanged(doc("foo/bar", 1, map("likes", 1, "stars", 0)).setHasLocalMutations()); + assertContains(doc("foo/bar", 1, map("likes", 1, "stars", 0)).setHasLocalMutations()); + + writeMutation(patchMutation("foo/bar", map("stars", 1))); + assertChanged(doc("foo/bar", 1, map("likes", 1, "stars", 1)).setHasLocalMutations()); + assertContains(doc("foo/bar", 1, map("likes", 1, "stars", 1)).setHasLocalMutations()); + + writeMutation(patchMutation("foo/bar", map("stars", 2))); + assertChanged(doc("foo/bar", 1, map("likes", 1, "stars", 2)).setHasLocalMutations()); + assertContains(doc("foo/bar", 1, map("likes", 1, "stars", 2)).setHasLocalMutations()); + } + + @Test + public void testMultipleFieldPatchesOnLocalDocs() { + writeMutation(setMutation("foo/bar", map("likes", 0, "stars", 0))); + assertChanged(doc("foo/bar", 0, map("likes", 0, "stars", 0)).setHasLocalMutations()); + assertContains(doc("foo/bar", 0, map("likes", 0, "stars", 0)).setHasLocalMutations()); + + writeMutation(patchMutation("foo/bar", map("likes", 1))); + assertChanged(doc("foo/bar", 0, map("likes", 1, "stars", 0)).setHasLocalMutations()); + assertContains(doc("foo/bar", 0, map("likes", 1, "stars", 0)).setHasLocalMutations()); + + writeMutation(patchMutation("foo/bar", map("stars", 1))); + assertChanged(doc("foo/bar", 0, map("likes", 1, "stars", 1)).setHasLocalMutations()); + assertContains(doc("foo/bar", 0, map("likes", 1, "stars", 1)).setHasLocalMutations()); + + writeMutation(patchMutation("foo/bar", map("stars", 2))); + assertChanged(doc("foo/bar", 0, map("likes", 1, "stars", 2)).setHasLocalMutations()); + assertContains(doc("foo/bar", 0, map("likes", 1, "stars", 2)).setHasLocalMutations()); + } } From 955e312cb74c0a0aeb4caa20146c7ff39b3d01f6 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Wed, 6 Apr 2022 15:09:34 -0400 Subject: [PATCH 02/10] Add missing file. --- .../google/firebase/firestore/QueryTest.java | 52 ------------------- .../firestore/local/OverlayedDocument.java | 22 ++++++++ 2 files changed, 22 insertions(+), 52 deletions(-) create mode 100644 firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java index a46436e35d8..1f4ed810eaa 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java @@ -920,56 +920,4 @@ public void testMultipleUpdatesWhileOffline() { QuerySnapshot snapshot2 = waitFor(collection.get(Source.CACHE)); assertEquals(asList(map("foo", "zzyzx", "bar", "2")), querySnapshotToValues(snapshot2)); } - - @Test - public void testMultipleUpdatesWhileOfflineOnRemoteDoc() { - CollectionReference collection = testCollection(); - DocumentReference doc = collection.document(); - doc.set(map("likes", 0, "stars", 0)); - - collection.getFirestore().disableNetwork(); - - doc.update("likes", 1); - QuerySnapshot snapshot1 = waitFor(collection.get(Source.CACHE)); - List result1 = querySnapshotToValues(snapshot1); - - doc.update("stars", 1); - snapshot1 = waitFor(collection.get(Source.CACHE)); - List result2 = querySnapshotToValues(snapshot1); - - doc.update("stars", 2); - snapshot1 = waitFor(collection.get(Source.CACHE)); - List result3 = querySnapshotToValues(snapshot1); - - result3.contains(0); - } - - @Test - public void testListenToMultipleUpdatesWhileOffline() { - CollectionReference collection = testCollection(); - collection.addSnapshotListener( - (snap, e) -> { - List result = querySnapshotToValues(snap); - result.contains(0); - }); - DocumentReference doc = collection.document(); - doc.set(map("likes", 0, "stars", 0)); - waitFor(doc.set(map("likes", 0, "stars", 0))); - - collection.getFirestore().disableNetwork(); - - doc.update("likes", 1); - QuerySnapshot snapshot1 = waitFor(collection.get(Source.CACHE)); - List result1 = querySnapshotToValues(snapshot1); - - doc.update("stars", 1); - snapshot1 = waitFor(collection.get(Source.CACHE)); - List result2 = querySnapshotToValues(snapshot1); - - doc.update("stars", 2); - snapshot1 = waitFor(collection.get(Source.CACHE)); - List result3 = querySnapshotToValues(snapshot1); - - result3.contains(0); - } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java new file mode 100644 index 00000000000..588cca303dc --- /dev/null +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java @@ -0,0 +1,22 @@ +package com.google.firebase.firestore.local; + +import com.google.firebase.firestore.model.Document; +import com.google.firebase.firestore.model.mutation.FieldMask; + +public class OverlayedDocument { + private Document overlay; + private FieldMask mutatedFields; + + OverlayedDocument(Document overlay, FieldMask mutatedFields) { + this.overlay = overlay; + this.mutatedFields = mutatedFields; + } + + public Document getOverlay() { + return overlay; + } + + public FieldMask getMutatedFields() { + return mutatedFields; + } +} From 173e8a8a69155befff1e88863e8c7b9133d4d07b Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Wed, 6 Apr 2022 15:10:36 -0400 Subject: [PATCH 03/10] Fixing header --- .../firestore/local/OverlayedDocument.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java index 588cca303dc..b0668cd25dd 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java @@ -1,3 +1,17 @@ +// Copyright 2021 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package com.google.firebase.firestore.local; import com.google.firebase.firestore.model.Document; From c0df56fd3b952727b2ac66ba3ccbd97252a7cbca Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Wed, 6 Apr 2022 20:44:09 -0400 Subject: [PATCH 04/10] A little refactor --- .../firestore/local/LocalDocumentsResult.java | 6 ++--- .../firestore/local/LocalDocumentsView.java | 23 +++++++++---------- .../firebase/firestore/local/LocalStore.java | 6 ++--- .../firestore/local/OverlayedDocument.java | 13 +++++++++-- .../model/mutation/MutationBatch.java | 4 ++-- 5 files changed, 30 insertions(+), 22 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsResult.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsResult.java index a1742c0908f..382bf558b98 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsResult.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsResult.java @@ -37,10 +37,10 @@ public final class LocalDocumentsResult { } public static LocalDocumentsResult fromOverlayedDocuments( - int batchId, ImmutableSortedMap overlays) { + int batchId, Map overlays) { ImmutableSortedMap documents = emptyDocumentMap(); - for (Map.Entry entry : overlays) { - documents = documents.insert(entry.getKey(), entry.getValue().getOverlay()); + for (Map.Entry entry : overlays.entrySet()) { + documents = documents.insert(entry.getKey(), entry.getValue().getDocument()); } return new LocalDocumentsResult(batchId, documents); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java index 9a7f91a0bf4..e3354843e98 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java @@ -121,20 +121,21 @@ ImmutableSortedMap getLocalViewOfDocuments( populateOverlays(overlays, docs.keySet()); ImmutableSortedMap result = emptyDocumentMap(); for (Map.Entry entry : - computeViews(docs, overlays, existenceStateChanged)) { - result = result.insert(entry.getKey(), entry.getValue().getOverlay()); + computeViews(docs, overlays, existenceStateChanged).entrySet()) { + result = result.insert(entry.getKey(), entry.getValue().getDocument()); } return result; } /** - * Similar to {@link #getDocuments}, but creates the local view from the given {@code baseDocs} - * without retrieving documents from the local store. + * Gets the overlayed documents for the given document map, which will include the local view of + * those documents and a {@code FieldMask} indicating which fields are mutated locally, null if + * overlay is a Set or Delete mutation. * * @param docs The documents to apply local mutations to get the local views. */ - ImmutableSortedMap getLocalViewOfDocuments( + Map getOverlayedDocuments( Map docs) { Map overlays = new HashMap<>(); populateOverlays(overlays, docs.keySet()); @@ -142,12 +143,10 @@ ImmutableSortedMap getLocalViewOfDocuments( } /*Computes the local view for doc */ - private ImmutableSortedMap computeViews( + private Map computeViews( Map docs, Map overlays, Set existenceStateChanged) { - ImmutableSortedMap results = - ImmutableSortedMap.Builder.emptyMap(DocumentKey.comparator()); Map recalculateDocuments = new HashMap<>(); Map mutatedFields = new HashMap<>(); for (MutableDocument doc : docs.values()) { @@ -171,13 +170,13 @@ private ImmutableSortedMap computeViews( recalculateAndSaveOverlays(recalculateDocuments); mutatedFields.putAll(recalculatedFields); + Map result = new HashMap<>(); for (Map.Entry entry : docs.entrySet()) { - results = - results.insert( + result.put( entry.getKey(), new OverlayedDocument(entry.getValue(), mutatedFields.get(entry.getKey()))); } - return results; + return result; } private Map recalculateAndSaveOverlays( @@ -328,7 +327,7 @@ LocalDocumentsResult getNextDocuments(String collectionGroup, IndexOffset offset } populateOverlays(overlays, docs.keySet()); - ImmutableSortedMap localDocs = + Map localDocs = computeViews(docs, overlays, Collections.emptySet()); return LocalDocumentsResult.fromOverlayedDocuments(largestBatchId, localDocs); } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java index b5cad63cf84..beeccb6b89a 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java @@ -258,8 +258,8 @@ public LocalDocumentsResult writeLocally(List mutations) { } // 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. - ImmutableSortedMap overlayedDocuments = - localDocuments.getLocalViewOfDocuments(remoteDocs); + Map overlayedDocuments = + localDocuments.getOverlayedDocuments(remoteDocs); // For non-idempotent mutations (such as `FieldValue.increment()`), we record the base // state in a separate patch mutation. This is later used to guarantee consistent values @@ -269,7 +269,7 @@ public LocalDocumentsResult writeLocally(List mutations) { for (Mutation mutation : mutations) { ObjectValue baseValue = mutation.extractTransformBaseValue( - overlayedDocuments.get(mutation.getKey()).getOverlay()); + overlayedDocuments.get(mutation.getKey()).getDocument()); if (baseValue != null) { // NOTE: The base state should only be applied if there's some existing // document to override, so use a Precondition of exists=true diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java index b0668cd25dd..d2dddb763d3 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java @@ -17,6 +17,11 @@ import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.mutation.FieldMask; +import javax.annotation.Nullable; + +/** + * Represents a local view (overlay) of a document, and the fields that are locally mutated. + */ public class OverlayedDocument { private Document overlay; private FieldMask mutatedFields; @@ -26,11 +31,15 @@ public class OverlayedDocument { this.mutatedFields = mutatedFields; } - public Document getOverlay() { + public Document getDocument() { return overlay; } - public FieldMask getMutatedFields() { + /** + * The fields that are locally mutated by patch mutations. If the overlayed document is from set + * or delete mutations, this returns null. + */ + public @Nullable FieldMask getMutatedFields() { return mutatedFields; } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java index 0111f0324ff..c3289b47366 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java @@ -136,7 +136,7 @@ public FieldMask applyToLocalView(MutableDocument document, @Nullable FieldMask * applications. */ public Map applyToLocalDocumentSet( - ImmutableSortedMap documentMap, + Map documentMap, Set documentsWithoutRemoteVersion) { // TODO(mrschmidt): This implementation is O(n^2). If we iterate through the mutations first // (as done in `applyToLocalView(MutableDocument d)`), we can reduce the complexity to @@ -145,7 +145,7 @@ public Map applyToLocalDocumentSet( for (DocumentKey key : getKeys()) { // TODO(mutabledocuments): This method should take a map of MutableDocuments and we should // remove this cast. - MutableDocument document = (MutableDocument) documentMap.get(key).getOverlay(); + MutableDocument document = (MutableDocument) documentMap.get(key).getDocument(); FieldMask mutatedFields = applyToLocalView(document, documentMap.get(key).getMutatedFields()); // Set mutationFields 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. From 270aab83bf7c2e2b293202158a17ce5d259342c6 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Wed, 6 Apr 2022 20:47:17 -0400 Subject: [PATCH 05/10] 2022 --- .../com/google/firebase/firestore/local/OverlayedDocument.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java index d2dddb763d3..a816d0c9f80 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java @@ -1,4 +1,4 @@ -// Copyright 2021 Google LLC +// Copyright 2022 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 3c3165524fc591612889cbd5362181b844c166ec Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Wed, 6 Apr 2022 21:03:00 -0400 Subject: [PATCH 06/10] Format --- .../google/firebase/firestore/local/LocalDocumentsView.java | 6 +++--- .../google/firebase/firestore/local/OverlayedDocument.java | 5 +---- .../firebase/firestore/model/mutation/MutationBatch.java | 1 - 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java index e3354843e98..d57f13c9103 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java @@ -172,9 +172,9 @@ private Map computeViews( Map result = new HashMap<>(); for (Map.Entry entry : docs.entrySet()) { - result.put( - entry.getKey(), - new OverlayedDocument(entry.getValue(), mutatedFields.get(entry.getKey()))); + result.put( + entry.getKey(), + new OverlayedDocument(entry.getValue(), mutatedFields.get(entry.getKey()))); } return result; } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java index a816d0c9f80..784a61f3670 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java @@ -16,12 +16,9 @@ import com.google.firebase.firestore.model.Document; import com.google.firebase.firestore.model.mutation.FieldMask; - import javax.annotation.Nullable; -/** - * Represents a local view (overlay) of a document, and the fields that are locally mutated. - */ +/** Represents a local view (overlay) of a document, and the fields that are locally mutated. */ public class OverlayedDocument { private Document overlay; private FieldMask mutatedFields; diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java index c3289b47366..c857cd4365d 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java @@ -18,7 +18,6 @@ import androidx.annotation.Nullable; import com.google.firebase.Timestamp; -import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.firestore.local.OverlayedDocument; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.MutableDocument; From bae85347ad3c26c43f69349a9bd9f5fea121e5f8 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Thu, 7 Apr 2022 09:32:19 -0400 Subject: [PATCH 07/10] Add one more test --- .../firestore/local/LocalStoreTestCase.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java index 219d81bd2d3..101ff47877a 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java @@ -1641,6 +1641,28 @@ public void testMultipleFieldPatchesOnRemoteDocs() { assertContains(doc("foo/bar", 1, map("likes", 1, "stars", 2)).setHasLocalMutations()); } + @Test + public void testMultipleFieldPatchesInOneBatchOnRemoteDocs() { + Query query = query("foo"); + allocateQuery(query); + assertTargetId(2); + + applyRemoteEvent( + addedRemoteEvent(doc("foo/bar", 1, map("likes", 0, "stars", 0)), asList(2), emptyList())); + assertChanged(doc("foo/bar", 1, map("likes", 0, "stars", 0))); + assertContains(doc("foo/bar", 1, map("likes", 0, "stars", 0))); + + writeMutations( + Lists.newArrayList( + patchMutation("foo/bar", map("likes", 1)), patchMutation("foo/bar", map("stars", 1)))); + assertChanged(doc("foo/bar", 1, map("likes", 1, "stars", 1)).setHasLocalMutations()); + assertContains(doc("foo/bar", 1, map("likes", 1, "stars", 1)).setHasLocalMutations()); + + writeMutation(patchMutation("foo/bar", map("stars", 2))); + assertChanged(doc("foo/bar", 1, map("likes", 1, "stars", 2)).setHasLocalMutations()); + assertContains(doc("foo/bar", 1, map("likes", 1, "stars", 2)).setHasLocalMutations()); + } + @Test public void testMultipleFieldPatchesOnLocalDocs() { writeMutation(setMutation("foo/bar", map("likes", 0, "stars", 0))); From 356f66177abdcccb845e2a107bead9601ca58a52 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Fri, 8 Apr 2022 12:08:03 -0400 Subject: [PATCH 08/10] Feedback --- .../firestore/local/LocalDocumentsView.java | 13 +++++++++++-- .../firebase/firestore/local/OverlayedDocument.java | 8 ++++---- .../firestore/model/mutation/DeleteMutation.java | 5 +++++ .../firebase/firestore/model/mutation/Mutation.java | 6 ++++++ .../firestore/model/mutation/MutationBatch.java | 11 ++++------- .../firebase/firestore/model/mutation/Overlay.java | 6 ------ .../firestore/model/mutation/PatchMutation.java | 3 ++- .../firestore/model/mutation/SetMutation.java | 5 +++++ .../firestore/model/mutation/VerifyMutation.java | 5 +++++ .../firebase/firestore/remote/RemoteSerializer.java | 2 +- 10 files changed, 43 insertions(+), 21 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java index d57f13c9103..0c4ce14b79b 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java @@ -142,7 +142,16 @@ Map getOverlayedDocuments( return computeViews(docs, overlays, new HashSet<>()); } - /*Computes the local view for doc */ + /** + * Computes the local view for the given documents. + * + * @param docs The documents to compute views for. It also has the base version of the documents. + * @param overlays The overlays that need to be applied to the given base version of the + * documents. + * @param existenceStateChanged A set of documents that might there existence states changed. This + * is used to determine if we need to re-calculate overlays from mutation queues. + * @return A map represents the local documents view. + */ private Map computeViews( Map docs, Map overlays, @@ -161,7 +170,7 @@ private Map computeViews( && (overlay == null || overlay.getMutation() instanceof PatchMutation)) { recalculateDocuments.put(doc.getKey(), doc); } else if (overlay != null) { - mutatedFields.put(doc.getKey(), overlay.getFieldMask()); + mutatedFields.put(doc.getKey(), overlay.getMutation().getFieldMask()); overlay.getMutation().applyToLocalView(doc, null, Timestamp.now()); } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java index 784a61f3670..50a82e6c94f 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/OverlayedDocument.java @@ -20,16 +20,16 @@ /** Represents a local view (overlay) of a document, and the fields that are locally mutated. */ public class OverlayedDocument { - private Document overlay; + private Document overlayedDocument; private FieldMask mutatedFields; - OverlayedDocument(Document overlay, FieldMask mutatedFields) { - this.overlay = overlay; + OverlayedDocument(Document overlayedDocument, FieldMask mutatedFields) { + this.overlayedDocument = overlayedDocument; this.mutatedFields = mutatedFields; } public Document getDocument() { - return overlay; + return overlayedDocument; } /** diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/DeleteMutation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/DeleteMutation.java index ffae5ab8063..89a0c3f38e8 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/DeleteMutation.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/DeleteMutation.java @@ -79,4 +79,9 @@ public void applyToRemoteDocument(MutableDocument document, MutationResult mutat return previousMask; } + + @Override + public @Nullable FieldMask getFieldMask() { + return null; + } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Mutation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Mutation.java index 26f2ccea4f2..c979c9c809c 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Mutation.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Mutation.java @@ -164,6 +164,12 @@ public abstract void applyToRemoteDocument( public abstract @Nullable FieldMask applyToLocalView( MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime); + /** + * Returns a {@code FieldMask} representing the fields that will be changed by applying this + * mutation. Returns {@code null} if the mutation will overwrite the entire document. + */ + public abstract @Nullable FieldMask getFieldMask(); + /** Helper for derived classes to implement .equals(). */ boolean hasSameKeyAndPrecondition(Mutation other) { return key.equals(other.key) && precondition.equals(other.precondition); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java index c857cd4365d..b1b486cd7be 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java @@ -100,14 +100,11 @@ public void applyToRemoteDocument(MutableDocument document, MutationBatchResult } /** - * Computes the local view of a document given all the mutations in this batch. Returns an {@link - * FieldMask} representing all the fields that are mutated. + * Computes the local view of a document given all the mutations in this batch. + * + * @param mutatedFields Fields that are already mutated before applying the current one. + * @return An {@link FieldMask} representing all the fields that are mutated. */ - public FieldMask applyToLocalView(MutableDocument document) { - FieldMask mutatedFields = FieldMask.fromSet(new HashSet<>()); - return applyToLocalView(document, mutatedFields); - } - public FieldMask applyToLocalView(MutableDocument document, @Nullable FieldMask mutatedFields) { // First, apply the base state. This allows us to apply non-idempotent transform against a // consistent set of values. diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Overlay.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Overlay.java index c83ebe4ae67..c88648e1bda 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Overlay.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Overlay.java @@ -36,10 +36,4 @@ public static Overlay create(int largestBatchId, Mutation mutation) { public DocumentKey getKey() { return getMutation().getKey(); } - - public FieldMask getFieldMask() { - return getMutation() instanceof PatchMutation - ? ((PatchMutation) getMutation()).getMask() - : null; - } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java index 2ee6b9e0404..c35eb66ffb0 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java @@ -101,7 +101,8 @@ public ObjectValue getValue() { * Returns the mask to apply to {@link #getValue}, where only fields that are in both the * fieldMask and the value will be updated. */ - public FieldMask getMask() { + @Override + public FieldMask getFieldMask() { return mask; } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/SetMutation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/SetMutation.java index cd5643b13da..d377781c931 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/SetMutation.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/SetMutation.java @@ -103,6 +103,11 @@ public FieldMask applyToLocalView( return null; } + @Override + public @Nullable FieldMask getFieldMask() { + return null; + } + /** Returns the object value to use when setting the document. */ public ObjectValue getValue() { return value; diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/VerifyMutation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/VerifyMutation.java index 98029b5fe31..941b0f8dceb 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/VerifyMutation.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/VerifyMutation.java @@ -66,4 +66,9 @@ public FieldMask applyToLocalView( MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime) { throw Assert.fail("VerifyMutation should only be used in Transactions."); } + + @Override + public @Nullable FieldMask getFieldMask() { + return null; + } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java index cc95d4270a3..cd8f6872b23 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java @@ -269,7 +269,7 @@ public com.google.firestore.v1.Write encodeMutation(Mutation mutation) { builder.setUpdate(encodeDocument(mutation.getKey(), ((SetMutation) mutation).getValue())); } else if (mutation instanceof PatchMutation) { builder.setUpdate(encodeDocument(mutation.getKey(), ((PatchMutation) mutation).getValue())); - builder.setUpdateMask(encodeDocumentMask(((PatchMutation) mutation).getMask())); + builder.setUpdateMask(encodeDocumentMask(((PatchMutation) mutation).getFieldMask())); } else if (mutation instanceof DeleteMutation) { builder.setDelete(encodeKey(mutation.getKey())); } else if (mutation instanceof VerifyMutation) { From c1938915c82b227f57a917fae103441cae2c5bf9 Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Fri, 8 Apr 2022 12:14:57 -0400 Subject: [PATCH 09/10] CHANGELOG --- firebase-firestore/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/firebase-firestore/CHANGELOG.md b/firebase-firestore/CHANGELOG.md index f81e8af6bc7..e75953b1355 100644 --- a/firebase-firestore/CHANGELOG.md +++ b/firebase-firestore/CHANGELOG.md @@ -3,6 +3,9 @@ by opting into a release at [go/firebase-android-release](http:go/firebase-android-release) (Googlers only). # Unreleased +- [fixed] Fixed an issue where patching multiple fields shadows each other (#3528). + +# 24.1.1 - [fixed] Fixed an issue in the experimental index engine that might have caused Firestore to exclude document results for limit queries with local modifications. From f8d79c285fd9541fc7b1cab35e0b0ae82fb58a1b Mon Sep 17 00:00:00 2001 From: Wu-Hui Date: Mon, 11 Apr 2022 11:56:11 -0400 Subject: [PATCH 10/10] Feedback. --- .../com/google/firebase/firestore/local/LocalDocumentsView.java | 2 +- .../google/firebase/firestore/model/mutation/MutationBatch.java | 1 + .../com/google/firebase/firestore/remote/RemoteSerializer.java | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java index 0c4ce14b79b..1ffab1aa3f9 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java @@ -148,7 +148,7 @@ Map getOverlayedDocuments( * @param docs The documents to compute views for. It also has the base version of the documents. * @param overlays The overlays that need to be applied to the given base version of the * documents. - * @param existenceStateChanged A set of documents that might there existence states changed. This + * @param existenceStateChanged A set of documents whose existence states might have changed. This * is used to determine if we need to re-calculate overlays from mutation queues. * @return A map represents the local documents view. */ diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java index b1b486cd7be..7fb8d48c2ad 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java @@ -102,6 +102,7 @@ public void applyToRemoteDocument(MutableDocument document, MutationBatchResult /** * Computes the local view of a document given all the mutations in this batch. * + * @param mutatedFields The document to be mutated. * @param mutatedFields Fields that are already mutated before applying the current one. * @return An {@link FieldMask} representing all the fields that are mutated. */ diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java index cd8f6872b23..bbaa103612a 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java @@ -269,7 +269,7 @@ public com.google.firestore.v1.Write encodeMutation(Mutation mutation) { builder.setUpdate(encodeDocument(mutation.getKey(), ((SetMutation) mutation).getValue())); } else if (mutation instanceof PatchMutation) { builder.setUpdate(encodeDocument(mutation.getKey(), ((PatchMutation) mutation).getValue())); - builder.setUpdateMask(encodeDocumentMask(((PatchMutation) mutation).getFieldMask())); + builder.setUpdateMask(encodeDocumentMask((mutation.getFieldMask()))); } else if (mutation instanceof DeleteMutation) { builder.setDelete(encodeKey(mutation.getKey())); } else if (mutation instanceof VerifyMutation) {