diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/NumericTransformsTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/NumericTransformsTest.java new file mode 100644 index 000000000000..5d78c2cd3f36 --- /dev/null +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/NumericTransformsTest.java @@ -0,0 +1,154 @@ +// Copyright 2018 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; + +import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testDocument; +import static com.google.firebase.firestore.testutil.IntegrationTestUtil.waitFor; +import static com.google.firebase.firestore.testutil.TestUtil.map; +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; + +import com.google.android.gms.tasks.Tasks; +import com.google.firebase.firestore.testutil.EventAccumulator; +import com.google.firebase.firestore.testutil.IntegrationTestUtil; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import org.junit.After; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; + +@Ignore("Not yet available in production") +public class NumericTransformsTest { + private static final double DOUBLE_EPSILON = 0.000001; + + // A document reference to read and write to. + private DocumentReference docRef; + + // Accumulator used to capture events during the test. + private EventAccumulator accumulator; + + // Listener registration for a listener maintained during the course of the test. + private ListenerRegistration listenerRegistration; + + @Before + public void setUp() { + docRef = testDocument(); + accumulator = new EventAccumulator<>(); + listenerRegistration = + docRef.addSnapshotListener(MetadataChanges.INCLUDE, accumulator.listener()); + + // Wait for initial null snapshot to avoid potential races. + DocumentSnapshot initialSnapshot = accumulator.await(); + assertFalse(initialSnapshot.exists()); + } + + @After + public void tearDown() { + listenerRegistration.remove(); + IntegrationTestUtil.tearDown(); + } + + /** Writes some initialData and consumes the events generated. */ + private void writeInitialData(Map initialData) { + waitFor(docRef.set(initialData)); + accumulator.awaitRemoteEvent(); + } + + private void expectLocalAndRemoteValue(double expectedSum) { + DocumentSnapshot snap = accumulator.awaitLocalEvent(); + assertEquals(expectedSum, snap.getDouble("sum"), DOUBLE_EPSILON); + snap = accumulator.awaitRemoteEvent(); + assertEquals(expectedSum, snap.getDouble("sum"), DOUBLE_EPSILON); + } + + private void expectLocalAndRemoteValue(long expectedSum) { + DocumentSnapshot snap = accumulator.awaitLocalEvent(); + assertEquals(expectedSum, (long) snap.getLong("sum")); + snap = accumulator.awaitRemoteEvent(); + assertEquals(expectedSum, (long) snap.getLong("sum")); + } + + @Test + public void createDocumentWithIncrement() { + waitFor(docRef.set(map("sum", FieldValue.numericAdd(1337)))); + expectLocalAndRemoteValue(1337L); + } + + @Test + public void integerIncrementExistingInteger() { + writeInitialData(map("sum", 1337L)); + waitFor(docRef.update("sum", FieldValue.numericAdd(1))); + expectLocalAndRemoteValue(1338L); + } + + @Test + public void doubleIncrementWithExistingDouble() { + writeInitialData(map("sum", 13.37D)); + waitFor(docRef.update("sum", FieldValue.numericAdd(0.1))); + expectLocalAndRemoteValue(13.47D); + } + + @Test + public void integerIncrementExistingDouble() { + writeInitialData(map("sum", 13.37D)); + waitFor(docRef.update("sum", FieldValue.numericAdd(1))); + expectLocalAndRemoteValue(14.37D); + } + + @Test + public void doubleIncrementExistingInteger() { + writeInitialData(map("sum", 1337L)); + waitFor(docRef.update("sum", FieldValue.numericAdd(0.1))); + expectLocalAndRemoteValue(1337.1D); + } + + @Test + public void integerIncrementExistingString() { + writeInitialData(map("sum", "overwrite")); + waitFor(docRef.update("sum", FieldValue.numericAdd(1337))); + expectLocalAndRemoteValue(1337L); + } + + @Test + public void doubleIncrementExistingString() { + writeInitialData(map("sum", "overwrite")); + waitFor(docRef.update("sum", FieldValue.numericAdd(13.37))); + expectLocalAndRemoteValue(13.37D); + } + + @Test + public void multipleDoubleIncrements() throws ExecutionException, InterruptedException { + writeInitialData(map("sum", 0.0D)); + + Tasks.await(docRef.getFirestore().disableNetwork()); + + docRef.update("sum", FieldValue.numericAdd(0.1D)); + docRef.update("sum", FieldValue.numericAdd(0.01D)); + docRef.update("sum", FieldValue.numericAdd(0.001D)); + + DocumentSnapshot snap = accumulator.awaitLocalEvent(); + assertEquals(0.1D, snap.getDouble("sum"), DOUBLE_EPSILON); + snap = accumulator.awaitLocalEvent(); + assertEquals(0.11D, snap.getDouble("sum"), DOUBLE_EPSILON); + snap = accumulator.awaitLocalEvent(); + assertEquals(0.111D, snap.getDouble("sum"), DOUBLE_EPSILON); + + Tasks.await(docRef.getFirestore().enableNetwork()); + + snap = accumulator.awaitRemoteEvent(); + assertEquals(0.111D, snap.getDouble("sum"), DOUBLE_EPSILON); + } +} diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java index c629b676b724..06cf36cdc1d1 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/testutil/IntegrationTestUtil.java @@ -20,6 +20,7 @@ import static junit.framework.Assert.fail; import android.content.Context; +import android.net.SSLCertificateSocketFactory; import android.support.test.InstrumentationRegistry; import com.google.android.gms.tasks.Task; import com.google.android.gms.tasks.TaskCompletionSource; @@ -38,10 +39,12 @@ import com.google.firebase.firestore.local.Persistence; import com.google.firebase.firestore.local.SQLitePersistence; import com.google.firebase.firestore.model.DatabaseId; +import com.google.firebase.firestore.remote.Datastore; import com.google.firebase.firestore.testutil.provider.FirestoreProvider; import com.google.firebase.firestore.util.AsyncQueue; import com.google.firebase.firestore.util.Logger; import com.google.firebase.firestore.util.Logger.Level; +import io.grpc.okhttp.OkHttpChannelBuilder; import java.io.File; import java.util.ArrayList; import java.util.HashMap; @@ -56,6 +59,13 @@ /** A set of helper methods for tests */ public class IntegrationTestUtil { + // Whether the integration tests should run against a local Firestore emulator instead of the + // Production environment. Note that the Android Emulator treats "10.0.2.2" as its host machine. + private static final boolean CONNECT_TO_EMULATOR = false; + + private static final String EMULATOR_HOST = "10.0.2.2"; + private static final int EMULATOR_PORT = 8081; + // Alternate project ID for creating "bad" references. Doesn't actually need to work. public static final String BAD_PROJECT_ID = "test-project-2"; @@ -63,7 +73,7 @@ public class IntegrationTestUtil { private static final Map firestoreStatus = new HashMap<>(); /** Default amount of time to wait for a given operation to complete, used by waitFor() helper. */ - private static final long OPERATION_WAIT_TIMEOUT_MS = 30000; + private static final long OPERATION_WAIT_TIMEOUT_MS = 300000; /** * Firestore databases can be subject to a ~30s "cold start" delay if they have not been used @@ -80,11 +90,19 @@ public static FirestoreProvider provider() { } public static DatabaseInfo testEnvDatabaseInfo() { - return new DatabaseInfo( - DatabaseId.forProject(provider.projectId()), - "test-persistenceKey", - provider.firestoreHost(), - /*sslEnabled=*/ true); + if (CONNECT_TO_EMULATOR) { + return new DatabaseInfo( + DatabaseId.forProject(provider.projectId()), + "test-persistenceKey", + String.format("%s:%d", EMULATOR_HOST, EMULATOR_PORT), + /*sslEnabled=*/ true); + } else { + return new DatabaseInfo( + DatabaseId.forProject(provider.projectId()), + "test-persistenceKey", + provider.firestoreHost(), + /*sslEnabled=*/ true); + } } public static FirebaseFirestoreSettings newTestSettings() { @@ -93,11 +111,33 @@ public static FirebaseFirestoreSettings newTestSettings() { public static FirebaseFirestoreSettings newTestSettingsWithSnapshotTimestampsEnabled( boolean enabled) { - return new FirebaseFirestoreSettings.Builder() - .setHost(provider.firestoreHost()) - .setPersistenceEnabled(true) - .setTimestampsInSnapshotsEnabled(enabled) - .build(); + FirebaseFirestoreSettings.Builder settings = new FirebaseFirestoreSettings.Builder(); + + if (CONNECT_TO_EMULATOR) { + settings.setHost(String.format("%s:%d", EMULATOR_HOST, EMULATOR_PORT)); + + // Disable SSL and hostname verification + OkHttpChannelBuilder channelBuilder = + new OkHttpChannelBuilder(EMULATOR_HOST, EMULATOR_PORT) { + @Override + protected String checkAuthority(String authority) { + return authority; + } + }; + channelBuilder.hostnameVerifier((hostname, session) -> true); + SSLCertificateSocketFactory insecureFactory = + (SSLCertificateSocketFactory) SSLCertificateSocketFactory.getInsecure(0, null); + channelBuilder.sslSocketFactory(insecureFactory); + + Datastore.overrideChannelBuilder(() -> channelBuilder); + } else { + settings.setHost(provider.firestoreHost()); + } + + settings.setPersistenceEnabled(true); + settings.setTimestampsInSnapshotsEnabled(enabled); + + return settings.build(); } /** Initializes a new Firestore instance that uses the default project. */ diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/FieldValue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/FieldValue.java index 0eb7780efbaf..a7bc8a3a3cd8 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/FieldValue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/FieldValue.java @@ -61,7 +61,7 @@ String getMethodName() { } List getElements() { - return this.elements; + return elements; } } @@ -79,7 +79,25 @@ String getMethodName() { } List getElements() { - return this.elements; + return elements; + } + } + + /* FieldValue class for numericAdd() transforms. */ + static class NumericAddFieldValue extends FieldValue { + private final Number operand; + + NumericAddFieldValue(Number operand) { + this.operand = operand; + } + + @Override + String getMethodName() { + return "FieldValue.numericAdd"; + } + + Number getOperand() { + return operand; } } @@ -134,4 +152,39 @@ public static FieldValue arrayUnion(@NonNull Object... elements) { public static FieldValue arrayRemove(@NonNull Object... elements) { return new ArrayRemoveFieldValue(Arrays.asList(elements)); } + + /** + * Returns a special value that can be used with set() or update() that tells the server to add + * the given value to the field's current value. + * + *

If the current field value is an integer, possible integer overflows are resolved to + * Long.MAX_VALUE or Long.MIN_VALUE respectively. If the current field value is a double, both + * values will be interpreted as doubles and the arithmetic will follow IEEE 754 semantics. + * + *

If field is not an integer or double, or if the field does not yet exist, the transformation + * will set the field to the given value. + * + * @return The FieldValue sentinel for use in a call to set() or update(). + */ + @NonNull + @PublicApi + public static FieldValue numericAdd(long l) { + return new NumericAddFieldValue(l); + } + + /** + * Returns a special value that can be used with set() or update() that tells the server to add + * the given value to the field's current value. + * + *

If the current value is an integer or a double, both the current and the given value will be + * interpreted as doubles and all arithmetic will follow IEEE 754 semantics. Otherwise, the + * transformation will set the field to the given value. + * + * @return The FieldValue sentinel for use in a call to set() or update(). + */ + @NonNull + @PublicApi + public static FieldValue numericAdd(double l) { + return new NumericAddFieldValue(l); + } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java index 898a189fa3a2..72c2e0b25317 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/UserDataConverter.java @@ -31,6 +31,7 @@ import com.google.firebase.firestore.model.FieldPath; import com.google.firebase.firestore.model.mutation.ArrayTransformOperation; import com.google.firebase.firestore.model.mutation.FieldMask; +import com.google.firebase.firestore.model.mutation.NumericAddTransformOperation; import com.google.firebase.firestore.model.mutation.ServerTimestampOperation; import com.google.firebase.firestore.model.value.ArrayValue; import com.google.firebase.firestore.model.value.BlobValue; @@ -40,6 +41,7 @@ import com.google.firebase.firestore.model.value.GeoPointValue; import com.google.firebase.firestore.model.value.IntegerValue; import com.google.firebase.firestore.model.value.NullValue; +import com.google.firebase.firestore.model.value.NumberValue; import com.google.firebase.firestore.model.value.ObjectValue; import com.google.firebase.firestore.model.value.ReferenceValue; import com.google.firebase.firestore.model.value.StringValue; @@ -349,6 +351,13 @@ private void parseSentinelFieldValue( ArrayTransformOperation arrayRemove = new ArrayTransformOperation.Remove(parsedElements); context.addToFieldTransforms(context.getPath(), arrayRemove); + } else if (value instanceof com.google.firebase.firestore.FieldValue.NumericAddFieldValue) { + com.google.firebase.firestore.FieldValue.NumericAddFieldValue numericAddFieldValue = + (com.google.firebase.firestore.FieldValue.NumericAddFieldValue) value; + NumberValue operand = (NumberValue) parseQueryValue(numericAddFieldValue.getOperand()); + NumericAddTransformOperation numericAdd = new NumericAddTransformOperation(operand); + context.addToFieldTransforms(context.getPath(), numericAdd); + } else { throw Assert.fail("Unknown FieldValue type: %s", Util.typeName(value)); } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalSerializer.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalSerializer.java index 55c8bd146ab1..abe2d65369dc 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalSerializer.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalSerializer.java @@ -160,6 +160,9 @@ com.google.firebase.firestore.proto.WriteBatch encodeMutationBatch(MutationBatch result.setBatchId(batch.getBatchId()); result.setLocalWriteTime(rpcSerializer.encodeTimestamp(batch.getLocalWriteTime())); + for (Mutation mutation : batch.getBaseMutations()) { + result.addBaseWrites(rpcSerializer.encodeMutation(mutation)); + } for (Mutation mutation : batch.getMutations()) { result.addWrites(rpcSerializer.encodeMutation(mutation)); } @@ -171,13 +174,17 @@ MutationBatch decodeMutationBatch(com.google.firebase.firestore.proto.WriteBatch int batchId = batch.getBatchId(); Timestamp localWriteTime = rpcSerializer.decodeTimestamp(batch.getLocalWriteTime()); - int count = batch.getWritesCount(); - List mutations = new ArrayList<>(count); - for (int i = 0; i < count; i++) { + int baseMutationsCount = batch.getBaseWritesCount(); + List baseMutations = new ArrayList<>(baseMutationsCount); + for (int i = 0; i < baseMutationsCount; i++) { + baseMutations.add(rpcSerializer.decodeMutation(batch.getBaseWrites(i))); + } + int mutationsCount = batch.getWritesCount(); + List mutations = new ArrayList<>(mutationsCount); + for (int i = 0; i < mutationsCount; i++) { mutations.add(rpcSerializer.decodeMutation(batch.getWrites(i))); } - - return new MutationBatch(batchId, localWriteTime, mutations); + return new MutationBatch(batchId, localWriteTime, baseMutations, mutations); } com.google.firebase.firestore.proto.Target encodeQueryData(QueryData queryData) { 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 6c203edd3ad1..80c467ce3eac 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 @@ -28,13 +28,18 @@ import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.MaybeDocument; import com.google.firebase.firestore.model.SnapshotVersion; +import com.google.firebase.firestore.model.mutation.FieldMask; import com.google.firebase.firestore.model.mutation.Mutation; import com.google.firebase.firestore.model.mutation.MutationBatch; import com.google.firebase.firestore.model.mutation.MutationBatchResult; +import com.google.firebase.firestore.model.mutation.PatchMutation; +import com.google.firebase.firestore.model.mutation.Precondition; +import com.google.firebase.firestore.model.value.ObjectValue; import com.google.firebase.firestore.remote.RemoteEvent; import com.google.firebase.firestore.remote.TargetChange; import com.google.firebase.firestore.util.Logger; import com.google.protobuf.ByteString; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -182,16 +187,46 @@ public ImmutableSortedMap handleUserChange(User user /** Accepts locally generated Mutations and commits them to storage. */ public LocalWriteResult writeLocally(List mutations) { Timestamp localWriteTime = Timestamp.now(); + // TODO: Call queryEngine.handleDocumentChange() appropriately. - MutationBatch batch = - persistence.runTransaction( - "Locally write mutations", - () -> mutationQueue.addMutationBatch(localWriteTime, mutations)); - Set keys = batch.getKeys(); - ImmutableSortedMap changedDocuments = + Set keys = new HashSet<>(); + for (Mutation mutation : mutations) { + keys.add(mutation.getKey()); + } + + // 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 existingDocuments = localDocuments.getDocuments(keys); - return new LocalWriteResult(batch.getBatchId(), changedDocuments); + + return persistence.runTransaction( + "Locally write mutations", + () -> { + + // For non-idempotent mutations (such as `FieldValue.numericAdd()`), we record the base + // state in a separate patch mutation. This is later used to guarantee consistent values + // and prevents flicker even if the backend sends us an update that already includes our + // transform. + List baseMutations = new ArrayList<>(); + for (Mutation mutation : mutations) { + MaybeDocument maybeDocument = existingDocuments.get(mutation.getKey()); + if (maybeDocument instanceof Document && !mutation.isIdempotent()) { + FieldMask fieldMask = mutation.getFieldMask(); + if (fieldMask != null) { + ObjectValue baseValues = fieldMask.applyTo(((Document) maybeDocument).getData()); + baseMutations.add( + new PatchMutation(mutation.getKey(), baseValues, fieldMask, Precondition.NONE)); + } + } + } + + MutationBatch batch = + mutationQueue.addMutationBatch(localWriteTime, baseMutations, mutations); + ImmutableSortedMap changedDocuments = + batch.applyToLocalView(existingDocuments); + return new LocalWriteResult(batch.getBatchId(), changedDocuments); + }); } /** diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryMutationQueue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryMutationQueue.java index 9af1b1faaba8..842e73e4aca4 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryMutationQueue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryMutationQueue.java @@ -138,7 +138,8 @@ public void setLastStreamToken(ByteString streamToken) { } @Override - public MutationBatch addMutationBatch(Timestamp localWriteTime, List mutations) { + public MutationBatch addMutationBatch( + Timestamp localWriteTime, List baseMutations, List mutations) { hardAssert(!mutations.isEmpty(), "Mutation batches should not be empty"); int batchId = nextBatchId; @@ -151,7 +152,7 @@ public MutationBatch addMutationBatch(Timestamp localWriteTime, List m prior.getBatchId() < batchId, "Mutation batchIds must be monotonically increasing order"); } - MutationBatch batch = new MutationBatch(batchId, localWriteTime, mutations); + MutationBatch batch = new MutationBatch(batchId, localWriteTime, baseMutations, mutations); queue.add(batch); // Track references by document key. diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MutationQueue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MutationQueue.java index da67269cbeab..728eb1abd990 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MutationQueue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MutationQueue.java @@ -47,8 +47,17 @@ interface MutationQueue { /** Sets the stream token for this mutation queue. */ void setLastStreamToken(ByteString streamToken); - /** Creates a new mutation batch and adds it to this mutation queue. */ - MutationBatch addMutationBatch(Timestamp localWriteTime, List mutations); + /** + * Creates a new mutation batch and adds it to this mutation queue. + * + * @param localWriteTime The original write time of this mutation. + * @param baseMutations Mutations that are used to populate the base values when this mutation is + * applied locally. These mutations are used to locally overwrite values that are persisted in + * the remote document cache. + * @param mutations The user-provided mutations in this mutation batch. + */ + MutationBatch addMutationBatch( + Timestamp localWriteTime, List baseMutations, List mutations); /** Loads the mutation batch with the given batchId. */ @Nullable diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java index ccf5bb70490e..485332cdf3d6 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java @@ -192,11 +192,12 @@ private void writeMutationQueueMetadata() { } @Override - public MutationBatch addMutationBatch(Timestamp localWriteTime, List mutations) { + public MutationBatch addMutationBatch( + Timestamp localWriteTime, List baseMutations, List mutations) { int batchId = nextBatchId; nextBatchId += 1; - MutationBatch batch = new MutationBatch(batchId, localWriteTime, mutations); + MutationBatch batch = new MutationBatch(batchId, localWriteTime, baseMutations, mutations); MessageLite proto = serializer.encodeMutationBatch(batch); db.execute( diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ArrayTransformOperation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ArrayTransformOperation.java index 89e10ec33648..73f2056ab8c4 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ArrayTransformOperation.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ArrayTransformOperation.java @@ -87,6 +87,11 @@ static ArrayList coercedFieldValuesArray(@Nullable FieldValue value) } } + @Override + public boolean isIdempotent() { + return true; + } + /** An array union transform operation. */ public static class Union extends ArrayTransformOperation { public Union(List elements) { 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 fb46cb8d6fbf..b7a00c2571aa 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 @@ -82,4 +82,15 @@ public MaybeDocument applyToLocalView( return new NoDocument(getKey(), SnapshotVersion.NONE, /*hasCommittedMutations=*/ false); } + + @Nullable + @Override + public FieldMask getFieldMask() { + return null; + } + + @Override + public boolean isIdempotent() { + return true; + } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/FieldMask.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/FieldMask.java index 0cbef886ccdd..ffb65f3c4aec 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/FieldMask.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/FieldMask.java @@ -15,6 +15,8 @@ package com.google.firebase.firestore.model.mutation; import com.google.firebase.firestore.model.FieldPath; +import com.google.firebase.firestore.model.value.FieldValue; +import com.google.firebase.firestore.model.value.ObjectValue; import java.util.Collection; /** @@ -64,6 +66,25 @@ public boolean covers(FieldPath fieldPath) { return false; } + /** + * Applies this field mask to the provided object value and returns an object that only contains + * fields that are specified in both the input object and this field mask. + */ + public ObjectValue applyTo(ObjectValue data) { + ObjectValue filteredObject = ObjectValue.emptyObject(); + for (FieldPath path : mask) { + if (path.isEmpty()) { + return data; + } else { + FieldValue newValue = data.get(path); + if (newValue != null) { + filteredObject = filteredObject.set(path, newValue); + } + } + } + return filteredObject; + } + @Override public int hashCode() { return mask.hashCode(); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/FieldTransform.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/FieldTransform.java index d6739a9f8b82..831f79e07397 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/FieldTransform.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/FieldTransform.java @@ -58,4 +58,8 @@ public int hashCode() { result = 31 * result + operation.hashCode(); return result; } + + public boolean isIdempotent() { + return operation.isIdempotent(); + } } 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 93fbaac2ebc1..fb56493a85bb 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 @@ -148,4 +148,15 @@ static SnapshotVersion getPostMutationVersion(@Nullable MaybeDocument maybeDoc) return SnapshotVersion.NONE; } } + + /** + * If applicable, returns the field mask for this mutation. Fields that are not included in this + * field mask are not modified when this mutation is applied. Mutations that replace all document + * values return 'null'. + */ + @Nullable + public abstract FieldMask getFieldMask(); + + /** Returns whether all operations in the mutation are idempotent. */ + public abstract boolean isIdempotent(); } 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 bc3ab7ec111c..68e2ff8ae74d 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 @@ -17,6 +17,7 @@ import static com.google.firebase.firestore.util.Assert.hardAssert; import com.google.firebase.Timestamp; +import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.MaybeDocument; import java.util.Collections; @@ -40,13 +41,33 @@ public final class MutationBatch { */ public static final int UNKNOWN = -1; + /** The unique ID of this mutation batch. */ private final int batchId; + + /** The original write time of this mutation. */ private final Timestamp localWriteTime; + + /** + * Mutations that are used to populate the base values when this mutation is applied locally. This + * can be used to locally overwrite values that are persisted in the remote document cache. Base + * mutations are never sent to the backend. + */ + private final List baseMutations; + + /** + * The user-provided mutations in this mutation batch. User-provided mutations are applied both + * locally and remotely on the backend. + */ private final List mutations; - public MutationBatch(int batchId, Timestamp localWriteTime, List mutations) { + public MutationBatch( + int batchId, + Timestamp localWriteTime, + List baseMutations, + List mutations) { this.batchId = batchId; this.localWriteTime = localWriteTime; + this.baseMutations = baseMutations; this.mutations = mutations; } @@ -98,8 +119,18 @@ public MaybeDocument applyToLocalView(DocumentKey documentKey, @Nullable MaybeDo maybeDoc.getKey()); } + // First, apply the base state. This allows us to apply certain non-idempotent transform against + // a consistent set of values. + for (int i = 0; i < baseMutations.size(); i++) { + Mutation mutation = baseMutations.get(i); + if (mutation.getKey().equals(documentKey)) { + maybeDoc = mutation.applyToLocalView(maybeDoc, maybeDoc, localWriteTime); + } + } + MaybeDocument baseDoc = maybeDoc; + // Second, apply all user-provided mutations. for (int i = 0; i < mutations.size(); i++) { Mutation mutation = mutations.get(i); if (mutation.getKey().equals(documentKey)) { @@ -109,6 +140,19 @@ public MaybeDocument applyToLocalView(DocumentKey documentKey, @Nullable MaybeDo return maybeDoc; } + /** Computes the local view for all provided documents given the mutations in this batch. */ + public ImmutableSortedMap applyToLocalView( + ImmutableSortedMap maybeDocumentMap) { + ImmutableSortedMap changedDocuments = maybeDocumentMap; + for (DocumentKey key : getKeys()) { + MaybeDocument maybeDocument = applyToLocalView(key, changedDocuments.get(key)); + if (maybeDocument != null) { + changedDocuments = changedDocuments.insert(maybeDocument.getKey(), maybeDocument); + } + } + return changedDocuments; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -121,6 +165,7 @@ public boolean equals(Object o) { MutationBatch that = (MutationBatch) o; return batchId == that.batchId && localWriteTime.equals(that.localWriteTime) + && baseMutations.equals(that.baseMutations) && mutations.equals(that.mutations); } @@ -128,6 +173,7 @@ public boolean equals(Object o) { public int hashCode() { int result = batchId; result = 31 * result + localWriteTime.hashCode(); + result = 31 * result + baseMutations.hashCode(); result = 31 * result + mutations.hashCode(); return result; } @@ -138,6 +184,8 @@ public String toString() { + batchId + ", localWriteTime=" + localWriteTime + + ", baseMutations=" + + baseMutations + ", mutations=" + mutations + ')'; @@ -177,10 +225,19 @@ public boolean isTombstone() { /** Converts this batch to a tombstone. */ public MutationBatch toTombstone() { - return new MutationBatch(batchId, localWriteTime, Collections.emptyList()); + return new MutationBatch(batchId, localWriteTime, baseMutations, Collections.emptyList()); } + /** @return The user-provided mutations in this mutation batch. */ public List getMutations() { return mutations; } + + /** + * @return The mutations that are used to populate the base values when this mutation batch is + * applied locally. + */ + public List getBaseMutations() { + return baseMutations; + } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/NumericAddTransformOperation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/NumericAddTransformOperation.java new file mode 100644 index 000000000000..f6f677499274 --- /dev/null +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/NumericAddTransformOperation.java @@ -0,0 +1,109 @@ +// Copyright 2018 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.model.mutation; + +import static com.google.firebase.firestore.util.Assert.fail; + +import com.google.firebase.Timestamp; +import com.google.firebase.firestore.model.value.DoubleValue; +import com.google.firebase.firestore.model.value.FieldValue; +import com.google.firebase.firestore.model.value.IntegerValue; +import com.google.firebase.firestore.model.value.NumberValue; + +/** + * Implements the backend semantics for locally computed NUMERIC_ADD transforms. Converts all field + * values to longs or doubles and resolves overflows to Long.MAX_VALUE/Long.MIN_VALUE. + */ +public class NumericAddTransformOperation implements TransformOperation { + private NumberValue operand; + + public NumericAddTransformOperation(NumberValue operand) { + this.operand = operand; + } + + @Override + public FieldValue applyToLocalView(FieldValue previousValue, Timestamp localWriteTime) { + // Return an integer value only if the previous value and the operand is an integer. + if (previousValue instanceof IntegerValue && operand instanceof IntegerValue) { + long sum = safeAdd(((IntegerValue) previousValue).getInternalValue(), operandAsLong()); + return IntegerValue.valueOf(sum); + } else if (previousValue instanceof IntegerValue) { + double sum = ((IntegerValue) previousValue).getInternalValue() + operandAsDouble(); + return DoubleValue.valueOf(sum); + } else if (previousValue instanceof DoubleValue) { + double sum = ((DoubleValue) previousValue).getInternalValue() + operandAsDouble(); + return DoubleValue.valueOf(sum); + } + + // If the existing value is not a number, use the value of the transform as the new base value. + return operand; + } + + /** + * Implementation of Java 8's `safeAdd()` that resolves positive and negative numeric overflows to + * Long.MAX_VALUE or Long.MIN_VALUE respectively (instead of throwing an ArithmeticException). + */ + private long safeAdd(long x, long y) { + long r = x + y; + + if (((x ^ r) & (y ^ r)) >= 0) { + return r; + } + + if (r > 0L) { + return Long.MIN_VALUE; + } else { + return Long.MAX_VALUE; + } + } + + private double operandAsDouble() { + if (operand instanceof DoubleValue) { + return ((DoubleValue) operand).getInternalValue(); + } else if (operand instanceof IntegerValue) { + return ((IntegerValue) operand).getInternalValue(); + } else { + throw fail( + "Expected 'operand' to be of Number type, but was " + + operand.getClass().getCanonicalName()); + } + } + + private long operandAsLong() { + if (operand instanceof DoubleValue) { + return (long) ((DoubleValue) operand).getInternalValue(); + } else if (operand instanceof IntegerValue) { + return ((IntegerValue) operand).getInternalValue(); + } else { + throw fail( + "Expected 'operand' to be of Number type, but was " + + operand.getClass().getCanonicalName()); + } + } + + @Override + public FieldValue applyToRemoteDocument(FieldValue previousValue, FieldValue transformResult) { + return transformResult; + } + + public FieldValue getOperand() { + return operand; + } + + @Override + public boolean isIdempotent() { + return false; + } +} 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 3f119ea663ee..ade731ba1c21 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 @@ -130,6 +130,12 @@ public MaybeDocument applyToLocalView( return new Document(getKey(), version, newData, Document.DocumentState.LOCAL_MUTATIONS); } + @Nullable + @Override + public FieldMask getFieldMask() { + return mask; + } + /** * Patches the data of document if available or creates a new document. Note that this does not * check whether or not the precondition of this patch holds. @@ -157,4 +163,9 @@ private ObjectValue patchObject(ObjectValue obj) { } return obj; } + + @Override + public boolean isIdempotent() { + return true; + } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ServerTimestampOperation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ServerTimestampOperation.java index 4d0eabd12f9e..0f75a9430b8e 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ServerTimestampOperation.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/ServerTimestampOperation.java @@ -38,6 +38,11 @@ public FieldValue applyToRemoteDocument(FieldValue previousValue, FieldValue tra return transformResult; } + @Override + public boolean isIdempotent() { + return true; + } + // NOTE: Since we've guaranteed a singleton instance, we can rely on Object's default // implementation of equals() / hashCode(). } 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 dc0fa550e8f7..f4fd44fe19e1 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 @@ -89,8 +89,19 @@ public MaybeDocument applyToLocalView( return new Document(getKey(), version, value, Document.DocumentState.LOCAL_MUTATIONS); } + @Nullable + @Override + public FieldMask getFieldMask() { + return null; + } + /** Returns the object value to use when setting the document. */ public ObjectValue getValue() { return value; } + + @Override + public boolean isIdempotent() { + return true; + } } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformMutation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformMutation.java index f925017fa50e..12a90088e726 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformMutation.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformMutation.java @@ -121,6 +121,25 @@ public MaybeDocument applyToLocalView( getKey(), doc.getVersion(), newData, Document.DocumentState.LOCAL_MUTATIONS); } + @Override + public FieldMask getFieldMask() { + List fieldMask = new ArrayList<>(); + for (FieldTransform transform : fieldTransforms) { + fieldMask.add(transform.getFieldPath()); + } + return FieldMask.fromCollection(fieldMask); + } + + @Override + public boolean isIdempotent() { + for (FieldTransform transform : fieldTransforms) { + if (!transform.isIdempotent()) { + return false; + } + } + return true; + } + /** * Asserts that the given MaybeDocument is actually a Document and verifies that it matches the * key for this mutation. Since we only support transformations with precondition exists this diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformOperation.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformOperation.java index 7e9bcf506109..22bda140c28a 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformOperation.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/TransformOperation.java @@ -30,4 +30,7 @@ public interface TransformOperation { * potentially using the server-provided transformResult. */ FieldValue applyToRemoteDocument(FieldValue previousValue, FieldValue transformResult); + + /** Returns whether this field transform is idempotent. */ + boolean isIdempotent(); } 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 fc4eb36bd300..b403e75d4e0c 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 @@ -45,6 +45,7 @@ import com.google.firebase.firestore.model.mutation.FieldTransform; import com.google.firebase.firestore.model.mutation.Mutation; import com.google.firebase.firestore.model.mutation.MutationResult; +import com.google.firebase.firestore.model.mutation.NumericAddTransformOperation; import com.google.firebase.firestore.model.mutation.PatchMutation; import com.google.firebase.firestore.model.mutation.Precondition; import com.google.firebase.firestore.model.mutation.ServerTimestampOperation; @@ -59,6 +60,7 @@ import com.google.firebase.firestore.model.value.GeoPointValue; import com.google.firebase.firestore.model.value.IntegerValue; import com.google.firebase.firestore.model.value.NullValue; +import com.google.firebase.firestore.model.value.NumberValue; import com.google.firebase.firestore.model.value.ObjectValue; import com.google.firebase.firestore.model.value.ReferenceValue; import com.google.firebase.firestore.model.value.StringValue; @@ -558,6 +560,12 @@ private DocumentTransform.FieldTransform encodeFieldTransform(FieldTransform fie .setFieldPath(fieldTransform.getFieldPath().canonicalString()) .setRemoveAllFromArray(encodeArrayTransformElements(remove.getElements())) .build(); + } else if (transform instanceof NumericAddTransformOperation) { + NumericAddTransformOperation numericAdd = (NumericAddTransformOperation) transform; + return DocumentTransform.FieldTransform.newBuilder() + .setFieldPath(fieldTransform.getFieldPath().canonicalString()) + .setNumericAdd(encodeValue(numericAdd.getOperand())) + .build(); } else { throw fail("Unknown transform: %s", transform); } @@ -594,6 +602,18 @@ private FieldTransform decodeFieldTransform(DocumentTransform.FieldTransform fie FieldPath.fromServerFormat(fieldTransform.getFieldPath()), new ArrayTransformOperation.Remove( decodeArrayTransformElements(fieldTransform.getRemoveAllFromArray()))); + case NUMERIC_ADD: + { + FieldValue operand = decodeValue(fieldTransform.getNumericAdd()); + hardAssert( + operand instanceof NumberValue, + "Expected NUMERIC_ADD transform to be of number type, but was %s", + operand.getClass().getCanonicalName()); + return new FieldTransform( + FieldPath.fromServerFormat(fieldTransform.getFieldPath()), + new NumericAddTransformOperation( + (NumberValue) decodeValue(fieldTransform.getNumericAdd()))); + } default: throw fail("Unknown FieldTransform proto: %s", fieldTransform); } diff --git a/firebase-firestore/src/proto/google/firebase/firestore/proto/mutation.proto b/firebase-firestore/src/proto/google/firebase/firestore/proto/mutation.proto index ea448c89a644..11482112bf09 100644 --- a/firebase-firestore/src/proto/google/firebase/firestore/proto/mutation.proto +++ b/firebase-firestore/src/proto/google/firebase/firestore/proto/mutation.proto @@ -55,4 +55,10 @@ message WriteBatch { // The local time at which the write batch was initiated. google.protobuf.Timestamp local_write_time = 3; + + // A list of writes that are used to populate a base state when this mutation + // is applied locally. These writes are used to locally overwrite values that + // are persisted in the remote document cache. These writes are never sent to + // the backend. + repeated google.firestore.v1beta1.Write base_writes = 4; } diff --git a/firebase-firestore/src/proto/google/firestore/v1beta1/write.proto b/firebase-firestore/src/proto/google/firestore/v1beta1/write.proto index 4f86df37edec..c50868573e7f 100644 --- a/firebase-firestore/src/proto/google/firestore/v1beta1/write.proto +++ b/firebase-firestore/src/proto/google/firestore/v1beta1/write.proto @@ -103,6 +103,18 @@ message DocumentTransform { // Sets the field to the given server value. ServerValue set_to_server_value = 2; + // Adds the given value to the field's current value. + // + // This must be an integer or a double value. + // If the field is not an integer or double, or if the field does not yet + // exist, the transformation will set the field to the given value. + // If either of the given value or the current field value are doubles, + // both values will be interpreted as doubles. Double arithmetic and + // representation of double values follow IEEE 754 semantics. + // If there is positive/negative integer overflow, the field is resolved + // to the largest magnitude positive/negative integer. + Value numeric_add = 3; + // Append the given elements in order if they are not already present in // the current field value. // If the field is not an array, or if the field does not yet exist, it is diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalSerializerTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalSerializerTest.java index db743396b29d..978a9238fe3d 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalSerializerTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalSerializerTest.java @@ -44,6 +44,7 @@ import com.google.firestore.v1beta1.Value; import com.google.firestore.v1beta1.Write; import com.google.protobuf.ByteString; +import java.util.Collections; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -66,6 +67,12 @@ public void setUp() { @Test public void testEncodesMutationBatch() { + Mutation baseWrite = + new PatchMutation( + key("foo/bar"), + TestUtil.wrapObject(map("a", "b")), + FieldMask.fromCollection(asList(field("a"))), + com.google.firebase.firestore.model.mutation.Precondition.NONE); Mutation set = setMutation("foo/bar", map("a", "b", "num", 1)); Mutation patch = new PatchMutation( @@ -75,7 +82,18 @@ public void testEncodesMutationBatch() { com.google.firebase.firestore.model.mutation.Precondition.exists(true)); Mutation del = deleteMutation("baz/quux"); Timestamp writeTime = Timestamp.now(); - MutationBatch model = new MutationBatch(42, writeTime, asList(set, patch, del)); + MutationBatch model = + new MutationBatch( + 42, writeTime, Collections.singletonList(baseWrite), asList(set, patch, del)); + + Write baseWriteProto = + Write.newBuilder() + .setUpdate( + com.google.firestore.v1beta1.Document.newBuilder() + .setName("projects/p/databases/d/documents/foo/bar") + .putFields("a", Value.newBuilder().setStringValue("b").build())) + .setUpdateMask(DocumentMask.newBuilder().addFieldPaths("a")) + .build(); Write setProto = Write.newBuilder() @@ -109,6 +127,7 @@ public void testEncodesMutationBatch() { com.google.firebase.firestore.proto.WriteBatch batchProto = com.google.firebase.firestore.proto.WriteBatch.newBuilder() .setBatchId(42) + .addBaseWrites(baseWriteProto) .addAllWrites(asList(setProto, patchProto, delProto)) .setLocalWriteTime(writeTimeProto) .build(); @@ -118,6 +137,7 @@ public void testEncodesMutationBatch() { assertEquals(model.getBatchId(), decoded.getBatchId()); assertEquals(model.getLocalWriteTime(), decoded.getLocalWriteTime()); assertEquals(model.getMutations(), decoded.getMutations()); + assertEquals(model.getBaseMutations(), decoded.getBaseMutations()); assertEquals(model.getKeys(), decoded.getKeys()); } 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 5144a91e62e2..1032b02f5e7b 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 @@ -25,11 +25,13 @@ import static com.google.firebase.firestore.testutil.TestUtil.query; import static com.google.firebase.firestore.testutil.TestUtil.resumeToken; import static com.google.firebase.firestore.testutil.TestUtil.setMutation; +import static com.google.firebase.firestore.testutil.TestUtil.transformMutation; import static com.google.firebase.firestore.testutil.TestUtil.unknownDoc; import static com.google.firebase.firestore.testutil.TestUtil.updateRemoteEvent; import static com.google.firebase.firestore.testutil.TestUtil.values; import static com.google.firebase.firestore.testutil.TestUtil.version; import static com.google.firebase.firestore.testutil.TestUtil.viewChanges; +import static com.google.firebase.firestore.testutil.TestUtil.wrap; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; @@ -43,6 +45,7 @@ import com.google.firebase.Timestamp; import com.google.firebase.database.collection.ImmutableSortedMap; import com.google.firebase.database.collection.ImmutableSortedSet; +import com.google.firebase.firestore.FieldValue; import com.google.firebase.firestore.TestUtil.TestTargetMetadataProvider; import com.google.firebase.firestore.auth.User; import com.google.firebase.firestore.core.Query; @@ -66,6 +69,8 @@ import com.google.firebase.firestore.testutil.TestUtil; import com.google.protobuf.ByteString; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map.Entry; import java.util.Set; @@ -118,7 +123,9 @@ private void writeMutation(Mutation mutation) { private void writeMutations(List mutations) { LocalWriteResult result = localStore.writeLocally(mutations); - batches.add(new MutationBatch(result.getBatchId(), Timestamp.now(), mutations)); + batches.add( + new MutationBatch( + result.getBatchId(), Timestamp.now(), Collections.emptyList(), mutations)); lastChanges = result.getChanges(); } @@ -130,16 +137,23 @@ private void notifyLocalViewChanges(LocalViewChanges changes) { localStore.notifyLocalViewChanges(asList(changes)); } - private void acknowledgeMutation(long documentVersion) { + private void acknowledgeMutation(long documentVersion, @Nullable Object transformResult) { MutationBatch batch = batches.remove(0); SnapshotVersion version = version(documentVersion); - MutationResult mutationResult = new MutationResult(version, /*transformResults=*/ null); + MutationResult mutationResult = + new MutationResult( + version, + transformResult != null ? Collections.singletonList(wrap(transformResult)) : null); MutationBatchResult result = MutationBatchResult.create( batch, version, singletonList(mutationResult), WriteStream.EMPTY_STREAM_TOKEN); lastChanges = localStore.acknowledgeBatch(result); } + private void acknowledgeMutation(long documentVersion) { + acknowledgeMutation(documentVersion, null); + } + private void rejectMutation() { MutationBatch batch = batches.get(0); batches.remove(0); @@ -203,7 +217,8 @@ private void assertNotContains(String keyPathString) { public void testMutationBatchKeys() { SetMutation set1 = setMutation("foo/bar", map("foo", "bar")); SetMutation set2 = setMutation("foo/baz", map("foo", "baz")); - MutationBatch batch = new MutationBatch(1, Timestamp.now(), asList(set1, set2)); + MutationBatch batch = + new MutationBatch(1, Timestamp.now(), Collections.emptyList(), asList(set1, set2)); Set keys = batch.getKeys(); assertEquals(2, keys.size()); } @@ -934,4 +949,143 @@ public void testRemoteDocumentKeysForTarget() { keys = localStore.getRemoteDocumentKeys(2); assertSetEquals(asList(key("foo/bar"), key("foo/baz")), keys); } + + @Test + public void testHandlesSetMutationThenTransformMutationThenTransformMutation() { + writeMutation(setMutation("foo/bar", map("sum", 0))); + assertContains(doc("foo/bar", 0, map("sum", 0), Document.DocumentState.LOCAL_MUTATIONS)); + assertChanged(doc("foo/bar", 0, map("sum", 0), Document.DocumentState.LOCAL_MUTATIONS)); + + writeMutation(transformMutation("foo/bar", map("sum", FieldValue.numericAdd(1)))); + assertContains(doc("foo/bar", 0, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS)); + assertChanged(doc("foo/bar", 0, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS)); + + writeMutation(transformMutation("foo/bar", map("sum", FieldValue.numericAdd(2)))); + assertContains(doc("foo/bar", 0, map("sum", 3), Document.DocumentState.LOCAL_MUTATIONS)); + assertChanged(doc("foo/bar", 0, map("sum", 3), Document.DocumentState.LOCAL_MUTATIONS)); + } + + @Test + public void testHandlesSetMutationThenAckThenTransformMutationThenAckThenTransformMutation() { + if (garbageCollectorIsEager()) { + return; + } + + writeMutation(setMutation("foo/bar", map("sum", 0))); + assertContains(doc("foo/bar", 0, map("sum", 0), Document.DocumentState.LOCAL_MUTATIONS)); + assertChanged(doc("foo/bar", 0, map("sum", 0), Document.DocumentState.LOCAL_MUTATIONS)); + + acknowledgeMutation(1); + assertChanged(doc("foo/bar", 1, map("sum", 0), Document.DocumentState.COMMITTED_MUTATIONS)); + assertContains(doc("foo/bar", 1, map("sum", 0), Document.DocumentState.COMMITTED_MUTATIONS)); + + writeMutation(transformMutation("foo/bar", map("sum", FieldValue.numericAdd(1)))); + assertContains(doc("foo/bar", 1, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS)); + assertChanged(doc("foo/bar", 1, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS)); + + acknowledgeMutation(2, 1); + assertChanged(doc("foo/bar", 2, map("sum", 1), Document.DocumentState.COMMITTED_MUTATIONS)); + assertContains(doc("foo/bar", 2, map("sum", 1), Document.DocumentState.COMMITTED_MUTATIONS)); + + writeMutation(transformMutation("foo/bar", map("sum", FieldValue.numericAdd(2)))); + assertContains(doc("foo/bar", 2, map("sum", 3), Document.DocumentState.LOCAL_MUTATIONS)); + assertChanged(doc("foo/bar", 2, map("sum", 3), Document.DocumentState.LOCAL_MUTATIONS)); + } + + @Test + public void testHoldsBackNonIdempotentTransforms() { + Query query = Query.atPath(ResourcePath.fromString("foo")); + allocateQuery(query); + assertTargetId(2); + + writeMutation(setMutation("foo/bar", map("sum", 0, "array_union", new ArrayList<>()))); + assertChanged( + doc( + "foo/bar", + 0, + map("sum", 0, "array_union", new ArrayList<>()), + Document.DocumentState.LOCAL_MUTATIONS)); + + acknowledgeMutation(1); + assertChanged( + doc( + "foo/bar", + 1, + map("sum", 0, "array_union", new ArrayList<>()), + Document.DocumentState.COMMITTED_MUTATIONS)); + + applyRemoteEvent( + addedRemoteEvent( + doc("foo/bar", 1, map("sum", 0, "array_union", new ArrayList<>())), + asList(2), + emptyList())); + assertChanged( + doc( + "foo/bar", + 1, + map("sum", 0, "array_union", new ArrayList<>()), + Document.DocumentState.SYNCED)); + + writeMutations( + Arrays.asList( + transformMutation("foo/bar", map("sum", FieldValue.numericAdd(1))), + transformMutation("foo/bar", map("array_union", FieldValue.arrayUnion("foo"))))); + assertChanged( + doc( + "foo/bar", + 1, + map("sum", 1, "array_union", Collections.singletonList("foo")), + Document.DocumentState.LOCAL_MUTATIONS)); + + // The sum transform is not idempotent and the backend's updated value is ignored. The + // ArrayUnion transform is recomputed and includes the backend value. + applyRemoteEvent( + addedRemoteEvent( + doc("foo/bar", 1, map("sum", 2, "array_union", Collections.singletonList("bar"))), + asList(2), + emptyList())); + assertChanged( + doc( + "foo/bar", + 1, + map("sum", 1, "array_union", Arrays.asList("bar", "foo")), + Document.DocumentState.LOCAL_MUTATIONS)); + } + + @Test + public void testHandlesSetMutationThenTransformMutationThenRemoteEventThenTransformMutation() { + Query query = Query.atPath(ResourcePath.fromString("foo")); + allocateQuery(query); + assertTargetId(2); + + writeMutation(setMutation("foo/bar", map("sum", 0))); + assertChanged(doc("foo/bar", 0, map("sum", 0), Document.DocumentState.LOCAL_MUTATIONS)); + assertContains(doc("foo/bar", 0, map("sum", 0), Document.DocumentState.LOCAL_MUTATIONS)); + + applyRemoteEvent(addedRemoteEvent(doc("foo/bar", 1, map("sum", 0)), asList(2), emptyList())); + acknowledgeMutation(1); + assertChanged(doc("foo/bar", 1, map("sum", 0), Document.DocumentState.SYNCED)); + assertContains(doc("foo/bar", 1, map("sum", 0), Document.DocumentState.SYNCED)); + + writeMutation(transformMutation("foo/bar", map("sum", FieldValue.numericAdd(1)))); + assertChanged(doc("foo/bar", 1, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS)); + assertContains(doc("foo/bar", 1, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS)); + + // The value in this remote event gets ignored since we still have a pending transform mutation. + applyRemoteEvent(addedRemoteEvent(doc("foo/bar", 2, map("sum", 1337)), asList(2), emptyList())); + assertChanged(doc("foo/bar", 2, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS)); + assertContains(doc("foo/bar", 2, map("sum", 1), Document.DocumentState.LOCAL_MUTATIONS)); + + // Add another increment. Note that we still compute the increment based on the local value. + writeMutation(transformMutation("foo/bar", map("sum", FieldValue.numericAdd(2)))); + assertChanged(doc("foo/bar", 2, map("sum", 3), Document.DocumentState.LOCAL_MUTATIONS)); + assertContains(doc("foo/bar", 2, map("sum", 3), Document.DocumentState.LOCAL_MUTATIONS)); + + acknowledgeMutation(3, 1); + assertChanged(doc("foo/bar", 3, map("sum", 3), Document.DocumentState.LOCAL_MUTATIONS)); + + acknowledgeMutation(4, 1339); + assertChanged(doc("foo/bar", 4, map("sum", 1339), Document.DocumentState.COMMITTED_MUTATIONS)); + assertContains(doc("foo/bar", 4, map("sum", 1339), Document.DocumentState.COMMITTED_MUTATIONS)); + } } diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java index 11cbc6a3493b..50f74781dcbc 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/LruGarbageCollectorTestCase.java @@ -40,6 +40,7 @@ import com.google.firebase.firestore.model.value.ObjectValue; import com.google.protobuf.ByteString; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -359,7 +360,7 @@ public void testRemoveOrphanedDocuments() { "actually register the mutations", () -> { Timestamp writeTime = Timestamp.now(); - mutationQueue.addMutationBatch(writeTime, mutations); + mutationQueue.addMutationBatch(writeTime, Collections.emptyList(), mutations); }); // Mark 5 documents eligible for GC. This simulates documents that were mutated then ack'd. diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MutationQueueTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MutationQueueTestCase.java index c0f2d45b7932..d741e979d2c5 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MutationQueueTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MutationQueueTestCase.java @@ -41,6 +41,7 @@ import com.google.firebase.firestore.remote.WriteStream; import com.google.protobuf.ByteString; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import org.junit.After; @@ -211,7 +212,9 @@ public void testAllMutationBatchesAffectingDocumentKey() { "New mutation batch", () -> { for (Mutation mutation : mutations) { - batches.add(mutationQueue.addMutationBatch(Timestamp.now(), asList(mutation))); + batches.add( + mutationQueue.addMutationBatch( + Timestamp.now(), Collections.emptyList(), asList(mutation))); } }); @@ -239,7 +242,9 @@ public void testAllMutationBatchesAffectingDocumentKeys() { "New mutation batch", () -> { for (Mutation mutation : mutations) { - batches.add(mutationQueue.addMutationBatch(Timestamp.now(), asList(mutation))); + batches.add( + mutationQueue.addMutationBatch( + Timestamp.now(), Collections.emptyList(), asList(mutation))); } }); @@ -268,7 +273,9 @@ public void testAllMutationBatchesAffectingDocumentLotsOfDocumentKeys() { "New mutation batch", () -> { for (Mutation mutation : mutations) { - batches.add(mutationQueue.addMutationBatch(Timestamp.now(), asList(mutation))); + batches.add( + mutationQueue.addMutationBatch( + Timestamp.now(), Collections.emptyList(), asList(mutation))); } }); @@ -304,7 +311,9 @@ public void testAllMutationBatchesAffectingQuery() { "New mutation batch", () -> { for (Mutation mutation : mutations) { - batches.add(mutationQueue.addMutationBatch(Timestamp.now(), asList(mutation))); + batches.add( + mutationQueue.addMutationBatch( + Timestamp.now(), Collections.emptyList(), asList(mutation))); } }); @@ -328,10 +337,12 @@ public void testAllMutationBatchesAffectingQuery_withCompoundBatches() { batches.add( mutationQueue.addMutationBatch( Timestamp.now(), + Collections.emptyList(), asList(setMutation("foo/bar", value), setMutation("foo/bar/baz/quux", value)))); batches.add( mutationQueue.addMutationBatch( Timestamp.now(), + Collections.emptyList(), asList(setMutation("foo/bar", value), setMutation("foo/baz", value)))); }); @@ -423,7 +434,9 @@ private MutationBatch addMutationBatch(String key) { return persistence.runTransaction( "New mutation batch", - () -> mutationQueue.addMutationBatch(Timestamp.now(), asList(mutation))); + () -> + mutationQueue.addMutationBatch( + Timestamp.now(), Collections.emptyList(), asList(mutation))); } /** 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 d686c4829ef5..e2cf5d985995 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 @@ -40,12 +40,14 @@ import com.google.firebase.firestore.model.mutation.PatchMutation; import com.google.firebase.firestore.model.mutation.Precondition; import com.google.firebase.firestore.model.mutation.TransformMutation; +import com.google.firebase.firestore.model.value.IntegerValue; import com.google.firebase.firestore.model.value.ObjectValue; import com.google.firebase.firestore.model.value.ServerTimestampValue; import com.google.firebase.firestore.model.value.StringValue; import com.google.firebase.firestore.model.value.TimestampValue; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; @@ -166,6 +168,69 @@ public void testAppliesLocalServerTimestampTransformsToDocuments() { assertEquals(expectedDoc, transformedDoc); } + @Test + public void testAppliesNumericAddTransformToDocument() { + Map baseDoc = + map("longPlusLong", 1, "longPlusDouble", 2, "doublePlusLong", 3.3, "doublePlusDouble", 4.0); + Map transform = + map( + "longPlusLong", + FieldValue.numericAdd(1), + "longPlusDouble", + FieldValue.numericAdd(2.2), + "doublePlusLong", + FieldValue.numericAdd(3), + "doublePlusDouble", + FieldValue.numericAdd(4.4)); + Map expected = + map( + "longPlusLong", + 2L, + "longPlusDouble", + 4.2D, + "doublePlusLong", + 6.3D, + "doublePlusDouble", + 8.4D); + + verifyTransform(baseDoc, transform, expected); + } + + @Test + public void testAppliesNumericAddTransformToUnexpectedType() { + Map baseDoc = map("string", "zero"); + Map transform = map("string", FieldValue.numericAdd(1)); + Map expected = map("string", 1); + verifyTransform(baseDoc, transform, expected); + } + + @Test + public void testAppliesNumericAddTransformToMissingField() { + Map baseDoc = map(); + Map transform = map("missing", FieldValue.numericAdd(1)); + Map expected = map("missing", 1); + verifyTransform(baseDoc, transform, expected); + } + + @Test + public void testAppliesNumericAddTransformsConsecutively() { + Map baseDoc = map("number", 1); + Map transform1 = map("number", FieldValue.numericAdd(2)); + Map transform2 = map("number", FieldValue.numericAdd(3)); + Map transform3 = map("number", FieldValue.numericAdd(4)); + Map expected = map("number", 10); + verifyTransform(baseDoc, Arrays.asList(transform1, transform2, transform3), expected); + } + + @Test + public void testAppliesNumericAddWithoutOverflow() { + Map baseDoc = map("max", Long.MAX_VALUE - 1, "min", Long.MIN_VALUE + 1); + Map transform = + map("max", FieldValue.numericAdd(2), "min", FieldValue.numericAdd(-2)); + Map expected = map("max", Long.MAX_VALUE, "min", Long.MIN_VALUE); + verifyTransform(baseDoc, transform, expected); + } + // NOTE: This is more a test of UserDataConverter code than Mutation code but we don't have unit // tests for it currently. We could consider removing this test once we have integration tests. @Test @@ -324,15 +389,42 @@ public void testAppliesLocalArrayRemoveTransformWithNonPrimitiveElements() { private void verifyTransform( Map baseData, - Map transformData, + List> transforms, Map expectedData) { - Document baseDoc = doc("collection/key", 0, baseData); - TransformMutation transform = transformMutation("collection/key", transformData); - MaybeDocument transformedDoc = transform.applyToLocalView(baseDoc, baseDoc, Timestamp.now()); + MaybeDocument currentDoc = doc("collection/key", 0, baseData); + + for (Map transformData : transforms) { + TransformMutation transform = transformMutation("collection/key", transformData); + currentDoc = transform.applyToLocalView(currentDoc, currentDoc, Timestamp.now()); + } Document expectedDoc = doc("collection/key", 0, expectedData, Document.DocumentState.LOCAL_MUTATIONS); - assertEquals(expectedDoc, transformedDoc); + assertEquals(expectedDoc, currentDoc); + } + + private void verifyTransform( + Map baseData, + Map transformData, + Map expectedData) { + verifyTransform(baseData, Collections.singletonList(transformData), expectedData); + } + + @Test + public void testAppliesServerAckedNumericAddTransformToDocuments() { + Map data = map("sum", 1); + Document baseDoc = doc("collection/key", 0, data); + + Mutation transform = transformMutation("collection/key", map("sum", FieldValue.numericAdd(2))); + MutationResult mutationResult = + new MutationResult(version(1), Collections.singletonList(IntegerValue.valueOf(3L))); + + MaybeDocument transformedDoc = transform.applyToRemoteDocument(baseDoc, mutationResult); + + Map expectedData = map("sum", 3L); + assertEquals( + doc("collection/key", 1, expectedData, Document.DocumentState.COMMITTED_MUTATIONS), + transformedDoc); } @Test