-
Notifications
You must be signed in to change notification settings - Fork 617
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,9 @@ | |
import com.google.firebase.firestore.model.mutation.Mutation; | ||
import com.google.firebase.firestore.model.mutation.MutationBatch; | ||
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.List; | ||
|
@@ -171,6 +174,35 @@ MutationBatch decodeMutationBatch(com.google.firebase.firestore.proto.WriteBatch | |
for (int i = 0; i < baseMutationsCount; i++) { | ||
baseMutations.add(rpcSerializer.decodeMutation(batch.getBaseWrites(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. | ||
for (int i = batch.getWritesCount() - 1; i >= 0; --i) { | ||
Write mutation = batch.getWrites(i); | ||
if (mutation.getTransform().getFieldTransformsCount() != 0) { | ||
hardAssert( | ||
i >= 1 | ||
&& batch.getWrites(i - 1).getTransform().getFieldTransformsCount() == 0 | ||
&& batch.getWrites(i - 1).getUpdate().getFieldsCount() != 0, | ||
"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); | ||
} | ||
|
||
batch = | ||
com.google.firebase.firestore.proto.WriteBatch.newBuilder(batch) | ||
.removeWrites(i) | ||
.removeWrites(i - 1) | ||
.addWrites(i - 1, newMutationBuilder.build()) | ||
.build(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you change this so it only runs once per batch? |
||
--i; | ||
} | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another idea would be to simply build up a new list of Mutations, which we can then iterate in the three lines below. That would avoid all toBuilder() conversions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Used a builder to accumulate everything before building at the end of the loop There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like prepending an element to a list in Protobuf is an O(n) operation. Would it be possible to reverse this loop so we can append at the end? We could also just scrap the Builder entirely and just build up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ended up using mutations directly. |
||
int mutationsCount = batch.getWritesCount(); | ||
List<Mutation> mutations = new ArrayList<>(mutationsCount); | ||
for (int i = 0; i < mutationsCount; i++) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proto3 does have "has" functions for nested messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.