Skip to content

Commit 3699360

Browse files
authored
Fix Overlay bug with nested field deletion (#10016)
* Fix Overlay bug with nested field deletion * Add more tests.
1 parent 61dfd02 commit 3699360

File tree

2 files changed

+46
-5
lines changed

2 files changed

+46
-5
lines changed

Firestore/core/src/model/mutation.cc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ TransformMap Mutation::Rep::LocalTransformResults(
167167

168168
absl::optional<Mutation> Mutation::CalculateOverlayMutation(
169169
const MutableDocument& doc, const absl::optional<FieldMask>& mask) {
170-
if ((!doc.has_local_mutations()) || (mask.has_value() && mask->empty())) {
170+
if ((!doc.has_local_mutations())) {
171171
return absl::nullopt;
172172
}
173173

@@ -199,9 +199,13 @@ absl::optional<Mutation> Mutation::CalculateOverlayMutation(
199199
path = path.PopLast();
200200
value = doc_value.Get(path);
201201
}
202-
HARD_ASSERT(value.has_value());
203-
patch_value.Set(
204-
path, Message<google_firestore_v1_Value>(DeepClone(value.value())));
202+
if (value.has_value()) {
203+
patch_value.Set(path, Message<google_firestore_v1_Value>(
204+
DeepClone(value.value())));
205+
} else {
206+
patch_value.Delete(path);
207+
}
208+
205209
mask_set.insert(path);
206210
}
207211
}

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

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ void VerifyOverlayRoundTrips(const MutableDocument& doc,
7373
MutableDocument doc_for_mutations = doc.Clone();
7474
MutableDocument doc_for_overlay = doc.Clone();
7575

76-
absl::optional<FieldMask> mask;
76+
absl::optional<FieldMask> mask = FieldMask();
7777
for (const Mutation& mutation : mutations) {
7878
mask = mutation.ApplyToLocalView(doc_for_mutations, mask, now);
7979
}
@@ -712,6 +712,43 @@ TEST(MutationTest, OverlayWithFieldDeletionOfNestedField) {
712712
VerifyOverlayRoundTrips(doc, {patch1, patch2, patch3});
713713
}
714714

715+
// See: https://github.com/firebase/firebase-ios-sdk/issues/9985
716+
TEST(MutationTest, OverlayWithFieldDeletionOfNestedFieldAndParentField) {
717+
MutableDocument doc = Doc("collection/key", 1, Map("foo", 1));
718+
Mutation patch1 =
719+
PatchMutation("collection/key", Map("foo", "foo-patched-value"),
720+
{{"bar.baz", Increment(1)}});
721+
Mutation patch2 =
722+
PatchMutation("collection/key", Map("foo", "foo-patched-value"),
723+
{{"bar.baz", ServerTimestamp()}, {"a.b.c", Increment(1)}});
724+
Mutation patch3 =
725+
PatchMutation("collection/key", Map("foo", "foo-patched-value"),
726+
{Field("bar.baz"), Field("a.b.c")}, {});
727+
728+
Mutation patch4 =
729+
PatchMutation("collection/key", Map("foo", "foo-patched-value"),
730+
{Field("bar"), Field("a.b")}, {});
731+
732+
VerifyOverlayRoundTrips(doc, {patch1, patch2, patch3, patch4});
733+
}
734+
735+
// See: https://github.com/firebase/firebase-ios-sdk/issues/10018
736+
// Same root cause as OverlayWithFieldDeletionOfNestedFieldAndParentField,
737+
// different way to trigger.
738+
TEST(MutationTest, OverlayWorksWithDeletingSameField) {
739+
MutableDocument doc = Doc("collection/key", 1, Map("foo", 1));
740+
Mutation patch1 =
741+
PatchMutation("collection/key", Map("foo", "foo-patched-value"),
742+
{{"bar", ServerTimestamp()}});
743+
Mutation patch2 = PatchMutation(
744+
"collection/key", Map("foo", "foo-patched-value"), {Field("bar")}, {});
745+
746+
Mutation patch3 = PatchMutation(
747+
"collection/key", Map("foo", "foo-patched-value"), {Field("bar")}, {});
748+
749+
VerifyOverlayRoundTrips(doc, {patch1, patch2, patch3});
750+
}
751+
715752
TEST(MutationTest, OverlayCreatedFromSetToEmptyWithMerge) {
716753
MutableDocument doc = DeletedDoc("collection/key", 1);
717754
Mutation merge = MergeMutation("collection/key", Map(), {});

0 commit comments

Comments
 (0)