Skip to content

Commit 92e5fc7

Browse files
wu-huiwu-hui
committed
Attempt to fix version
1 parent 9401155 commit 92e5fc7

File tree

7 files changed

+43
-44
lines changed

7 files changed

+43
-44
lines changed

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/MutableDocument.java

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -226,13 +226,7 @@ public boolean equals(Object o) {
226226
MutableDocument document = (MutableDocument) o;
227227

228228
if (!key.equals(document.key)) return false;
229-
// TODO(Overlay): The version of the overlay is not correct.
230-
// Example:
231-
// Doc at version 1 + Delete + Set
232-
// With current implementation: Delete sets version to 0 + set keeps version = v0
233-
// With overlay: No delete + set keeps version = v1
234-
// The tests in the original PR are passing because they use version 0
235-
// if (!version.equals(document.version)) return false;
229+
if (!version.equals(document.version)) return false;
236230
if (!documentType.equals(document.documentType)) return false;
237231
if (!documentState.equals(document.documentState)) return false;
238232
return value.equals(document.value);

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/DeleteMutation.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import static com.google.firebase.firestore.util.Assert.hardAssert;
1818

1919
import androidx.annotation.Nullable;
20-
2120
import com.google.firebase.Timestamp;
2221
import com.google.firebase.firestore.model.DocumentKey;
2322
import com.google.firebase.firestore.model.MutableDocument;
@@ -71,7 +70,7 @@ public void applyToRemoteDocument(MutableDocument document, MutationResult mutat
7170

7271
@Override
7372
public @Nullable FieldMask applyToLocalView(
74-
MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime) {
73+
MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime) {
7574
verifyKeyMatches(document);
7675

7776
if (getPrecondition().isValidFor(document)) {

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Mutation.java

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import static com.google.firebase.firestore.util.Assert.hardAssert;
1818

1919
import androidx.annotation.Nullable;
20-
2120
import com.google.firebase.Timestamp;
2221
import com.google.firebase.firestore.model.Document;
2322
import com.google.firebase.firestore.model.DocumentKey;
@@ -73,6 +72,9 @@ public abstract class Mutation {
7372

7473
private final List<FieldTransform> fieldTransforms;
7574

75+
// TODO(Overlay): Serialize this field for local storage.
76+
private SnapshotVersion postMutationVersion = null;
77+
7678
Mutation(DocumentKey key, Precondition precondition) {
7779
this(key, precondition, new ArrayList<>());
7880
}
@@ -141,13 +143,21 @@ void verifyKeyMatches(MutableDocument document) {
141143
"Can only apply a mutation to a document with the same key");
142144
}
143145

146+
Mutation withPostMutationVersion(SnapshotVersion version) {
147+
postMutationVersion = version;
148+
return this;
149+
}
150+
144151
/**
145-
* Returns the version from the given document for use as the result of a mutation. Mutations are
146-
* defined to return the version of the base document only if it is an existing document. Deleted
147-
* and unknown documents have a post-mutation version of {@code SnapshotVersion.NONE}.
152+
* Returns the version for the given document for use as the result of a mutation.
153+
*
154+
* <p>Returns {@code postMutationVersion} if it is set, otherwise returns the version of the base
155+
* document if it is not unknown. Returns {@code SnapshotVersion.NONE} for unknown documents.
148156
*/
149-
static SnapshotVersion getPostMutationVersion(MutableDocument document) {
150-
if (document.isFoundDocument()) {
157+
SnapshotVersion getPostMutationVersion(MutableDocument document) {
158+
if (postMutationVersion != null) {
159+
return postMutationVersion;
160+
} else if (document.isFoundDocument()) {
151161
return document.getVersion();
152162
} else {
153163
return SnapshotVersion.NONE;

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/PatchMutation.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package com.google.firebase.firestore.model.mutation;
1616

1717
import androidx.annotation.Nullable;
18-
1918
import com.google.firebase.Timestamp;
2019
import com.google.firebase.firestore.model.DocumentKey;
2120
import com.google.firebase.firestore.model.FieldPath;
@@ -130,7 +129,7 @@ public void applyToRemoteDocument(MutableDocument document, MutationResult mutat
130129

131130
@Override
132131
public @Nullable FieldMask applyToLocalView(
133-
MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime) {
132+
MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime) {
134133
verifyKeyMatches(document);
135134

136135
if (!getPrecondition().isValidFor(document)) {

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/SetMutation.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package com.google.firebase.firestore.model.mutation;
1616

1717
import androidx.annotation.Nullable;
18-
1918
import com.google.firebase.Timestamp;
2019
import com.google.firebase.firestore.model.DocumentKey;
2120
import com.google.firebase.firestore.model.FieldPath;
@@ -89,7 +88,7 @@ public void applyToRemoteDocument(MutableDocument document, MutationResult mutat
8988

9089
@Override
9190
public FieldMask applyToLocalView(
92-
MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime) {
91+
MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime) {
9392
verifyKeyMatches(document);
9493

9594
if (!this.getPrecondition().isValidFor(document)) {

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/VerifyMutation.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package com.google.firebase.firestore.model.mutation;
1616

1717
import androidx.annotation.Nullable;
18-
1918
import com.google.firebase.Timestamp;
2019
import com.google.firebase.firestore.model.DocumentKey;
2120
import com.google.firebase.firestore.model.MutableDocument;
@@ -64,7 +63,7 @@ public void applyToRemoteDocument(MutableDocument document, MutationResult mutat
6463

6564
@Override
6665
public FieldMask applyToLocalView(
67-
MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime) {
66+
MutableDocument document, @Nullable FieldMask previousMask, Timestamp localWriteTime) {
6867
throw Assert.fail("VerifyMutation should only be used in Transactions.");
6968
}
7069
}

‎firebase-firestore/src/test/java/com/google/firebase/firestore/model/MutationTest.java renamed to ‎firebase-firestore/src/test/java/com/google/firebase/firestore/model/mutation/MutationTest.java

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package com.google.firebase.firestore.model;
15+
package com.google.firebase.firestore.model.mutation;
1616

1717
import static com.google.firebase.firestore.testutil.TestUtil.deleteMutation;
1818
import static com.google.firebase.firestore.testutil.TestUtil.deletedDoc;
@@ -39,15 +39,13 @@
3939
import com.google.common.collect.Sets;
4040
import com.google.firebase.Timestamp;
4141
import com.google.firebase.firestore.FieldValue;
42-
import com.google.firebase.firestore.model.mutation.ArrayTransformOperation;
43-
import com.google.firebase.firestore.model.mutation.DeleteMutation;
44-
import com.google.firebase.firestore.model.mutation.FieldMask;
45-
import com.google.firebase.firestore.model.mutation.FieldTransform;
46-
import com.google.firebase.firestore.model.mutation.Mutation;
47-
import com.google.firebase.firestore.model.mutation.MutationResult;
48-
import com.google.firebase.firestore.model.mutation.PatchMutation;
49-
import com.google.firebase.firestore.model.mutation.Precondition;
50-
import com.google.firebase.firestore.model.mutation.SetMutation;
42+
import com.google.firebase.firestore.model.DocumentKey;
43+
import com.google.firebase.firestore.model.FieldPath;
44+
import com.google.firebase.firestore.model.MutableDocument;
45+
import com.google.firebase.firestore.model.ObjectValue;
46+
import com.google.firebase.firestore.model.ServerTimestamps;
47+
import com.google.firebase.firestore.model.SnapshotVersion;
48+
import com.google.firebase.firestore.model.Values;
5149
import com.google.firestore.v1.Value;
5250
import java.util.Arrays;
5351
import java.util.Collection;
@@ -89,13 +87,13 @@ public void testAppliesPatchToDocuments() {
8987

9088
@Test
9189
public void testAppliesPatchWithMergeToDocuments() {
92-
MutableDocument mergeDoc = deletedDoc("collection/key", 1);
90+
MutableDocument mergeDoc = deletedDoc("collection/key", 0);
9391
Mutation upsert =
9492
mergeMutation(
9593
"collection/key", map("foo.bar", "new-bar-value"), Arrays.asList(field("foo.bar")));
9694
upsert.applyToLocalView(mergeDoc, /* previousMask= */ null, Timestamp.now());
9795
Map<String, Object> expectedData = map("foo", map("bar", "new-bar-value"));
98-
assertEquals(doc("collection/key", 1, expectedData).setHasLocalMutations(), mergeDoc);
96+
assertEquals(doc("collection/key", 0, expectedData).setHasLocalMutations(), mergeDoc);
9997
}
10098

10199
@Test
@@ -990,7 +988,7 @@ private int runPermutationTests(List<MutableDocument> docs, List<Mutation> mutat
990988
Collections2.permutations(Lists.newArrayList(mutations));
991989
for (MutableDocument doc : docs) {
992990
for (List<Mutation> permutation : permutations) {
993-
verifyOverlayRoundTrips(doc, permutation.toArray(new Mutation[]{}));
991+
verifyOverlayRoundTrips(doc, permutation.toArray(new Mutation[] {}));
994992
testCases += 1;
995993
}
996994
}
@@ -1029,25 +1027,26 @@ private void verifyOverlayRoundTrips(MutableDocument doc, Mutation... mutations)
10291027
}
10301028

10311029
Mutation overlay = null;
1032-
boolean isLatencyCompensatedDelete = docForMutations.getVersion().equals(SnapshotVersion.NONE) && docForMutations.isNoDocument();
1030+
boolean isLatencyCompensatedDelete =
1031+
docForMutations.getVersion().equals(SnapshotVersion.NONE) && docForMutations.isNoDocument();
10331032
if (docForMutations.hasLocalMutations() || isLatencyCompensatedDelete) {
1034-
overlay = getOverlayMutation(docForMutations, mask);
1033+
overlay = getOverlayMutation(docForMutations, mask);
10351034
overlay.applyToLocalView(docForOverlay, /* previousMask= */ null, now);
10361035
}
10371036

10381037
assertEquals(
1039-
getDescription(doc, Arrays.asList(mutations), overlay),
1040-
docForOverlay,
1041-
docForMutations);
1038+
getDescription(doc, Arrays.asList(mutations), overlay), docForOverlay, docForMutations);
10421039
}
10431040

10441041
// TODO(Overlay): This is production code, find a place for this.
10451042
private Mutation getOverlayMutation(MutableDocument doc, @Nullable FieldMask mask) {
10461043
if (mask == null) {
10471044
if (doc.isNoDocument()) {
1048-
return new DeleteMutation(doc.getKey(), Precondition.NONE);
1045+
return new DeleteMutation(doc.getKey(), Precondition.NONE)
1046+
.withPostMutationVersion(doc.getVersion());
10491047
} else {
1050-
return new SetMutation(doc.getKey(), doc.getData(), Precondition.NONE);
1048+
return new SetMutation(doc.getKey(), doc.getData(), Precondition.NONE)
1049+
.withPostMutationVersion(doc.getVersion());
10511050
}
10521051
} else {
10531052
ObjectValue docValue = doc.getData();
@@ -1072,9 +1071,9 @@ private Mutation getOverlayMutation(MutableDocument doc, @Nullable FieldMask mas
10721071
maskSet.add(path);
10731072
}
10741073
}
1075-
return
1076-
new PatchMutation(
1077-
doc.getKey(), patchValue, FieldMask.fromSet(maskSet), Precondition.NONE);
1074+
return new PatchMutation(
1075+
doc.getKey(), patchValue, FieldMask.fromSet(maskSet), Precondition.NONE)
1076+
.withPostMutationVersion(doc.getVersion());
10781077
}
10791078
}
10801079
}

0 commit comments

Comments
 (0)