Skip to content

Commit 6b64d5e

Browse files
author
Brian Chen
committed
Remove baseDoc check in localTransformResults()
1 parent b4f6400 commit 6b64d5e

13 files changed

+19
-48
lines changed

‎Firestore/core/src/local/local_documents_view.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,8 @@ DocumentMap LocalDocumentsView::GetDocumentsMatchingCollectionQuery(
187187
absl::optional<MaybeDocument> base_doc =
188188
results.underlying_map().get(key);
189189

190-
absl::optional<MaybeDocument> mutated_doc = mutation.ApplyToLocalView(
191-
base_doc, base_doc, batch.local_write_time());
190+
absl::optional<MaybeDocument> mutated_doc =
191+
mutation.ApplyToLocalView(base_doc, batch.local_write_time());
192192

193193
if (mutated_doc && mutated_doc->is_document()) {
194194
results = results.insert(key, Document(*mutated_doc));

‎Firestore/core/src/model/delete_mutation.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ MaybeDocument DeleteMutation::Rep::ApplyToRemoteDocument(
6060
}
6161

6262
absl::optional<MaybeDocument> DeleteMutation::Rep::ApplyToLocalView(
63-
const absl::optional<MaybeDocument>& maybe_doc,
64-
const absl::optional<MaybeDocument>&,
65-
const Timestamp&) const {
63+
const absl::optional<MaybeDocument>& maybe_doc, const Timestamp&) const {
6664
VerifyKeyMatches(maybe_doc);
6765

6866
if (!precondition().IsValidFor(maybe_doc)) {

‎Firestore/core/src/model/delete_mutation.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ class DeleteMutation : public Mutation {
5858

5959
absl::optional<MaybeDocument> ApplyToLocalView(
6060
const absl::optional<MaybeDocument>& maybe_doc,
61-
const absl::optional<MaybeDocument>&,
6261
const Timestamp&) const override;
6362

6463
// Does not override Equals or Hash; Mutation's versions are sufficient.

‎Firestore/core/src/model/mutation.cc

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,8 @@ MaybeDocument Mutation::ApplyToRemoteDocument(
5656

5757
absl::optional<MaybeDocument> Mutation::ApplyToLocalView(
5858
const absl::optional<MaybeDocument>& maybe_doc,
59-
const absl::optional<MaybeDocument>& base_doc,
6059
const Timestamp& local_write_time) const {
61-
return rep().ApplyToLocalView(maybe_doc, base_doc, local_write_time);
60+
return rep().ApplyToLocalView(maybe_doc, local_write_time);
6261
}
6362

6463
absl::optional<ObjectValue> Mutation::Rep::ExtractTransformBaseValue(
@@ -147,7 +146,6 @@ std::vector<FieldValue> Mutation::Rep::ServerTransformResults(
147146

148147
std::vector<FieldValue> Mutation::Rep::LocalTransformResults(
149148
const absl::optional<MaybeDocument>& maybe_doc,
150-
const absl::optional<MaybeDocument>& base_doc,
151149
const Timestamp& local_write_time) const {
152150
std::vector<FieldValue> transform_results;
153151
for (const FieldTransform& field_transform : field_transforms_) {
@@ -158,15 +156,6 @@ std::vector<FieldValue> Mutation::Rep::LocalTransformResults(
158156
previous_value = Document(*maybe_doc).field(field_transform.path());
159157
}
160158

161-
// TODO: remove this part
162-
if (!previous_value && base_doc && base_doc->is_document()) {
163-
// If the current document does not contain a value for the mutated field,
164-
// use the value that existed before applying this mutation batch. This
165-
// solves an edge case where a PatchMutation clears the values in a nested
166-
// map before the TransformMutation is applied.
167-
previous_value = Document(*base_doc).field(field_transform.path());
168-
}
169-
170159
transform_results.push_back(
171160
transform.ApplyToLocalView(previous_value, local_write_time));
172161
}

‎Firestore/core/src/model/mutation.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,6 @@ class Mutation {
222222
* @param maybe_doc The document to mutate. The input document can be
223223
* `nullopt` if the client has no knowledge of the pre-mutation state of
224224
* the document.
225-
* @param base_doc The state of the document prior to this mutation batch. The
226-
* input document can be nullopt if the client has no knowledge of the
227-
* pre-mutation state of the document.
228225
* @param local_write_time A timestamp indicating the local write time of the
229226
* batch this mutation is a part of.
230227
* @return The mutated document. The returned document may be nullopt, but
@@ -233,7 +230,6 @@ class Mutation {
233230
*/
234231
absl::optional<MaybeDocument> ApplyToLocalView(
235232
const absl::optional<MaybeDocument>& maybe_doc,
236-
const absl::optional<MaybeDocument>& base_doc,
237233
const Timestamp& local_write_time) const;
238234

239235
/**
@@ -300,7 +296,6 @@ class Mutation {
300296

301297
virtual absl::optional<MaybeDocument> ApplyToLocalView(
302298
const absl::optional<MaybeDocument>& maybe_doc,
303-
const absl::optional<MaybeDocument>& base_doc,
304299
const Timestamp& local_write_time) const = 0;
305300

306301
virtual absl::optional<ObjectValue> ExtractTransformBaseValue(
@@ -334,7 +329,6 @@ class Mutation {
334329
*/
335330
virtual std::vector<FieldValue> LocalTransformResults(
336331
const absl::optional<MaybeDocument>& maybe_doc,
337-
const absl::optional<MaybeDocument>& base_doc,
338332
const Timestamp& local_write_time) const;
339333

340334
virtual ObjectValue TransformObject(

‎Firestore/core/src/model/mutation_batch.cc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ absl::optional<MaybeDocument> MutationBatch::ApplyToLocalDocument(
7474
// transform against a consistent set of values.
7575
for (const Mutation& mutation : base_mutations_) {
7676
if (mutation.key() == document_key) {
77-
maybe_doc =
78-
mutation.ApplyToLocalView(maybe_doc, maybe_doc, local_write_time_);
77+
maybe_doc = mutation.ApplyToLocalView(maybe_doc, local_write_time_);
7978
}
8079
}
8180

@@ -84,8 +83,7 @@ absl::optional<MaybeDocument> MutationBatch::ApplyToLocalDocument(
8483
// Second, apply all user-provided mutations.
8584
for (const Mutation& mutation : mutations_) {
8685
if (mutation.key() == document_key) {
87-
maybe_doc =
88-
mutation.ApplyToLocalView(maybe_doc, base_doc, local_write_time_);
86+
maybe_doc = mutation.ApplyToLocalView(maybe_doc, local_write_time_);
8987
}
9088
}
9189
return maybe_doc;

‎Firestore/core/src/model/patch_mutation.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ MaybeDocument PatchMutation::Rep::ApplyToRemoteDocument(
100100

101101
absl::optional<MaybeDocument> PatchMutation::Rep::ApplyToLocalView(
102102
const absl::optional<MaybeDocument>& maybe_doc,
103-
const absl::optional<MaybeDocument>& base_doc,
104103
const Timestamp& local_write_time) const {
105104
VerifyKeyMatches(maybe_doc);
106105

@@ -109,7 +108,7 @@ absl::optional<MaybeDocument> PatchMutation::Rep::ApplyToLocalView(
109108
}
110109

111110
std::vector<FieldValue> transform_results =
112-
LocalTransformResults(maybe_doc, base_doc, local_write_time);
111+
LocalTransformResults(maybe_doc, local_write_time);
113112

114113
ObjectValue new_data = PatchDocument(maybe_doc, transform_results);
115114
SnapshotVersion version = GetPostMutationVersion(maybe_doc);

‎Firestore/core/src/model/patch_mutation.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ class PatchMutation : public Mutation {
108108

109109
absl::optional<MaybeDocument> ApplyToLocalView(
110110
const absl::optional<MaybeDocument>& maybe_doc,
111-
const absl::optional<MaybeDocument>& base_doc,
112111
const Timestamp& local_write_time) const override;
113112

114113
bool Equals(const Mutation::Rep& other) const override;

‎Firestore/core/src/model/set_mutation.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ MaybeDocument SetMutation::Rep::ApplyToRemoteDocument(
8989

9090
absl::optional<MaybeDocument> SetMutation::Rep::ApplyToLocalView(
9191
const absl::optional<MaybeDocument>& maybe_doc,
92-
const absl::optional<MaybeDocument>& base_doc,
9392
const Timestamp& local_write_time) const {
9493
VerifyKeyMatches(maybe_doc);
9594

@@ -98,7 +97,7 @@ absl::optional<MaybeDocument> SetMutation::Rep::ApplyToLocalView(
9897
}
9998

10099
std::vector<FieldValue> transforms_results =
101-
LocalTransformResults(maybe_doc, base_doc, local_write_time);
100+
LocalTransformResults(maybe_doc, local_write_time);
102101
ObjectValue new_data = TransformObject(value_, transforms_results);
103102

104103
SnapshotVersion version = GetPostMutationVersion(maybe_doc);

‎Firestore/core/src/model/set_mutation.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ class SetMutation : public Mutation {
7979

8080
absl::optional<MaybeDocument> ApplyToLocalView(
8181
const absl::optional<MaybeDocument>& maybe_doc,
82-
const absl::optional<MaybeDocument>& base_doc,
8382
const Timestamp& local_write_time) const override;
8483

8584
bool Equals(const Mutation::Rep& other) const override;

‎Firestore/core/src/model/verify_mutation.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,7 @@ MaybeDocument VerifyMutation::Rep::ApplyToRemoteDocument(
4444
}
4545

4646
absl::optional<MaybeDocument> VerifyMutation::Rep::ApplyToLocalView(
47-
const absl::optional<MaybeDocument>&,
48-
const absl::optional<MaybeDocument>&,
49-
const Timestamp&) const {
47+
const absl::optional<MaybeDocument>&, const Timestamp&) const {
5048
HARD_FAIL("VerifyMutation should only be used in Transactions.");
5149
}
5250

‎Firestore/core/src/model/verify_mutation.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ class VerifyMutation : public Mutation {
6969

7070
absl::optional<MaybeDocument> ApplyToLocalView(
7171
const absl::optional<MaybeDocument>& maybe_doc,
72-
const absl::optional<MaybeDocument>&,
7372
const Timestamp&) const override;
7473

7574
// Does not override Equals or Hash; Mutation's versions are sufficient.

‎Firestore/core/test/unit/model/mutation_test.cc

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ TEST(MutationTest, AppliesSetsToDocuments) {
5555
Doc("collection/key", 0, Map("foo", "foo-value", "baz", "baz-value"));
5656

5757
Mutation set = SetMutation("collection/key", Map("bar", "bar-value"));
58-
auto result = set.ApplyToLocalView(base_doc, base_doc, now);
58+
auto result = set.ApplyToLocalView(base_doc, now);
5959

6060
EXPECT_EQ(result, Doc("collection/key", 0, Map("bar", "bar-value"),
6161
DocumentState::kLocalMutations));
@@ -68,7 +68,7 @@ TEST(MutationTest, AppliesPatchToDocuments) {
6868

6969
Mutation patch =
7070
PatchMutation("collection/key", Map("foo.bar", "new-bar-value"));
71-
auto result = patch.ApplyToLocalView(base_doc, base_doc, now);
71+
auto result = patch.ApplyToLocalView(base_doc, now);
7272

7373
EXPECT_EQ(result,
7474
Doc("collection/key", 0,
@@ -81,7 +81,7 @@ TEST(MutationTest, AppliesPatchWithMergeToNoDocuments) {
8181

8282
Mutation upsert = MergeMutation(
8383
"collection/key", Map("foo.bar", "new-bar-value"), {Field("foo.bar")});
84-
auto result = upsert.ApplyToLocalView(base_doc, base_doc, now);
84+
auto result = upsert.ApplyToLocalView(base_doc, now);
8585

8686
EXPECT_EQ(result,
8787
Doc("collection/key", 0, Map("foo", Map("bar", "new-bar-value")),
@@ -93,7 +93,7 @@ TEST(MutationTest, AppliesPatchWithMergeToNullDocuments) {
9393

9494
Mutation upsert = MergeMutation(
9595
"collection/key", Map("foo.bar", "new-bar-value"), {Field("foo.bar")});
96-
auto result = upsert.ApplyToLocalView(base_doc, base_doc, now);
96+
auto result = upsert.ApplyToLocalView(base_doc, now);
9797

9898
EXPECT_EQ(result,
9999
Doc("collection/key", 0, Map("foo", Map("bar", "new-bar-value")),
@@ -106,7 +106,7 @@ TEST(MutationTest, DeletesValuesFromTheFieldMask) {
106106
Map("foo", Map("bar", "bar-value", "baz", "baz-value")));
107107

108108
Mutation patch = MergeMutation("collection/key", Map(), {Field("foo.bar")});
109-
auto result = patch.ApplyToLocalView(base_doc, base_doc, now);
109+
auto result = patch.ApplyToLocalView(base_doc, now);
110110

111111
EXPECT_EQ(result,
112112
Doc("collection/key", 0, Map("foo", Map("baz", "baz-value")),
@@ -119,7 +119,7 @@ TEST(MutationTest, PatchesPrimitiveValue) {
119119

120120
Mutation patch =
121121
PatchMutation("collection/key", Map("foo.bar", "new-bar-value"));
122-
auto result = patch.ApplyToLocalView(base_doc, base_doc, now);
122+
auto result = patch.ApplyToLocalView(base_doc, now);
123123

124124
EXPECT_EQ(result,
125125
Doc("collection/key", 0,
@@ -131,7 +131,7 @@ TEST(MutationTest, PatchingDeletedDocumentsDoesNothing) {
131131
NoDocument base_doc = testutil::DeletedDoc("collection/key", 0);
132132

133133
Mutation patch = PatchMutation("collection/key", Map("foo", "bar"));
134-
auto result = patch.ApplyToLocalView(base_doc, base_doc, now);
134+
auto result = patch.ApplyToLocalView(base_doc, now);
135135

136136
EXPECT_EQ(result, base_doc);
137137
}
@@ -143,7 +143,7 @@ TEST(MutationTest, AppliesLocalServerTimestampTransformToDocuments) {
143143

144144
Mutation transform = PatchMutation("collection/key", Map(),
145145
{{"foo.bar", ServerTimestampTransform()}});
146-
auto result = transform.ApplyToLocalView(base_doc, base_doc, now);
146+
auto result = transform.ApplyToLocalView(base_doc, now);
147147

148148
// Server timestamps aren't parsed, so we manually insert it.
149149
ObjectValue expected_data =
@@ -178,7 +178,7 @@ void TransformBaseDoc(const FieldValue::Map& base_data,
178178

179179
for (const auto& transform : transforms) {
180180
Mutation mutation = PatchMutation("collection/key", Map(), {transform});
181-
auto result = mutation.ApplyToLocalView(current_doc, current_doc, now);
181+
auto result = mutation.ApplyToLocalView(current_doc, now);
182182
ASSERT_NE(result, absl::nullopt);
183183
ASSERT_EQ(result->type(), MaybeDocument::Type::Document);
184184
current_doc = Document(*result);
@@ -471,7 +471,7 @@ TEST(MutationTest, DeleteDeletes) {
471471
Document base_doc = Doc("collection/key", 0, Map("foo", "bar"));
472472

473473
Mutation del = DeleteMutation("collection/key");
474-
auto result = del.ApplyToLocalView(base_doc, base_doc, now);
474+
auto result = del.ApplyToLocalView(base_doc, now);
475475

476476
EXPECT_EQ(result, DeletedDoc("collection/key", 0));
477477
}

0 commit comments

Comments
 (0)