Skip to content

Remove unused baseDoc argument #2340

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.

Sign up for GitHub

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 1 commit into from
Jan 20, 2021
Merged

Remove unused baseDoc argument #2340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection

DocumentKey key = mutation.getKey();
MaybeDocument baseDoc = results.get(key);
MaybeDocument mutatedDoc =
mutation.applyToLocalView(baseDoc, baseDoc, batch.getLocalWriteTime());
MaybeDocument mutatedDoc = mutation.applyToLocalView(baseDoc, batch.getLocalWriteTime());
if (mutatedDoc instanceof Document) {
results = results.insert(key, (Document) mutatedDoc);
} else {
Expand Down
2 changes: 1 addition & 1 deletion ...-firestore/src/main/java/com/google/firebase/firestore/model/mutation/DeleteMutation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public MaybeDocument applyToRemoteDocument(
@Nullable
@Override
public MaybeDocument applyToLocalView(
@Nullable MaybeDocument maybeDoc, @Nullable MaybeDocument baseDoc, Timestamp localWriteTime) {
@Nullable MaybeDocument maybeDoc, Timestamp localWriteTime) {
verifyKeyMatches(maybeDoc);

if (!this.getPrecondition().isValidFor(maybeDoc)) {
Expand Down
15 changes: 6 additions & 9 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Mutation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,14 @@ public abstract MaybeDocument applyToRemoteDocument(
*
* @param maybeDoc The document to mutate. The input document can be null if the client has no
* knowledge of the pre-mutation state of the document.
* @param baseDoc The state of the document prior to this mutation batch. The input document can
* be null if the client has no knowledge of the pre-mutation state of the document.
* @param localWriteTime A timestamp indicating the local write time of the batch this mutation is
* a part of.
* @return The mutated document. The returned document may be null, but only if maybeDoc was null
* and the mutation would not create a new document.
*/
@Nullable
public abstract MaybeDocument applyToLocalView(
@Nullable MaybeDocument maybeDoc, @Nullable MaybeDocument baseDoc, Timestamp localWriteTime);
@Nullable MaybeDocument maybeDoc, Timestamp localWriteTime);

/** Helper for derived classes to implement .equals(). */
boolean hasSameKeyAndPrecondition(Mutation other) {
Expand Down Expand Up @@ -164,12 +162,12 @@ static SnapshotVersion getPostMutationVersion(@Nullable MaybeDocument maybeDoc)
* result of applying a transform) for use after a mutation containing transforms has been
* acknowledged by the server.
*
* @param baseDoc The document prior to applying this mutation batch.
* @param maybeDoc The current state of the document after applying all previous mutations.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching the rename. Will port this to the iOS PR as well.

* @param serverTransformResults The transform results received by the server.
* @return The transform results list.
*/
protected List<Value> serverTransformResults(
@Nullable MaybeDocument baseDoc, List<Value> serverTransformResults) {
@Nullable MaybeDocument maybeDoc, List<Value> serverTransformResults) {
ArrayList<Value> transformResults = new ArrayList<>(fieldTransforms.size());
hardAssert(
fieldTransforms.size() == serverTransformResults.size(),
Expand All @@ -182,8 +180,8 @@ protected List<Value> serverTransformResults(
TransformOperation transform = fieldTransform.getOperation();

Value previousValue = null;
if (baseDoc instanceof Document) {
previousValue = ((Document) baseDoc).getField(fieldTransform.getFieldPath());
if (maybeDoc instanceof Document) {
previousValue = ((Document) maybeDoc).getField(fieldTransform.getFieldPath());
}

transformResults.add(
Expand All @@ -198,11 +196,10 @@ protected List<Value> serverTransformResults(
*
* @param localWriteTime The local time of the mutation (used to generate ServerTimestampValues).
* @param maybeDoc The current state of the document after applying all previous mutations.
* @param baseDoc The document prior to applying this mutation batch.
* @return The transform results list.
*/
protected List<Value> localTransformResults(
Timestamp localWriteTime, @Nullable MaybeDocument maybeDoc, @Nullable MaybeDocument baseDoc) {
Timestamp localWriteTime, @Nullable MaybeDocument maybeDoc) {
ArrayList<Value> transformResults = new ArrayList<>(fieldTransforms.size());
for (FieldTransform fieldTransform : fieldTransforms) {
TransformOperation transform = fieldTransform.getOperation();
Expand Down
6 changes: 2 additions & 4 deletions ...e-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,15 @@ public MaybeDocument applyToLocalView(DocumentKey documentKey, @Nullable MaybeDo
for (int i = 0; i < baseMutations.size(); i++) {
Mutation mutation = baseMutations.get(i);
if (mutation.getKey().equals(documentKey)) {
maybeDoc = mutation.applyToLocalView(maybeDoc, maybeDoc, localWriteTime);
maybeDoc = mutation.applyToLocalView(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)) {
maybeDoc = mutation.applyToLocalView(maybeDoc, baseDoc, localWriteTime);
maybeDoc = mutation.applyToLocalView(maybeDoc, localWriteTime);
}
}
return maybeDoc;
Expand Down
4 changes: 2 additions & 2 deletions ...e-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,14 @@ public MaybeDocument applyToRemoteDocument(
@Nullable
@Override
public MaybeDocument applyToLocalView(
@Nullable MaybeDocument maybeDoc, @Nullable MaybeDocument baseDoc, Timestamp localWriteTime) {
@Nullable MaybeDocument maybeDoc, Timestamp localWriteTime) {
verifyKeyMatches(maybeDoc);

if (!getPrecondition().isValidFor(maybeDoc)) {
return maybeDoc;
}

List<Value> transformResults = localTransformResults(localWriteTime, maybeDoc, baseDoc);
List<Value> transformResults = localTransformResults(localWriteTime, maybeDoc);
SnapshotVersion version = getPostMutationVersion(maybeDoc);
ObjectValue newData = patchDocument(maybeDoc, transformResults);
return new Document(getKey(), version, newData, Document.DocumentState.LOCAL_MUTATIONS);
Expand Down
4 changes: 2 additions & 2 deletions ...ase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/SetMutation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,14 @@ public MaybeDocument applyToRemoteDocument(
@Nullable
@Override
public MaybeDocument applyToLocalView(
@Nullable MaybeDocument maybeDoc, @Nullable MaybeDocument baseDoc, Timestamp localWriteTime) {
@Nullable MaybeDocument maybeDoc, Timestamp localWriteTime) {
verifyKeyMatches(maybeDoc);

if (!this.getPrecondition().isValidFor(maybeDoc)) {
return maybeDoc;
}

List<Value> transformResults = localTransformResults(localWriteTime, maybeDoc, baseDoc);
List<Value> transformResults = localTransformResults(localWriteTime, maybeDoc);
ObjectValue newData = transformObject(value, transformResults);

SnapshotVersion version = getPostMutationVersion(maybeDoc);
Expand Down
2 changes: 1 addition & 1 deletion ...-firestore/src/main/java/com/google/firebase/firestore/model/mutation/VerifyMutation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public MaybeDocument applyToRemoteDocument(
@Nullable
@Override
public MaybeDocument applyToLocalView(
@Nullable MaybeDocument maybeDoc, @Nullable MaybeDocument baseDoc, Timestamp localWriteTime) {
@Nullable MaybeDocument maybeDoc, Timestamp localWriteTime) {
throw Assert.fail("VerifyMutation should only be used in Transactions.");
}
}
24 changes: 12 additions & 12 deletions firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void testAppliesSetsToDocuments() {
Document baseDoc = doc("collection/key", 0, data);

Mutation set = setMutation("collection/key", map("bar", "bar-value"));
MaybeDocument setDoc = set.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
MaybeDocument setDoc = set.applyToLocalView(baseDoc, Timestamp.now());
assertEquals(
doc("collection/key", 0, map("bar", "bar-value"), Document.DocumentState.LOCAL_MUTATIONS),
setDoc);
Expand All @@ -75,7 +75,7 @@ public void testAppliesPatchToDocuments() {
Document baseDoc = doc("collection/key", 0, data);

Mutation patch = patchMutation("collection/key", map("foo.bar", "new-bar-value"));
MaybeDocument local = patch.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
MaybeDocument local = patch.applyToLocalView(baseDoc, Timestamp.now());
Map<String, Object> expectedData = map("foo", map("bar", "new-bar-value"), "baz", "baz-value");
assertEquals(
doc("collection/key", 0, expectedData, Document.DocumentState.LOCAL_MUTATIONS), local);
Expand All @@ -87,7 +87,7 @@ public void testAppliesPatchWithMergeToDocuments() {
Mutation upsert =
mergeMutation(
"collection/key", map("foo.bar", "new-bar-value"), Arrays.asList(field("foo.bar")));
MaybeDocument newDoc = upsert.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
MaybeDocument newDoc = upsert.applyToLocalView(baseDoc, Timestamp.now());
Map<String, Object> expectedData = map("foo", map("bar", "new-bar-value"));
assertEquals(
doc("collection/key", 0, expectedData, Document.DocumentState.LOCAL_MUTATIONS), newDoc);
Expand All @@ -99,7 +99,7 @@ public void testAppliesPatchToNullDocWithMergeToDocuments() {
Mutation upsert =
mergeMutation(
"collection/key", map("foo.bar", "new-bar-value"), Arrays.asList(field("foo.bar")));
MaybeDocument newDoc = upsert.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
MaybeDocument newDoc = upsert.applyToLocalView(baseDoc, Timestamp.now());
Map<String, Object> expectedData = map("foo", map("bar", "new-bar-value"));
assertEquals(
doc("collection/key", 0, expectedData, Document.DocumentState.LOCAL_MUTATIONS), newDoc);
Expand All @@ -114,7 +114,7 @@ public void testDeletesValuesFromTheFieldMask() {
FieldMask mask = fieldMask("foo.bar");
Mutation patch = new PatchMutation(key, ObjectValue.emptyObject(), mask, Precondition.NONE);

MaybeDocument patchDoc = patch.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
MaybeDocument patchDoc = patch.applyToLocalView(baseDoc, Timestamp.now());
Map<String, Object> expectedData = map("foo", map("baz", "baz-value"));
assertEquals(
doc("collection/key", 0, expectedData, Document.DocumentState.LOCAL_MUTATIONS), patchDoc);
Expand All @@ -126,7 +126,7 @@ public void testPatchesPrimitiveValue() {
Document baseDoc = doc("collection/key", 0, data);

Mutation patch = patchMutation("collection/key", map("foo.bar", "new-bar-value"));
MaybeDocument patchedDoc = patch.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
MaybeDocument patchedDoc = patch.applyToLocalView(baseDoc, Timestamp.now());
Map<String, Object> expectedData = map("foo", map("bar", "new-bar-value"), "baz", "baz-value");
assertEquals(
doc("collection/key", 0, expectedData, Document.DocumentState.LOCAL_MUTATIONS), patchedDoc);
Expand All @@ -136,7 +136,7 @@ public void testPatchesPrimitiveValue() {
public void testPatchingDeletedDocumentsDoesNothing() {
MaybeDocument baseDoc = deletedDoc("collection/key", 0);
Mutation patch = patchMutation("collection/key", map("foo", "bar"));
MaybeDocument patchedDoc = patch.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
MaybeDocument patchedDoc = patch.applyToLocalView(baseDoc, Timestamp.now());
assertEquals(baseDoc, patchedDoc);
}

Expand All @@ -148,7 +148,7 @@ public void testAppliesLocalServerTimestampTransformsToDocuments() {
Timestamp timestamp = Timestamp.now();
Mutation transform =
patchMutation("collection/key", map("foo.bar", FieldValue.serverTimestamp()));
MaybeDocument transformedDoc = transform.applyToLocalView(baseDoc, baseDoc, timestamp);
MaybeDocument transformedDoc = transform.applyToLocalView(baseDoc, timestamp);

// Server timestamps aren't parsed, so we manually insert it.
ObjectValue expectedData =
Expand Down Expand Up @@ -462,7 +462,7 @@ private void verifyTransform(

for (Map<String, Object> transformData : transforms) {
PatchMutation transform = patchMutation("collection/key", transformData);
currentDoc = transform.applyToLocalView(currentDoc, currentDoc, Timestamp.now());
currentDoc = transform.applyToLocalView(currentDoc, Timestamp.now());
}

Document expectedDoc =
Expand Down Expand Up @@ -544,7 +544,7 @@ public void testDeleteDeletes() {
Document baseDoc = doc("collection/key", 0, data);

Mutation delete = deleteMutation("collection/key");
MaybeDocument deletedDoc = delete.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
MaybeDocument deletedDoc = delete.applyToLocalView(baseDoc, Timestamp.now());
assertEquals(deletedDoc("collection/key", 0), deletedDoc);
}

Expand Down Expand Up @@ -691,8 +691,8 @@ public void testIncrementTwice() {
Map<String, Object> increment = map("sum", FieldValue.increment(1));
Mutation mutation = patchMutation("collection/key", increment);

MaybeDocument mutatedDoc = mutation.applyToLocalView(baseDoc, baseDoc, Timestamp.now());
mutatedDoc = mutation.applyToLocalView(mutatedDoc, baseDoc, Timestamp.now());
MaybeDocument mutatedDoc = mutation.applyToLocalView(baseDoc, Timestamp.now());
mutatedDoc = mutation.applyToLocalView(mutatedDoc, Timestamp.now());

assertEquals(wrap(2L), ((Document) mutatedDoc).getField(field("sum")));
}
Expand Down