From 563d878d0f852f69cafe7353e079596600279564 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Feb 2021 11:37:33 -0700 Subject: [PATCH 1/2] Make TransformResult non-nullable Gil asked that this should not be part of #2383, but I think it still makes sense as an indepedent simplification --- .../firestore/model/mutation/DeleteMutation.java | 2 +- .../firestore/model/mutation/MutationResult.java | 7 +++---- .../firebase/firestore/model/mutation/PatchMutation.java | 6 +----- .../firebase/firestore/model/mutation/SetMutation.java | 9 +++------ .../firebase/firestore/remote/RemoteSerializer.java | 9 +++------ .../firebase/firestore/local/LocalStoreTestCase.java | 2 +- .../google/firebase/firestore/model/MutationTest.java | 3 +-- .../com/google/firebase/firestore/spec/SpecTestCase.java | 2 +- .../com/google/firebase/firestore/testutil/TestUtil.java | 2 +- 9 files changed, 15 insertions(+), 27 deletions(-) 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 177e02fb944..74f36a4b2ed 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 @@ -59,7 +59,7 @@ public MaybeDocument applyToRemoteDocument( verifyKeyMatches(maybeDoc); hardAssert( - mutationResult.getTransformResults() == null, + mutationResult.getTransformResults().isEmpty(), "Transform results received by DeleteMutation."); // Unlike applyToLocalView, if we're applying a mutation to a remote document the server has diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationResult.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationResult.java index 7c76973d6c0..236011fba9c 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationResult.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationResult.java @@ -31,9 +31,9 @@ */ public final class MutationResult { private final SnapshotVersion version; - @Nullable private final List transformResults; + private final List transformResults; - public MutationResult(SnapshotVersion version, @Nullable List transformResults) { + public MutationResult(SnapshotVersion version, List transformResults) { this.version = checkNotNull(version); this.transformResults = transformResults; } @@ -58,9 +58,8 @@ public SnapshotVersion getVersion() { * The resulting fields returned from the backend after a mutation containing field transforms has * been committed. Contains one Value for each FieldTransform that was in the mutation. * - *

Will be null if the mutation did not contain any field transforms. + *

Returns an empty list if the mutation did not contain any field transforms. */ - @Nullable public List getTransformResults() { return transformResults; } 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 a1a31080432..6214663c8c5 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 @@ -117,11 +117,7 @@ public MaybeDocument applyToRemoteDocument( return new UnknownDocument(this.getKey(), mutationResult.getVersion()); } - List transformResults = - mutationResult.getTransformResults() != null - ? serverTransformResults(maybeDoc, mutationResult.getTransformResults()) - : new ArrayList<>(); - + List transformResults = serverTransformResults(maybeDoc, mutationResult.getTransformResults()); SnapshotVersion version = mutationResult.getVersion(); ObjectValue newData = patchDocument(maybeDoc, transformResults); return new Document(getKey(), version, newData, Document.DocumentState.COMMITTED_MUTATIONS); 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 6e67dfea864..5a15c9bc191 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 @@ -81,12 +81,9 @@ public MaybeDocument applyToRemoteDocument( SnapshotVersion version = mutationResult.getVersion(); - ObjectValue newData = value; - if (mutationResult.getTransformResults() != null) { - List transformResults = - serverTransformResults(maybeDoc, mutationResult.getTransformResults()); - newData = transformObject(newData, transformResults); - } + List transformResults = + serverTransformResults(maybeDoc, mutationResult.getTransformResults()); + ObjectValue newData = transformObject(value, transformResults); return new Document(getKey(), version, newData, Document.DocumentState.COMMITTED_MUTATIONS); } 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 883d01c0cf7..cee94657701 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 @@ -445,13 +445,10 @@ public MutationResult decodeMutationResult( version = commitVersion; } - List transformResults = null; int transformResultsCount = proto.getTransformResultsCount(); - if (transformResultsCount > 0) { - transformResults = new ArrayList<>(transformResultsCount); - for (int i = 0; i < transformResultsCount; i++) { - transformResults.add(proto.getTransformResults(i)); - } + List transformResults = new ArrayList<>(transformResultsCount); + for (int i = 0; i < transformResultsCount; i++) { + transformResults.add(proto.getTransformResults(i)); } return new MutationResult(version, transformResults); } 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 30088f25aa2..ad6aced6847 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 @@ -159,7 +159,7 @@ private void acknowledgeMutation(long documentVersion, @Nullable Object transfor version, transformResult != null ? Collections.singletonList(TestUtil.wrap(transformResult)) - : null); + : Collections.emptyList()); MutationBatchResult result = MutationBatchResult.create( batch, version, singletonList(mutationResult), WriteStream.EMPTY_STREAM_TOKEN); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java index b73f5fb77a2..129b9b64c00 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java @@ -597,8 +597,7 @@ public void testTransitions() { doc("collection/key", 7, map(), Document.DocumentState.COMMITTED_MUTATIONS); UnknownDocument docV7Unknown = unknownDoc("collection/key", 7); - MutationResult mutationResult = new MutationResult(version(7), /*transformResults=*/ null); - MutationResult transformResult = new MutationResult(version(7), Collections.emptyList()); + MutationResult mutationResult = new MutationResult(version(7), /*transformResults=*/ Collections.emptyList()); assertVersionTransitions(set, docV3, mutationResult, docV7Committed); assertVersionTransitions(set, deletedV3, mutationResult, docV7Committed); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/SpecTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/SpecTestCase.java index ed6acf79235..7baabab6ff4 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/SpecTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/spec/SpecTestCase.java @@ -712,7 +712,7 @@ private void doWriteAck(JSONObject writeAckSpec) throws Exception { validateNextWriteSent(write.first); MutationResult mutationResult = - new MutationResult(version(version), /*transformResults=*/ null); + new MutationResult(version(version), /*transformResults=*/ Collections.emptyList()); queue.runSync(() -> datastore.ackWrite(version(version), singletonList(mutationResult))); } diff --git a/firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java b/firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java index 91f159007d2..9bad07314d6 100644 --- a/firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java +++ b/firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java @@ -550,7 +550,7 @@ public static VerifyMutation verifyMutation(String path, int micros) { } public static MutationResult mutationResult(long version) { - return new MutationResult(version(version), null); + return new MutationResult(version(version), Collections.emptyList()); } public static LocalViewChanges viewChanges( From 7bfed6c002fb50963f386bbd838b0bf3609444a3 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 9 Feb 2021 12:06:31 -0700 Subject: [PATCH 2/2] Format --- .../firebase/firestore/model/mutation/MutationResult.java | 1 - .../firebase/firestore/model/mutation/PatchMutation.java | 3 ++- .../google/firebase/firestore/model/mutation/SetMutation.java | 2 +- .../java/com/google/firebase/firestore/model/MutationTest.java | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationResult.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationResult.java index 236011fba9c..bef1799c0f7 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationResult.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationResult.java @@ -16,7 +16,6 @@ import static com.google.firebase.firestore.util.Preconditions.checkNotNull; -import androidx.annotation.Nullable; import com.google.firebase.firestore.model.SnapshotVersion; import com.google.firestore.v1.Value; import java.util.List; 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 6214663c8c5..91ccf43585f 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 @@ -117,7 +117,8 @@ public MaybeDocument applyToRemoteDocument( return new UnknownDocument(this.getKey(), mutationResult.getVersion()); } - List transformResults = serverTransformResults(maybeDoc, mutationResult.getTransformResults()); + List transformResults = + serverTransformResults(maybeDoc, mutationResult.getTransformResults()); SnapshotVersion version = mutationResult.getVersion(); ObjectValue newData = patchDocument(maybeDoc, transformResults); return new Document(getKey(), version, newData, Document.DocumentState.COMMITTED_MUTATIONS); 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 5a15c9bc191..7cca0e44ee7 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 @@ -82,7 +82,7 @@ public MaybeDocument applyToRemoteDocument( SnapshotVersion version = mutationResult.getVersion(); List transformResults = - serverTransformResults(maybeDoc, mutationResult.getTransformResults()); + serverTransformResults(maybeDoc, mutationResult.getTransformResults()); ObjectValue newData = transformObject(value, transformResults); return new Document(getKey(), version, newData, Document.DocumentState.COMMITTED_MUTATIONS); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java index 129b9b64c00..a3c79222c54 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java @@ -597,7 +597,8 @@ public void testTransitions() { doc("collection/key", 7, map(), Document.DocumentState.COMMITTED_MUTATIONS); UnknownDocument docV7Unknown = unknownDoc("collection/key", 7); - MutationResult mutationResult = new MutationResult(version(7), /*transformResults=*/ Collections.emptyList()); + MutationResult mutationResult = + new MutationResult(version(7), /*transformResults=*/ Collections.emptyList()); assertVersionTransitions(set, docV3, mutationResult, docV7Committed); assertVersionTransitions(set, deletedV3, mutationResult, docV7Committed);