Skip to content

Commit cb2cfcb

Browse files
dconeybewu-hui
dconeybe
and
wu-hui
authored
mutation squashing (#9684)
* mutation squashing * Complete implementation and add manual tests * permutatioins and combinations * Fixes * Fix test failures Co-authored-by: Wu-Hui <[email protected]>
1 parent a18c31b commit cb2cfcb

20 files changed

+542
-74
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ DocumentMap LocalDocumentsView::GetDocumentsMatchingCollectionQuery(
167167
document = MutableDocument::InvalidDocument(key);
168168
}
169169

170-
mutation.ApplyToLocalView(*document, batch.local_write_time());
170+
// TODO(Overlay): Use proper previous mask.
171+
mutation.ApplyToLocalView(*document, absl::nullopt,
172+
batch.local_write_time());
171173
remote_documents = remote_documents.insert(key, *document);
172174
}
173175
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ model::DocumentMap LocalStore::ApplyRemoteEvent(
331331
target_cache_->SetLastRemoteSnapshotVersion(remote_version);
332332
}
333333

334-
return local_documents_->GetLocalViewOfDocuments(changed_docs);
334+
return local_documents_->GetLocalViewOfDocuments(std::move(changed_docs));
335335
});
336336
}
337337

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,19 @@ void DeleteMutation::Rep::ApplyToRemoteDocument(
5656
.SetHasCommittedMutations();
5757
}
5858

59-
void DeleteMutation::Rep::ApplyToLocalView(MutableDocument& document,
60-
const Timestamp&) const {
59+
absl::optional<FieldMask> DeleteMutation::Rep::ApplyToLocalView(
60+
MutableDocument& document,
61+
absl::optional<FieldMask> previous_mask,
62+
const Timestamp&) const {
6163
VerifyKeyMatches(document);
6264

6365
if (precondition().IsValidFor(document)) {
64-
document.ConvertToNoDocument(SnapshotVersion::None());
66+
document.ConvertToNoDocument(SnapshotVersion::None())
67+
.SetHasLocalMutations();
68+
return absl::nullopt;
6569
}
70+
71+
return previous_mask;
6672
}
6773

6874
std::string DeleteMutation::Rep::ToString() const {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,10 @@ class DeleteMutation : public Mutation {
5656
MutableDocument& document,
5757
const MutationResult& mutation_result) const override;
5858

59-
void ApplyToLocalView(MutableDocument& document,
60-
const Timestamp&) const override;
59+
absl::optional<FieldMask> ApplyToLocalView(
60+
MutableDocument& document,
61+
absl::optional<FieldMask> previous_mask,
62+
const Timestamp&) const override;
6163

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

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ class FieldMask {
6767
return fields_.size();
6868
}
6969

70+
bool empty() const {
71+
return fields_.empty();
72+
}
73+
7074
/**
7175
* Verifies that `field_path` is included by at least one field in this field
7276
* mask.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ MutableDocument& MutableDocument::SetHasCommittedMutations() {
9292

9393
MutableDocument& MutableDocument::SetHasLocalMutations() {
9494
document_state_ = DocumentState::kHasLocalMutations;
95+
version_ = SnapshotVersion::None();
9596
return *this;
9697
}
9798

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

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,17 @@
1616

1717
#include "Firestore/core/src/model/mutation.h"
1818

19-
#include <cstdlib>
2019
#include <ostream>
21-
#include <sstream>
20+
#include <set>
2221
#include <utility>
2322

23+
#include "Firestore/core/src/model/delete_mutation.h"
2424
#include "Firestore/core/src/model/document.h"
2525
#include "Firestore/core/src/model/field_path.h"
2626
#include "Firestore/core/src/model/mutable_document.h"
2727
#include "Firestore/core/src/model/object_value.h"
28+
#include "Firestore/core/src/model/patch_mutation.h"
29+
#include "Firestore/core/src/model/set_mutation.h"
2830
#include "Firestore/core/src/nanopb/message.h"
2931
#include "Firestore/core/src/util/hard_assert.h"
3032
#include "Firestore/core/src/util/to_string.h"
@@ -56,9 +58,12 @@ void Mutation::ApplyToRemoteDocument(
5658
return rep().ApplyToRemoteDocument(document, mutation_result);
5759
}
5860

59-
void Mutation::ApplyToLocalView(MutableDocument& document,
60-
const Timestamp& local_write_time) const {
61-
return rep().ApplyToLocalView(document, local_write_time);
61+
absl::optional<FieldMask> Mutation::ApplyToLocalView(
62+
MutableDocument& document,
63+
absl::optional<FieldMask> previous_mask,
64+
const Timestamp& local_write_time) const {
65+
return rep().ApplyToLocalView(document, std::move(previous_mask),
66+
local_write_time);
6267
}
6368

6469
absl::optional<ObjectValue> Mutation::Rep::ExtractTransformBaseValue(
@@ -150,6 +155,51 @@ TransformMap Mutation::Rep::LocalTransformResults(
150155
return transform_results;
151156
}
152157

158+
absl::optional<Mutation> Mutation::CalculateOverlayMutation(
159+
const MutableDocument& doc, const absl::optional<FieldMask>& mask) {
160+
if ((!doc.has_local_mutations()) || (mask.has_value() && mask->empty())) {
161+
return absl::nullopt;
162+
}
163+
164+
// !mask.has_value() when there are Set or Delete being applied to get to the
165+
// current document.
166+
if (!mask.has_value()) {
167+
if (doc.is_no_document()) {
168+
return DeleteMutation(doc.key(), Precondition::None());
169+
} else {
170+
return SetMutation(doc.key(), doc.data(), Precondition::None());
171+
}
172+
} else {
173+
const ObjectValue& doc_value = doc.data();
174+
ObjectValue patch_value;
175+
std::set<FieldPath> mask_set;
176+
for (FieldPath path : mask.value()) {
177+
if (mask_set.find(path) == mask_set.end()) {
178+
absl::optional<google_firestore_v1_Value> value = doc_value.Get(path);
179+
// If we are deleting a nested field, we take the immediate parent as
180+
// the mask used to construct resulting mutation.
181+
// Justification: Nested fields can create parent fields implicitly. If
182+
// only a leaf entry is deleted in later mutations, the parent field
183+
// should still remain, but we may have lost this information.
184+
// Consider mutation (foo.bar 1), then mutation (foo.bar delete()).
185+
// This leaves the final result (foo, {}). Despite the fact that `doc`
186+
// has the correct result, `foo` is not in `mask`, and the resulting
187+
// mutation would miss `foo`.
188+
if (!value.has_value() && path.size() > 1) {
189+
path = path.PopLast();
190+
value = doc_value.Get(path);
191+
}
192+
HARD_ASSERT(value.has_value());
193+
patch_value.Set(
194+
path, Message<google_firestore_v1_Value>(DeepClone(value.value())));
195+
mask_set.insert(path);
196+
}
197+
}
198+
return PatchMutation(doc.key(), std::move(patch_value),
199+
FieldMask(std::move(mask_set)), Precondition::None());
200+
}
201+
}
202+
153203
bool operator==(const Mutation& lhs, const Mutation& rhs) {
154204
return lhs.rep_ == nullptr
155205
? rhs.rep_ == nullptr

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,10 @@ class Mutation {
211211
* @param local_write_time A timestamp indicating the local write time of the
212212
* batch this mutation is a part of.
213213
*/
214-
void ApplyToLocalView(MutableDocument& document,
215-
const Timestamp& local_write_time) const;
214+
absl::optional<FieldMask> ApplyToLocalView(
215+
MutableDocument& document,
216+
absl::optional<FieldMask> previous_mask,
217+
const Timestamp& local_write_time) const;
216218

217219
/**
218220
* If this mutation is not idempotent, returns the base value to persist with
@@ -235,6 +237,18 @@ class Mutation {
235237
return rep_->ExtractTransformBaseValue(document);
236238
}
237239

240+
/**
241+
* A utility method to calculate an `Mutation` representing the overlay from
242+
* the final state of the document, and a `FieldMask` representing the fields
243+
* that are mutated by the local mutations, or a `absl::nullopt` if all fields
244+
* should be overwritten.
245+
*
246+
* Returns a mutation that can be applied to a base document to get `doc`, or
247+
* `nullopt` if base document and `doc` is the same.
248+
*/
249+
static absl::optional<Mutation> CalculateOverlayMutation(
250+
const MutableDocument& doc, const absl::optional<FieldMask>& mask);
251+
238252
friend bool operator==(const Mutation& lhs, const Mutation& rhs);
239253

240254
size_t Hash() const {
@@ -276,8 +290,10 @@ class Mutation {
276290
MutableDocument& document,
277291
const MutationResult& mutation_result) const = 0;
278292

279-
virtual void ApplyToLocalView(MutableDocument& document,
280-
const Timestamp& local_write_time) const = 0;
293+
virtual absl::optional<FieldMask> ApplyToLocalView(
294+
MutableDocument& document,
295+
absl::optional<FieldMask> previous_mask,
296+
const Timestamp& local_write_time) const = 0;
281297

282298
virtual absl::optional<ObjectValue> ExtractTransformBaseValue(
283299
const Document& document) const;

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,16 @@ void MutationBatch::ApplyToLocalDocument(MutableDocument& document) const {
6262
// transform against a consistent set of values.
6363
for (const Mutation& mutation : base_mutations_) {
6464
if (mutation.key() == document.key()) {
65-
mutation.ApplyToLocalView(document, local_write_time_);
65+
// TODO(Overlay): Use proper previous_mask
66+
mutation.ApplyToLocalView(document, absl::nullopt, local_write_time_);
6667
}
6768
}
6869

6970
// Second, apply all user-provided mutations.
7071
for (const Mutation& mutation : mutations_) {
7172
if (mutation.key() == document.key()) {
72-
mutation.ApplyToLocalView(document, local_write_time_);
73+
// TODO(Overlay): Use proper previous_mask
74+
mutation.ApplyToLocalView(document, absl::nullopt, local_write_time_);
7375
}
7476
}
7577
}

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

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "Firestore/core/src/model/patch_mutation.h"
1818

1919
#include <cstdlib>
20+
#include <set>
2021
#include <utility>
2122

2223
#include "Firestore/core/src/model/field_path.h"
@@ -94,20 +95,34 @@ void PatchMutation::Rep::ApplyToRemoteDocument(
9495
.SetHasCommittedMutations();
9596
}
9697

97-
void PatchMutation::Rep::ApplyToLocalView(
98-
MutableDocument& document, const Timestamp& local_write_time) const {
98+
absl::optional<FieldMask> PatchMutation::Rep::ApplyToLocalView(
99+
MutableDocument& document,
100+
absl::optional<FieldMask> previous_mask,
101+
const Timestamp& local_write_time) const {
99102
VerifyKeyMatches(document);
100103

101104
if (!precondition().IsValidFor(document)) {
102-
return;
105+
return previous_mask;
103106
}
104107

105108
ObjectValue& data = document.data();
106109
auto transform_results = LocalTransformResults(data, local_write_time);
107110
data.SetAll(GetPatch());
108111
data.SetAll(std::move(transform_results));
109-
document.ConvertToFoundDocument(GetPostMutationVersion(document))
110-
.SetHasLocalMutations();
112+
document.ConvertToFoundDocument(document.version()).SetHasLocalMutations();
113+
114+
if (!previous_mask.has_value()) {
115+
return absl::nullopt;
116+
}
117+
118+
std::set<FieldPath> merged_set(previous_mask.value().begin(),
119+
previous_mask.value().end());
120+
merged_set.insert(mask_.begin(), mask_.end());
121+
std::vector<FieldPath> transformed;
122+
for (const auto& transform : this->field_transforms()) {
123+
merged_set.insert(transform.path());
124+
}
125+
return FieldMask{merged_set};
111126
}
112127

113128
TransformMap PatchMutation::Rep::GetPatch() const {

0 commit comments

Comments
 (0)