diff --git a/Firestore/Source/API/FIRQuery.mm b/Firestore/Source/API/FIRQuery.mm index 3d5a5cc5045..86249f0ab75 100644 --- a/Firestore/Source/API/FIRQuery.mm +++ b/Firestore/Source/API/FIRQuery.mm @@ -50,8 +50,8 @@ #include "Firestore/core/src/model/resource_path.h" #include "Firestore/core/src/model/server_timestamp_util.h" #include "Firestore/core/src/model/value_util.h" -#include "Firestore/core/src/nanopb/nanopb_util.h" #include "Firestore/core/src/nanopb/message.h" +#include "Firestore/core/src/nanopb/nanopb_util.h" #include "Firestore/core/src/util/error_apple.h" #include "Firestore/core/src/util/exception.h" #include "Firestore/core/src/util/hard_assert.h" @@ -632,7 +632,6 @@ - (Bound)boundFromFieldValues:(NSArray *)fieldValues isBefore:(BOOL)isBefore fieldValue.release(); components.values[idx] = *fieldValue; } - } return Bound(components, isBefore); diff --git a/Firestore/core/src/core/user_data.cc b/Firestore/core/src/core/user_data.cc index f1b32bf4eaf..afdaf1d287e 100644 --- a/Firestore/core/src/core/user_data.cc +++ b/Firestore/core/src/core/user_data.cc @@ -72,6 +72,7 @@ void ParseAccumulator::AddToFieldMask(FieldPath field_path) { void ParseAccumulator::AddToFieldTransforms( FieldPath field_path, TransformOperation transform_operation) { + // TODO(mrschmidt): Validate that the paths are unique field_transforms_.emplace_back(std::move(field_path), std::move(transform_operation)); } diff --git a/Firestore/core/src/model/delete_mutation.cc b/Firestore/core/src/model/delete_mutation.cc index 91ba76cff89..cf786617a23 100644 --- a/Firestore/core/src/model/delete_mutation.cc +++ b/Firestore/core/src/model/delete_mutation.cc @@ -19,10 +19,8 @@ #include #include -#include "Firestore/core/src/model/document.h" #include "Firestore/core/src/model/field_path.h" -#include "Firestore/core/src/model/field_value.h" -#include "Firestore/core/src/model/no_document.h" +#include "Firestore/core/src/model/mutable_document.h" #include "Firestore/core/src/util/hard_assert.h" namespace firebase { @@ -41,12 +39,11 @@ DeleteMutation::DeleteMutation(const Mutation& mutation) : Mutation(mutation) { HARD_ASSERT(type() == Type::Delete); } -MaybeDocument DeleteMutation::Rep::ApplyToRemoteDocument( - const absl::optional& maybe_doc, - const MutationResult& mutation_result) const { - VerifyKeyMatches(maybe_doc); +void DeleteMutation::Rep::ApplyToRemoteDocument( + MutableDocument& document, const MutationResult& mutation_result) const { + VerifyKeyMatches(document); - HARD_ASSERT(mutation_result.transform_results() == absl::nullopt, + HARD_ASSERT(mutation_result.transform_results().values_count == 0, "Transform results received by DeleteMutation."); // Unlike ApplyToLocalView, if we're applying a mutation to a remote document @@ -55,20 +52,17 @@ MaybeDocument DeleteMutation::Rep::ApplyToRemoteDocument( // We store the deleted document at the commit version of the delete. Any // document version that the server sends us before the delete was applied is // discarded. - return NoDocument(key(), mutation_result.version(), - /* has_committed_mutations= */ true); + document.ConvertToNoDocument(mutation_result.version()) + .SetHasCommittedMutations(); } -absl::optional DeleteMutation::Rep::ApplyToLocalView( - const absl::optional& maybe_doc, const Timestamp&) const { - VerifyKeyMatches(maybe_doc); +void DeleteMutation::Rep::ApplyToLocalView(MutableDocument& document, + const Timestamp&) const { + VerifyKeyMatches(document); - if (!precondition().IsValidFor(maybe_doc)) { - return maybe_doc; + if (precondition().IsValidFor(document)) { + document.ConvertToNoDocument(SnapshotVersion::None()); } - - return NoDocument(key(), SnapshotVersion::None(), - /* has_committed_mutations= */ false); } std::string DeleteMutation::Rep::ToString() const { diff --git a/Firestore/core/src/model/delete_mutation.h b/Firestore/core/src/model/delete_mutation.h index 130975f7599..a4fc3b576e8 100644 --- a/Firestore/core/src/model/delete_mutation.h +++ b/Firestore/core/src/model/delete_mutation.h @@ -52,13 +52,12 @@ class DeleteMutation : public Mutation { return Type::Delete; } - MaybeDocument ApplyToRemoteDocument( - const absl::optional& maybe_doc, + void ApplyToRemoteDocument( + MutableDocument& document, const MutationResult& mutation_result) const override; - absl::optional ApplyToLocalView( - const absl::optional& maybe_doc, - const Timestamp&) const override; + void ApplyToLocalView(MutableDocument& document, + const Timestamp&) const override; // Does not override Equals or Hash; Mutation's versions are sufficient. diff --git a/Firestore/core/src/model/mutation.cc b/Firestore/core/src/model/mutation.cc index 9feda0d9ffb..228b40bc66c 100644 --- a/Firestore/core/src/model/mutation.cc +++ b/Firestore/core/src/model/mutation.cc @@ -23,8 +23,8 @@ #include "Firestore/core/src/model/document.h" #include "Firestore/core/src/model/field_path.h" -#include "Firestore/core/src/model/field_value.h" -#include "Firestore/core/src/model/no_document.h" +#include "Firestore/core/src/model/mutable_document.h" +#include "Firestore/core/src/model/object_value.h" #include "Firestore/core/src/util/hard_assert.h" #include "Firestore/core/src/util/to_string.h" #include "absl/strings/str_cat.h" @@ -48,39 +48,29 @@ bool operator==(const MutationResult& lhs, const MutationResult& rhs) { lhs.transform_results() == rhs.transform_results(); } -MaybeDocument Mutation::ApplyToRemoteDocument( - const absl::optional& maybe_doc, - const MutationResult& mutation_result) const { - return rep().ApplyToRemoteDocument(maybe_doc, mutation_result); +void Mutation::ApplyToRemoteDocument( + MutableDocument& document, const MutationResult& mutation_result) const { + return rep().ApplyToRemoteDocument(document, mutation_result); } -absl::optional Mutation::ApplyToLocalView( - const absl::optional& maybe_doc, - const Timestamp& local_write_time) const { - return rep().ApplyToLocalView(maybe_doc, local_write_time); +void Mutation::ApplyToLocalView(MutableDocument& document, + const Timestamp& local_write_time) const { + return rep().ApplyToLocalView(document, local_write_time); } absl::optional Mutation::Rep::ExtractTransformBaseValue( - const absl::optional& maybe_doc) const { + const Document& document) const { absl::optional base_object; - absl::optional document; - if (maybe_doc && maybe_doc->is_document()) { - document = Document(*maybe_doc); - } for (const FieldTransform& transform : field_transforms_) { - absl::optional existing_value; - if (document) { - existing_value = document->field(transform.path()); - } - - absl::optional coerced_value = + auto existing_value = document->field(transform.path()); + auto coerced_value = transform.transformation().ComputeBaseValue(existing_value); if (coerced_value) { if (!base_object) { - base_object = ObjectValue::Empty(); + base_object = ObjectValue{}; } - base_object = base_object->Set(transform.path(), *coerced_value); + base_object->Set(transform.path(), *coerced_value); } } @@ -107,79 +97,54 @@ bool Mutation::Rep::Equals(const Mutation::Rep& other) const { field_transforms_ == other.field_transforms_; } -void Mutation::Rep::VerifyKeyMatches( - const absl::optional& maybe_doc) const { - if (maybe_doc) { - HARD_ASSERT(maybe_doc->key() == key(), - "Can only apply a mutation to a document with the same key"); - } +void Mutation::Rep::VerifyKeyMatches(const MutableDocument& document) const { + HARD_ASSERT(document.key() == key(), + "Can only apply a mutation to a document with the same key"); } SnapshotVersion Mutation::Rep::GetPostMutationVersion( - const absl::optional& maybe_doc) { - if (maybe_doc && maybe_doc->type() == MaybeDocument::Type::Document) { - return maybe_doc->version(); + const MutableDocument& document) { + if (document.is_found_document()) { + return document.version(); } else { return SnapshotVersion::None(); } } -std::vector Mutation::Rep::ServerTransformResults( - const absl::optional& maybe_doc, - const std::vector& server_transform_results) const { - HARD_ASSERT(field_transforms_.size() == server_transform_results.size(), +TransformMap Mutation::Rep::ServerTransformResults( + const ObjectValue& previous_data, + const google_firestore_v1_ArrayValue& server_transform_results) const { + TransformMap transform_results; + HARD_ASSERT(field_transforms_.size() == server_transform_results.values_count, "server transform result size (%s) should match field transforms " "size (%s)", - server_transform_results.size(), field_transforms_.size()); + server_transform_results.values_count, field_transforms_.size()); - std::vector transform_results; - for (size_t i = 0; i < server_transform_results.size(); i++) { + for (size_t i = 0; i < server_transform_results.values_count; ++i) { const FieldTransform& field_transform = field_transforms_[i]; const TransformOperation& transform = field_transform.transformation(); - - absl::optional previous_value; - if (maybe_doc && maybe_doc->is_document()) { - previous_value = Document(*maybe_doc).field(field_transform.path()); - } - - transform_results.push_back(transform.ApplyToRemoteDocument( - previous_value, server_transform_results[i])); + const auto& previous_value = previous_data.Get(field_transform.path()); + google_firestore_v1_Value transformed_value = + transform.ApplyToRemoteDocument(previous_value, + server_transform_results.values[i]); + transform_results[field_transform.path()] = transformed_value; } return transform_results; } -std::vector Mutation::Rep::LocalTransformResults( - const absl::optional& maybe_doc, - const Timestamp& local_write_time) const { - std::vector transform_results; +TransformMap Mutation::Rep::LocalTransformResults( + const ObjectValue& previous_data, const Timestamp& local_write_time) const { + TransformMap transform_results; for (const FieldTransform& field_transform : field_transforms_) { const TransformOperation& transform = field_transform.transformation(); - - absl::optional previous_value; - if (maybe_doc && maybe_doc->is_document()) { - previous_value = Document(*maybe_doc).field(field_transform.path()); - } - - transform_results.push_back( - transform.ApplyToLocalView(previous_value, local_write_time)); + const auto& previous_value = previous_data.Get(field_transform.path()); + google_firestore_v1_Value transformed_value = + transform.ApplyToLocalView(*previous_value, local_write_time); + transform_results[field_transform.path()] = transformed_value; } return transform_results; } -ObjectValue Mutation::Rep::TransformObject( - ObjectValue object_value, - const std::vector& transform_results) const { - HARD_ASSERT(transform_results.size() == field_transforms_.size(), - "Transform results size mismatch."); - - for (size_t i = 0; i < field_transforms_.size(); i++) { - const FieldTransform& field_transform = field_transforms_[i]; - const FieldPath& field_path = field_transform.path(); - object_value = object_value.Set(field_path, transform_results[i]); - } - return object_value; -} - bool operator==(const Mutation& lhs, const Mutation& rhs) { return lhs.rep_ == nullptr ? rhs.rep_ == nullptr diff --git a/Firestore/core/src/model/mutation.h b/Firestore/core/src/model/mutation.h index afecdd5dc54..3d162e5e65d 100644 --- a/Firestore/core/src/model/mutation.h +++ b/Firestore/core/src/model/mutation.h @@ -18,23 +18,33 @@ #define FIRESTORE_CORE_SRC_MODEL_MUTATION_H_ #include +#include #include #include #include #include +#include "Firestore/Protos/nanopb/google/firestore/v1/document.nanopb.h" #include "Firestore/core/src/model/document_key.h" #include "Firestore/core/src/model/field_transform.h" -#include "Firestore/core/src/model/field_value.h" +#include "Firestore/core/src/model/object_value.h" #include "Firestore/core/src/model/precondition.h" #include "Firestore/core/src/model/snapshot_version.h" +#include "Firestore/core/src/nanopb/message.h" #include "absl/types/optional.h" namespace firebase { namespace firestore { namespace model { -class MaybeDocument; +// A map of FieldPaths to transforms. Sorted so it can be used in +// ObjectValue::SetAll, which is more efficient it the input map is sorted as +// it processes field maps one layer at a time. +using TransformMap = + std::map>; + +class Document; +class MutableDocument; /** * The result of applying a mutation to the server. This is a model of the @@ -46,10 +56,15 @@ class MaybeDocument; */ class MutationResult { public: - MutationResult( - SnapshotVersion version, - absl::optional> transform_results) - : version_(version), transform_results_(std::move(transform_results)) { + /** Takes ownership of `transform_results`. */ + MutationResult(SnapshotVersion version, + google_firestore_v1_ArrayValue transform_results) + : version_(version), transform_results_{transform_results} { + } + + MutationResult(MutationResult&& other) noexcept + : version_{other.version_}, + transform_results_{std::move(other.transform_results_)} { } /** @@ -73,9 +88,8 @@ class MutationResult { * * Will be nullopt if the mutation was not a TransformMutation. */ - const absl::optional>& transform_results() - const { - return transform_results_; + const google_firestore_v1_ArrayValue& transform_results() const { + return *transform_results_; } std::string ToString() const; @@ -87,7 +101,7 @@ class MutationResult { private: SnapshotVersion version_; - absl::optional> transform_results_; + nanopb::Message transform_results_; }; /** @@ -175,26 +189,19 @@ class Mutation { } /** - * Applies this mutation to the given MaybeDocument for the purposes of - * computing the committed state of the document after the server has - * acknowledged that this mutation has been successfully committed. This - * means that if the input document doesn't match the expected state (e.g. it - * is `nullopt` or outdated), the local cache must have been incorrect so an - * `UnknownDocument` is returned. + * Applies this mutation to the given Document for the purposes of computing + * the committed state of the document after the server has acknowledged that + * this mutation has been successfully committed. + * + * If the input document doesn't match the expected state (e.g. it is invalid + * or outdated), the document state may transition to unknown. * - * @param maybe_doc The document to mutate. The input document can be - * `nullopt` if the client has no knowledge of the pre-mutation state of - * the document. + * @param document The document to mutate. * @param mutation_result The backend's response of successfully applying the * mutation. - * @return The mutated document. The returned document is not optional - * because the server successfully committed this mutation. If the local - * cache might have caused a `nullopt` result, this method will return an - * `UnknownDocument` instead. */ - MaybeDocument ApplyToRemoteDocument( - const absl::optional& maybe_doc, - const MutationResult& mutation_result) const; + void ApplyToRemoteDocument(MutableDocument& document, + const MutationResult& mutation_result) const; /** * Estimates the latency compensated view of this mutation applied to the @@ -204,33 +211,12 @@ class Mutation { * been committed and so it's possible that the mutation is operating on a * locally non-existent document and may produce a non-existent document. * - * Note: `maybe_doc` and `base_doc` are similar but not the same: - * - * - `base_doc` is the pristine version of the document as it was _before_ - * applying any of the mutations in the batch. This means that for each - * mutation in the batch, `base_doc` stays unchanged; - * - `maybe_doc` is the state of the document _after_ applying all the - * preceding mutations from the batch. In other words, `maybe_doc` is - * passed on from one mutation in the batch to the next, accumulating - * changes. - * - * The only time `maybe_doc` and `base_doc` are guaranteed to be the same is - * for the very first mutation in the batch. The distinction between - * `maybe_doc` and `base_doc` helps ServerTimestampTransform determine the - * "previous" value in a way that makes sense to users. - * - * @param maybe_doc The document to mutate. The input document can be - * `nullopt` if the client has no knowledge of the pre-mutation state of - * the document. + * @param document The document to mutate. * @param local_write_time 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 nullopt, but - * only if maybe_doc was nullopt and the mutation would not create a new - * document. */ - absl::optional ApplyToLocalView( - const absl::optional& maybe_doc, - const Timestamp& local_write_time) const; + void ApplyToLocalView(MutableDocument& document, + const Timestamp& local_write_time) const; /** * If this mutation is not idempotent, returns the base value to persist with @@ -249,8 +235,8 @@ class Mutation { * idempotent mutations. */ absl::optional ExtractTransformBaseValue( - const absl::optional& maybe_doc) const { - return rep_->ExtractTransformBaseValue(maybe_doc); + const Document& document) const { + return rep_->ExtractTransformBaseValue(document); } friend bool operator==(const Mutation& lhs, const Mutation& rhs); @@ -290,50 +276,42 @@ class Mutation { return field_transforms_; } - virtual MaybeDocument ApplyToRemoteDocument( - const absl::optional& maybe_doc, + virtual void ApplyToRemoteDocument( + MutableDocument& document, const MutationResult& mutation_result) const = 0; - virtual absl::optional ApplyToLocalView( - const absl::optional& maybe_doc, - const Timestamp& local_write_time) const = 0; + virtual void ApplyToLocalView(MutableDocument& document, + const Timestamp& local_write_time) const = 0; virtual absl::optional ExtractTransformBaseValue( - const absl::optional&) const; + const Document& document) const; /** - * Creates an array of "transform results" (a transform result is a field + * Creates a map of "transform results" (a transform result is a field * value representing the result of applying a transform) for use after a * mutation containing transforms has been acknowledged by the server. * - * @param maybe_doc The current state of the document after applying all - * previous mutations. + * @param previous_data The state of the data before applying this mutation. * @param server_transform_results The transform results received by the - * server. - * @return The transform results array. + * server. + * @return A map of fields to transform results. */ - virtual std::vector ServerTransformResults( - const absl::optional& maybe_doc, - const std::vector& server_transform_results) const; + TransformMap ServerTransformResults( + const ObjectValue& existing_data, + const google_firestore_v1_ArrayValue& server_transform_results) const; /** - * Creates an array of "transform results" (a transform result is a field + * Creates a map of "transform results" (a transform result is a field * value representing the result of applying a transform) for use when * applying a transform locally. * - * @param maybe_doc The current state of the document after applying all - * previous mutations. - * @param local_write_time The local time of the transform (used to - * generate ServerTimestampValues). - * @return The transform results array. + * @param previous_data The state of the data before applying this mutation. + * @param local_write_time The local time of the mutation (used to generate + * ServerTimestampValues). + * @return A map of fields to transform results. */ - virtual std::vector LocalTransformResults( - const absl::optional& maybe_doc, - const Timestamp& local_write_time) const; - - virtual ObjectValue TransformObject( - ObjectValue object_value, - const std::vector& transform_results) const; + TransformMap LocalTransformResults(const ObjectValue& previous_data, + const Timestamp& local_write_time) const; virtual bool Equals(const Rep& other) const; @@ -342,10 +320,10 @@ class Mutation { virtual std::string ToString() const = 0; protected: - void VerifyKeyMatches(const absl::optional& maybe_doc) const; + void VerifyKeyMatches(const MutableDocument& document) const; static SnapshotVersion GetPostMutationVersion( - const absl::optional& maybe_doc); + const MutableDocument& document); private: DocumentKey key_; diff --git a/Firestore/core/src/model/mutation_batch.cc b/Firestore/core/src/model/mutation_batch.cc index 406af46fe96..d9644a28b35 100644 --- a/Firestore/core/src/model/mutation_batch.cc +++ b/Firestore/core/src/model/mutation_batch.cc @@ -19,8 +19,9 @@ #include #include +#include "Firestore/core/src/model/document.h" #include "Firestore/core/src/model/document_key_set.h" -#include "Firestore/core/src/model/maybe_document.h" +#include "Firestore/core/src/model/mutable_document.h" #include "Firestore/core/src/model/mutation_batch_result.h" #include "Firestore/core/src/util/hard_assert.h" #include "Firestore/core/src/util/to_string.h" @@ -40,14 +41,9 @@ MutationBatch::MutationBatch(int batch_id, HARD_ASSERT(!mutations_.empty(), "Cannot create an empty mutation batch"); } -absl::optional MutationBatch::ApplyToRemoteDocument( - absl::optional maybe_doc, - const DocumentKey& document_key, +void MutationBatch::ApplyToRemoteDocument( + MutableDocument& document, const MutationBatchResult& mutation_batch_result) const { - HARD_ASSERT(!maybe_doc || maybe_doc->key() == document_key, - "ApplyTo: key %s doesn't match maybe_doc key %s", - document_key.ToString(), maybe_doc->key().ToString()); - const auto& mutation_results = mutation_batch_result.mutation_results(); HARD_ASSERT(mutation_results.size() == mutations_.size(), "Mismatch between mutations length (%s) and results length (%s)", @@ -55,59 +51,49 @@ absl::optional MutationBatch::ApplyToRemoteDocument( for (size_t i = 0; i < mutations_.size(); i++) { const Mutation& mutation = mutations_[i]; - const MutationResult& mutation_result = mutation_results[i]; - if (mutation.key() == document_key) { - maybe_doc = mutation.ApplyToRemoteDocument(maybe_doc, mutation_result); + if (mutation.key() == document.key()) { + const MutationResult& mutation_result = mutation_results[i]; + mutation.ApplyToRemoteDocument(document, mutation_result); } } - return maybe_doc; } -absl::optional MutationBatch::ApplyToLocalDocument( - absl::optional maybe_doc, - const DocumentKey& document_key) const { - HARD_ASSERT(!maybe_doc || maybe_doc->key() == document_key, - "key %s doesn't match maybe_doc key %s", document_key.ToString(), - maybe_doc->key().ToString()); - +void MutationBatch::ApplyToLocalDocument(MutableDocument& document) const { // First, apply the base state. This allows us to apply non-idempotent // transform against a consistent set of values. for (const Mutation& mutation : base_mutations_) { - if (mutation.key() == document_key) { - maybe_doc = mutation.ApplyToLocalView(maybe_doc, local_write_time_); + if (mutation.key() == document.key()) { + mutation.ApplyToLocalView(document, local_write_time_); } } - absl::optional base_doc = maybe_doc; - // Second, apply all user-provided mutations. for (const Mutation& mutation : mutations_) { - if (mutation.key() == document_key) { - maybe_doc = mutation.ApplyToLocalView(maybe_doc, local_write_time_); + if (mutation.key() == document.key()) { + mutation.ApplyToLocalView(document, local_write_time_); } } - return maybe_doc; } -MaybeDocumentMap MutationBatch::ApplyToLocalDocumentSet( - const MaybeDocumentMap& document_set) const { +void MutationBatch::ApplyToLocalDocumentSet(DocumentMap& document_map) const { // TODO(mrschmidt): This implementation is O(n^2). If we iterate through the // mutations first (as done in `applyToLocalDocument:documentKey:`), we can // reduce the complexity to O(n). - MaybeDocumentMap mutated_documents = document_set; for (const Mutation& mutation : mutations_) { const DocumentKey& key = mutation.key(); - absl::optional previous_document = - mutated_documents.get(key); - absl::optional mutated_document = - ApplyToLocalDocument(std::move(previous_document), key); - if (mutated_document) { - mutated_documents = mutated_documents.insert(key, *mutated_document); + auto it = document_map.find(key); + HARD_ASSERT(it != document_map.end(), "document for key %s not found", + key.ToString()); + // TODO(mutabledocuments): This method should take a map of MutableDocuments + // and we should remove this cast. + auto& document = const_cast(it->second.get()); + ApplyToLocalDocument(document); + if (!document.is_valid_document()) { + document.ConvertToNoDocument(SnapshotVersion::None()); } } - return mutated_documents; } DocumentKeySet MutationBatch::keys() const { diff --git a/Firestore/core/src/model/mutation_batch.h b/Firestore/core/src/model/mutation_batch.h index e6ff40eecd0..f71879ee241 100644 --- a/Firestore/core/src/model/mutation_batch.h +++ b/Firestore/core/src/model/mutation_batch.h @@ -31,6 +31,8 @@ namespace firebase { namespace firestore { namespace model { +class MutableDocument; + /** * A BatchID that was searched for and not found or a batch ID value known to * be before all known batches. @@ -87,15 +89,12 @@ class MutationBatch { * Applies all the mutations in this MutationBatch to the specified document * to create a new remote document. * - * @param maybe_doc The document to which to apply mutations or nullopt if - * there's no existing document. - * @param document_key The key of the document to apply mutations to. + * @param document The document to which to apply mutations. * @param mutation_batch_result The result of applying the MutationBatch to * the backend. */ - absl::optional ApplyToRemoteDocument( - absl::optional maybe_doc, - const DocumentKey& document_key, + void ApplyToRemoteDocument( + MutableDocument& document, const MutationBatchResult& mutation_batch_result) const; /** @@ -106,20 +105,15 @@ class MutationBatch { * been committed and so it's possible that the mutation is operating on a * locally non-existent document and may produce a non-existent document. * - * @param maybe_doc The document to which to apply mutations or nullopt if - * there's no existing document. - * @param document_key The key of the document to apply mutations to. + * @param document The document to which to apply mutations. */ - absl::optional ApplyToLocalDocument( - absl::optional maybe_doc, - const DocumentKey& document_key) const; + void ApplyToLocalDocument(MutableDocument& document) const; /** * Computes the local view for all provided documents given the mutations in * this batch. */ - MaybeDocumentMap ApplyToLocalDocumentSet( - const MaybeDocumentMap& document_set) const; + void ApplyToLocalDocumentSet(DocumentMap& document_map) const; /** * Returns the set of unique keys referenced by all mutations in the batch. diff --git a/Firestore/core/src/model/object_value.cc b/Firestore/core/src/model/object_value.cc index 4cb370a532a..a4b7a968b6d 100644 --- a/Firestore/core/src/model/object_value.cc +++ b/Firestore/core/src/model/object_value.cc @@ -302,13 +302,18 @@ void ObjectValue::Set(const FieldPath& path, ApplyChanges(parent_map, upserts, /*deletes=*/{}); } -void ObjectValue::SetAll(const FieldMask& field_mask, const ObjectValue& data) { +void ObjectValue::SetAll( + const std::map>& + data) { FieldPath parent; std::map upserts; std::set deletes; - for (const FieldPath& path : field_mask) { + for (const auto& it : data) { + const FieldPath& path = it.first; + const google_firestore_v1_Value& value = it.second; + if (!parent.IsImmediateParentOf(path)) { // Insert the accumulated changes at this parent location google_firestore_v1_MapValue* parent_map = ParentMap(parent); @@ -318,7 +323,6 @@ void ObjectValue::SetAll(const FieldMask& field_mask, const ObjectValue& data) { parent = path.PopLast(); } - absl::optional value = data.Get(path); if (value) { upserts.emplace(path.last_segment(), *value); } else { diff --git a/Firestore/core/src/model/object_value.h b/Firestore/core/src/model/object_value.h index 2e3ea991ef2..6fa638e2cf7 100644 --- a/Firestore/core/src/model/object_value.h +++ b/Firestore/core/src/model/object_value.h @@ -90,14 +90,13 @@ class ObjectValue { void Set(const FieldPath& path, const google_firestore_v1_Value& value); /** - * Sets the provided fields to the provided values. Only fields included in - * `field_mask` are modified. If a field is included in field_mask, but - * missing in `data`, it is deleted. + * Sets the provided fields to the provided values. Fields set to `nullopt` + * are deleted. * - * @param field_mask The field mask that controls which fields to modify. - * @param data A MutableObjectValue that contains the field values. + * @param data A map of fields to values (or nullopt for deletes) */ - void SetAll(const FieldMask& field_mask, const ObjectValue& data); + void SetAll(const std::map>& data); /** * Removes the field at the specified path. If there is no field at the diff --git a/Firestore/core/src/model/patch_mutation.cc b/Firestore/core/src/model/patch_mutation.cc index 209551a288f..1357a7cb4e3 100644 --- a/Firestore/core/src/model/patch_mutation.cc +++ b/Firestore/core/src/model/patch_mutation.cc @@ -19,11 +19,8 @@ #include #include -#include "Firestore/core/src/model/document.h" #include "Firestore/core/src/model/field_path.h" -#include "Firestore/core/src/model/field_value.h" -#include "Firestore/core/src/model/no_document.h" -#include "Firestore/core/src/model/unknown_document.h" +#include "Firestore/core/src/model/mutable_document.h" #include "Firestore/core/src/util/hard_assert.h" #include "Firestore/core/src/util/to_string.h" @@ -73,75 +70,53 @@ PatchMutation::Rep::Rep(DocumentKey&& key, mask_(std::move(mask)) { } -MaybeDocument PatchMutation::Rep::ApplyToRemoteDocument( - const absl::optional& maybe_doc, - const MutationResult& mutation_result) const { - VerifyKeyMatches(maybe_doc); +void PatchMutation::Rep::ApplyToRemoteDocument( + MutableDocument& document, const MutationResult& mutation_result) const { + VerifyKeyMatches(document); - if (!precondition().IsValidFor(maybe_doc)) { + if (!precondition().IsValidFor(document)) { // Since the mutation was not rejected, we know that the precondition // matched on the backend. We therefore must not have the expected version // of the document in our cache and return an UnknownDocument with the known // update_time. - return UnknownDocument(key(), mutation_result.version()); + document.ConvertToUnknownDocument(mutation_result.version()); + return; } - std::vector transform_results = - mutation_result.transform_results() != absl::nullopt - ? ServerTransformResults(maybe_doc, - *mutation_result.transform_results()) - : std::vector(); - - ObjectValue new_data = PatchDocument(maybe_doc, transform_results); - const SnapshotVersion& version = mutation_result.version(); - return Document(std::move(new_data), key(), version, - DocumentState::kCommittedMutations); + ObjectValue& data = document.data(); + auto transform_results = + ServerTransformResults(data, mutation_result.transform_results()); + data.SetAll(GetPatch()); + data.SetAll(transform_results); + document.ConvertToFoundDocument(mutation_result.version()) + .SetHasCommittedMutations(); } -absl::optional PatchMutation::Rep::ApplyToLocalView( - const absl::optional& maybe_doc, - const Timestamp& local_write_time) const { - VerifyKeyMatches(maybe_doc); +void PatchMutation::Rep::ApplyToLocalView( + MutableDocument& document, const Timestamp& local_write_time) const { + VerifyKeyMatches(document); - if (!precondition().IsValidFor(maybe_doc)) { - return maybe_doc; + if (!precondition().IsValidFor(document)) { + return; } - std::vector transform_results = - LocalTransformResults(maybe_doc, local_write_time); - - ObjectValue new_data = PatchDocument(maybe_doc, transform_results); - SnapshotVersion version = GetPostMutationVersion(maybe_doc); - return Document(std::move(new_data), key(), version, - DocumentState::kLocalMutations); -} - -ObjectValue PatchMutation::Rep::PatchDocument( - const absl::optional& maybe_doc, - const std::vector& transform_results) const { - ObjectValue data; - if (maybe_doc && maybe_doc->type() == MaybeDocument::Type::Document) { - data = Document(*maybe_doc).data(); - } else { - data = ObjectValue::Empty(); - } - data = PatchObject(data); - data = TransformObject(data, transform_results); - return data; + ObjectValue& data = document.data(); + auto transform_results = LocalTransformResults(data, local_write_time); + data.SetAll(GetPatch()); + data.SetAll(transform_results); + document.ConvertToFoundDocument(GetPostMutationVersion(document)) + .SetHasLocalMutations(); } -ObjectValue PatchMutation::Rep::PatchObject(ObjectValue obj) const { +std::map> +PatchMutation::Rep::GetPatch() const { + std::map> result; for (const FieldPath& path : mask_) { if (!path.empty()) { - absl::optional new_value = value_.Get(path); - if (!new_value) { - obj = obj.Delete(path); - } else { - obj = obj.Set(path, *new_value); - } + result[path] = value_.Get(path); } } - return obj; + return result; } bool PatchMutation::Rep::Equals(const Mutation::Rep& other) const { diff --git a/Firestore/core/src/model/patch_mutation.h b/Firestore/core/src/model/patch_mutation.h index 250aa3a80c6..587b44ecb8b 100644 --- a/Firestore/core/src/model/patch_mutation.h +++ b/Firestore/core/src/model/patch_mutation.h @@ -23,7 +23,6 @@ #include #include "Firestore/core/src/model/field_mask.h" -#include "Firestore/core/src/model/field_value.h" #include "Firestore/core/src/model/model_fwd.h" #include "Firestore/core/src/model/mutation.h" @@ -102,13 +101,19 @@ class PatchMutation : public Mutation { return mask_; } - MaybeDocument ApplyToRemoteDocument( - const absl::optional& maybe_doc, + /** + * Returns this patch mutation as a list of field paths to values (or + * nullopt for deletes). + */ + std::map> GetPatch() + const; + + void ApplyToRemoteDocument( + MutableDocument& document, const MutationResult& mutation_result) const override; - absl::optional ApplyToLocalView( - const absl::optional& maybe_doc, - const Timestamp& local_write_time) const override; + void ApplyToLocalView(MutableDocument& document, + const Timestamp& local_write_time) const override; bool Equals(const Mutation::Rep& other) const override; @@ -117,12 +122,6 @@ class PatchMutation : public Mutation { std::string ToString() const override; private: - ObjectValue PatchDocument( - const absl::optional& maybe_doc, - const std::vector& transform_results) const; - - ObjectValue PatchObject(ObjectValue obj) const; - ObjectValue value_; FieldMask mask_; }; diff --git a/Firestore/core/src/model/precondition.cc b/Firestore/core/src/model/precondition.cc index 4822cb0cb55..cacf7889c39 100644 --- a/Firestore/core/src/model/precondition.cc +++ b/Firestore/core/src/model/precondition.cc @@ -16,7 +16,7 @@ #include "Firestore/core/src/model/precondition.h" -#include "Firestore/core/src/model/maybe_document.h" +#include "Firestore/core/src/model/mutable_document.h" #include "Firestore/core/src/model/snapshot_version.h" #include "Firestore/core/src/util/hard_assert.h" #include "Firestore/core/src/util/hashing.h" @@ -47,15 +47,12 @@ Precondition Precondition::None() { return Precondition{Type::None, SnapshotVersion::None(), false}; } -bool Precondition::IsValidFor( - const absl::optional& maybe_doc) const { +bool Precondition::IsValidFor(const MutableDocument& document) const { switch (type_) { case Type::UpdateTime: - return maybe_doc && maybe_doc->type() == MaybeDocument::Type::Document && - maybe_doc->version() == update_time_; + return document.is_found_document() && document.version() == update_time_; case Type::Exists: - return (exists_ == (maybe_doc && - maybe_doc->type() == MaybeDocument::Type::Document)); + return exists_ == document.is_found_document(); case Type::None: return true; } diff --git a/Firestore/core/src/model/precondition.h b/Firestore/core/src/model/precondition.h index 5f42ca67fe6..0f58a89d091 100644 --- a/Firestore/core/src/model/precondition.h +++ b/Firestore/core/src/model/precondition.h @@ -27,7 +27,7 @@ namespace firebase { namespace firestore { namespace model { -class MaybeDocument; +class MutableDocument; /** * Encodes a precondition for a mutation. This follows the model that the @@ -61,7 +61,7 @@ class Precondition { * Returns true if the precondition is valid for the given document (and the * document is available). */ - bool IsValidFor(const absl::optional& maybe_doc) const; + bool IsValidFor(const MutableDocument& document) const; Type type() const { return type_; diff --git a/Firestore/core/src/model/set_mutation.cc b/Firestore/core/src/model/set_mutation.cc index 855c92e8a99..c1a707f51ff 100644 --- a/Firestore/core/src/model/set_mutation.cc +++ b/Firestore/core/src/model/set_mutation.cc @@ -19,9 +19,8 @@ #include #include -#include "Firestore/core/src/model/document.h" -#include "Firestore/core/src/model/field_value.h" -#include "Firestore/core/src/model/no_document.h" +#include "Firestore/core/src/model/mutable_document.h" +#include "Firestore/core/src/model/value_util.h" #include "Firestore/core/src/util/hard_assert.h" #include "Firestore/core/src/util/hashing.h" #include "Firestore/core/src/util/to_string.h" @@ -67,40 +66,37 @@ SetMutation::Rep::Rep(DocumentKey&& key, value_(std::move(value)) { } -MaybeDocument SetMutation::Rep::ApplyToRemoteDocument( - const absl::optional& maybe_doc, - const MutationResult& mutation_result) const { - VerifyKeyMatches(maybe_doc); +void SetMutation::Rep::ApplyToRemoteDocument( + MutableDocument& document, const MutationResult& mutation_result) const { + VerifyKeyMatches(document); // Unlike ApplyToLocalView, if we're applying a mutation to a remote document // the server has accepted the mutation so the precondition must have held. - - ObjectValue new_data = value_; - if (mutation_result.transform_results() != absl::nullopt) { - std::vector transform_results = - ServerTransformResults(maybe_doc, *mutation_result.transform_results()); - new_data = TransformObject(new_data, transform_results); - } - - const SnapshotVersion& version = mutation_result.version(); - return Document(new_data, key(), version, DocumentState::kCommittedMutations); + auto transform_results = ServerTransformResults( + document.data(), mutation_result.transform_results()); + ObjectValue new_data{DeepClone(value_.Get())}; + new_data.SetAll(transform_results); + document + .ConvertToFoundDocument(mutation_result.version(), std::move(new_data)) + .SetHasCommittedMutations(); } -absl::optional SetMutation::Rep::ApplyToLocalView( - const absl::optional& maybe_doc, - const Timestamp& local_write_time) const { - VerifyKeyMatches(maybe_doc); +void SetMutation::Rep::ApplyToLocalView( + MutableDocument& document, const Timestamp& local_write_time) const { + VerifyKeyMatches(document); - if (!precondition().IsValidFor(maybe_doc)) { - return maybe_doc; + if (!precondition().IsValidFor(document)) { + return; } - std::vector transforms_results = - LocalTransformResults(maybe_doc, local_write_time); - ObjectValue new_data = TransformObject(value_, transforms_results); - - SnapshotVersion version = GetPostMutationVersion(maybe_doc); - return Document(new_data, key(), version, DocumentState::kLocalMutations); + auto transform_results = + LocalTransformResults(document.data(), local_write_time); + ObjectValue new_data{DeepClone(value_.Get())}; + new_data.SetAll(transform_results); + document + .ConvertToFoundDocument(GetPostMutationVersion(document), + std::move(new_data)) + .SetHasLocalMutations(); } bool SetMutation::Rep::Equals(const Mutation::Rep& other) const { diff --git a/Firestore/core/src/model/set_mutation.h b/Firestore/core/src/model/set_mutation.h index b2fa584a5ed..b876f7b981e 100644 --- a/Firestore/core/src/model/set_mutation.h +++ b/Firestore/core/src/model/set_mutation.h @@ -22,8 +22,8 @@ #include #include -#include "Firestore/core/src/model/field_value.h" #include "Firestore/core/src/model/mutation.h" +#include "Firestore/core/src/model/object_value.h" #include "absl/types/optional.h" namespace firebase { @@ -73,13 +73,12 @@ class SetMutation : public Mutation { return value_; } - MaybeDocument ApplyToRemoteDocument( - const absl::optional& maybe_doc, + void ApplyToRemoteDocument( + MutableDocument& document, const MutationResult& mutation_result) const override; - absl::optional ApplyToLocalView( - const absl::optional& maybe_doc, - const Timestamp& local_write_time) const override; + void ApplyToLocalView(MutableDocument& document, + const Timestamp& local_write_time) const override; bool Equals(const Mutation::Rep& other) const override; diff --git a/Firestore/core/src/model/verify_mutation.cc b/Firestore/core/src/model/verify_mutation.cc index d18392a9441..456e606c4c0 100644 --- a/Firestore/core/src/model/verify_mutation.cc +++ b/Firestore/core/src/model/verify_mutation.cc @@ -19,7 +19,7 @@ #include #include "Firestore/core/src/model/field_path.h" -#include "Firestore/core/src/model/no_document.h" +#include "Firestore/core/src/model/mutable_document.h" #include "Firestore/core/src/util/hard_assert.h" namespace firebase { @@ -38,13 +38,13 @@ VerifyMutation::VerifyMutation(const Mutation& mutation) : Mutation(mutation) { HARD_ASSERT(type() == Type::Verify); } -MaybeDocument VerifyMutation::Rep::ApplyToRemoteDocument( - const absl::optional&, const MutationResult&) const { +void VerifyMutation::Rep::ApplyToRemoteDocument(MutableDocument&, + const MutationResult&) const { HARD_FAIL("VerifyMutation should only be used in Transactions."); } -absl::optional VerifyMutation::Rep::ApplyToLocalView( - const absl::optional&, const Timestamp&) const { +void VerifyMutation::Rep::ApplyToLocalView(MutableDocument&, + const Timestamp&) const { HARD_FAIL("VerifyMutation should only be used in Transactions."); } diff --git a/Firestore/core/test/unit/model/object_value_test.cc b/Firestore/core/test/unit/model/object_value_test.cc index 1c705b81123..247a44328cb 100644 --- a/Firestore/core/test/unit/model/object_value_test.cc +++ b/Firestore/core/test/unit/model/object_value_test.cc @@ -127,11 +127,11 @@ TEST_F(ObjectValueTest, AddsMultipleFields) { ObjectValue object_value{}; EXPECT_EQ(ObjectValue{}, object_value); - object_value.SetAll( - FieldMask( - {Field("a"), Field("b"), Field("c.d"), Field("c.e"), Field("c.f")}), - WrapObject("a", 1, "b", 2, "c", - Map("d", 3, "e", 4, "f", Map("g", 5), "ignored", 6))); + object_value.SetAll({{Field("a"), Value(1)}, + {Field("b"), Value(2)}, + {Field("c.d"), Value(3)}, + {Field("c.e"), Value(4)}, + {Field("c.f.g"), Value(5)}}); EXPECT_EQ( WrapObject("a", 1, "b", 2, "c", Map("d", 3, "e", 4, "f", Map("g", 5))), object_value); @@ -241,8 +241,9 @@ TEST_F(ObjectValueTest, DeletesMultipleKeys) { ObjectValue object_value = WrapObject("a", 1, "b", 2, "c", Map("d", 3, "e", 4)); - object_value.SetAll(FieldMask({Field("a"), Field("b"), Field("c.d")}), - WrapObject()); + object_value.SetAll({{Field("a"), absl::nullopt}, + {Field("b"), absl::nullopt}, + {Field("c.d"), absl::nullopt}}); EXPECT_EQ(WrapObject("c", Map("e", 4)), object_value); } @@ -295,7 +296,7 @@ TEST_F(ObjectValueTest, AddsAndDeletesField) { TEST_F(ObjectValueTest, AddsAndDeletesMultipleFields) { ObjectValue object_value = WrapObject("b", 2, "c", 3); - object_value.SetAll(FieldMask({Field("a"), Field("b")}), WrapObject("a", 1)); + object_value.SetAll({{Field("a"), Value(1)}, {Field("b"), absl::nullopt}}); EXPECT_EQ(WrapObject("a", 1, "c", 3), object_value); }