Skip to content

Replace usage of transform with updateTransforms #2262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions firebase-firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
This more closely aligns the SDK with the Firestore backend, which has always
accepted null and NaN for all operators, even though this isn't necessarily
useful.
- [changed] A write to a document that contains FieldValue transforms is no
longer split up into two separate operations. This reduces the number of
writes the backend performs and allows each WriteBatch to hold 500 writes
regardless of how many FieldValue transformations are attached.

# (22.0.0)
- [changed] Removed the deprecated `timestampsInSnapshotsEnabled` setting.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import com.google.firebase.firestore.util.Assert;
import com.google.firebase.firestore.util.Executors;
import com.google.firebase.firestore.util.Util;
import java.util.Collections;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
Expand Down Expand Up @@ -166,7 +167,7 @@ public Task<Void> set(@NonNull Object data, @NonNull SetOptions options) {
: firestore.getUserDataReader().parseSetData(data);
return firestore
.getClient()
.write(parsed.toMutationList(key, Precondition.NONE))
.write(Collections.singletonList(parsed.toMutation(key, Precondition.NONE)))
.continueWith(Executors.DIRECT_EXECUTOR, voidErrorTransformer());
}

Expand Down Expand Up @@ -230,7 +231,7 @@ public Task<Void> update(
private Task<Void> update(@NonNull ParsedUpdateData parsedData) {
return firestore
.getClient()
.write(parsedData.toMutationList(key, Precondition.exists(true)))
.write(Collections.singletonList(parsedData.toMutation(key, Precondition.exists(true))))
.continueWith(Executors.DIRECT_EXECUTOR, voidErrorTransformer());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public Value parseQueryValue(Object input, boolean allowArrays) {
}

/** Converts a POJO to native types and then parses it into model types. */
private Value convertAndParseFieldData(Object input, ParseContext context) {
public Value convertAndParseFieldData(Object input, ParseContext context) {
Object converted = CustomClassMapper.convertToPlainJavaTypes(input);
return parseData(converted, context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public WriteBatch set(
options.isMerge()
? firestore.getUserDataReader().parseMergeData(data, options.getFieldMask())
: firestore.getUserDataReader().parseSetData(data);
mutations.addAll(parsed.toMutationList(documentRef.getKey(), Precondition.NONE));
mutations.add(parsed.toMutation(documentRef.getKey(), Precondition.NONE));
return this;
}

Expand Down Expand Up @@ -163,7 +163,7 @@ private WriteBatch update(
@NonNull DocumentReference documentRef, @NonNull ParsedUpdateData updateData) {
firestore.validateReference(documentRef);
verifyNotCommitted();
mutations.addAll(updateData.toMutationList(documentRef.getKey(), Precondition.exists(true)));
mutations.add(updateData.toMutation(documentRef.getKey(), Precondition.exists(true)));
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public Task<List<MaybeDocument>> lookup(List<DocumentKey> keys) {

/** Stores a set mutation for the given key and value, to be committed when commit() is called. */
public void set(DocumentKey key, ParsedSetData data) {
write(data.toMutationList(key, precondition(key)));
write(Collections.singletonList(data.toMutation(key, precondition(key))));
writtenDocs.add(key);
}

Expand All @@ -113,7 +113,7 @@ public void set(DocumentKey key, ParsedSetData data) {
*/
public void update(DocumentKey key, ParsedUpdateData data) {
try {
write(data.toMutationList(key, preconditionForUpdate(key)));
write(Collections.singletonList(data.toMutation(key, preconditionForUpdate(key))));
} catch (FirebaseFirestoreException e) {
lastWriteError = e;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.google.firebase.firestore.model.mutation.PatchMutation;
import com.google.firebase.firestore.model.mutation.Precondition;
import com.google.firebase.firestore.model.mutation.SetMutation;
import com.google.firebase.firestore.model.mutation.TransformMutation;
import com.google.firebase.firestore.model.mutation.TransformOperation;
import com.google.firebase.firestore.util.Assert;
import java.util.ArrayList;
Expand Down Expand Up @@ -335,23 +334,32 @@ public static class ParsedSetData {
this.fieldTransforms = fieldTransforms;
}

public List<Mutation> toMutationList(DocumentKey key, Precondition precondition) {
ArrayList<Mutation> mutations = new ArrayList<>();
public ObjectValue getData() {
return data;
}

@Nullable
public FieldMask getFieldMask() {
return fieldMask;
}

public List<FieldTransform> getFieldTransforms() {
return fieldTransforms;
}

public Mutation toMutation(DocumentKey key, Precondition precondition) {
if (fieldMask != null) {
mutations.add(new PatchMutation(key, data, fieldMask, precondition));
return new PatchMutation(key, data, fieldMask, precondition, fieldTransforms);
} else {
mutations.add(new SetMutation(key, data, precondition));
}
if (!fieldTransforms.isEmpty()) {
mutations.add(new TransformMutation(key, fieldTransforms));
return new SetMutation(key, data, precondition, fieldTransforms);
}
return mutations;
}
}

/** The result of parsing "update" data (i.e. for an updateData call). */
public static class ParsedUpdateData {
private final ObjectValue data;
// The fieldMask does not include document transforms.
private final FieldMask fieldMask;
private final List<FieldTransform> fieldTransforms;

Expand All @@ -361,17 +369,20 @@ public static class ParsedUpdateData {
this.fieldTransforms = fieldTransforms;
}

public ObjectValue getData() {
return data;
}

public FieldMask getFieldMask() {
return fieldMask;
}

public List<FieldTransform> getFieldTransforms() {
return fieldTransforms;
}

public List<Mutation> toMutationList(DocumentKey key, Precondition precondition) {
ArrayList<Mutation> mutations = new ArrayList<>();
mutations.add(new PatchMutation(key, data, fieldMask, precondition));
if (!fieldTransforms.isEmpty()) {
mutations.add(new TransformMutation(key, fieldTransforms));
}
return mutations;
public Mutation toMutation(DocumentKey key, Precondition precondition) {
return new PatchMutation(key, data, fieldMask, precondition, fieldTransforms);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,14 @@
import com.google.firebase.firestore.model.UnknownDocument;
import com.google.firebase.firestore.model.mutation.Mutation;
import com.google.firebase.firestore.model.mutation.MutationBatch;
import com.google.firebase.firestore.proto.WriteBatch;
import com.google.firebase.firestore.remote.RemoteSerializer;
import com.google.firestore.v1.DocumentTransform.FieldTransform;
import com.google.firestore.v1.Write;
import com.google.firestore.v1.Write.Builder;
import com.google.protobuf.ByteString;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

/** Serializer for values stored in the LocalStore. */
Expand Down Expand Up @@ -171,11 +176,34 @@ MutationBatch decodeMutationBatch(com.google.firebase.firestore.proto.WriteBatch
for (int i = 0; i < baseMutationsCount; i++) {
baseMutations.add(rpcSerializer.decodeMutation(batch.getBaseWrites(i)));
}

int mutationsCount = batch.getWritesCount();
List<Mutation> mutations = new ArrayList<>(mutationsCount);
for (int i = 0; i < mutationsCount; i++) {
mutations.add(rpcSerializer.decodeMutation(batch.getWrites(i)));

// Squash old transform mutations into existing patch or set mutations. The replacement of
// representing `transforms` with `update_transforms` on the SDK means that old `transform`
// mutations stored in IndexedDB need to be updated to `update_transforms`.
// TODO(b/174608374): Remove this code once we perform a schema migration.
WriteBatch.Builder squashedBatchBuilder = WriteBatch.newBuilder();
for (int i = batch.getWritesCount() - 1; i >= 0; --i) {
Write mutation = batch.getWrites(i);
if (mutation.hasTransform()) {
hardAssert(
i >= 1 && !batch.getWrites(i - 1).hasTransform() && batch.getWrites(i - 1).hasUpdate(),
"TransformMutation should be preceded by a patch or set mutation");
Write mutationToJoin = batch.getWrites(i - 1);
Builder newMutationBuilder = Write.newBuilder(mutationToJoin);
for (FieldTransform fieldTransform : mutation.getTransform().getFieldTransformsList()) {

newMutationBuilder.addUpdateTransforms(fieldTransform);
}
mutations.add(rpcSerializer.decodeMutation(newMutationBuilder.build()));
--i;
} else {
mutations.add(rpcSerializer.decodeMutation(mutation));
}
}
Collections.reverse(mutations);
return new MutationBatch(batchId, localWriteTime, baseMutations, mutations);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public LocalWriteResult writeLocally(List<Mutation> mutations) {
List<Mutation> baseMutations = new ArrayList<>();
for (Mutation mutation : mutations) {
ObjectValue baseValue =
mutation.extractBaseValue(existingDocuments.get(mutation.getKey()));
mutation.extractTransformBaseValue(existingDocuments.get(mutation.getKey()));
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@
* `__previous_value__` and `__local_write_time__` fields respectively.
*
* <p>Notes:
* <li>ServerTimestamp Values are created as the result of applying a TransformMutation (see
* TransformMutation.applyTo()). They can only exist in the local view of a document. Therefore
* they do not need to be parsed or serialized.
* <li>ServerTimestamp Values are created as the result of applying a transform. They can only exist
* in the local view of a document. Therefore they do not need to be parsed or serialized.
* <li>When evaluated locally (e.g. via DocumentSnapshot data), they evaluate to null.
* <li>They sort after all Timestamp Values. With respect to other ServerTimestamp Values, they sort
* by their localWriteTime.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.MaybeDocument;
import com.google.firebase.firestore.model.NoDocument;
import com.google.firebase.firestore.model.ObjectValue;
import com.google.firebase.firestore.model.SnapshotVersion;

/** Represents a Delete operation */
Expand Down Expand Up @@ -83,10 +82,4 @@ public MaybeDocument applyToLocalView(

return new NoDocument(getKey(), SnapshotVersion.NONE, /*hasCommittedMutations=*/ false);
}

@Nullable
@Override
public ObjectValue extractBaseValue(@Nullable MaybeDocument maybeDoc) {
return null;
}
}
Loading