diff --git a/Firestore/core/src/firebase/firestore/local/local_serializer.cc b/Firestore/core/src/firebase/firestore/local/local_serializer.cc index 49d25968f4f..7f252969186 100644 --- a/Firestore/core/src/firebase/firestore/local/local_serializer.cc +++ b/Firestore/core/src/firebase/firestore/local/local_serializer.cc @@ -42,7 +42,6 @@ using model::MaybeDocument; using model::Mutation; using model::MutationBatch; using model::NoDocument; -using model::ObjectValue; using model::SnapshotVersion; using model::UnknownDocument; using nanopb::Reader; @@ -124,13 +123,13 @@ google_firestore_v1_Document LocalSerializer::EncodeDocument( rpc_serializer_.EncodeString(rpc_serializer_.EncodeKey(doc.key())); // Encode Document.fields (unless it's empty) - size_t count = doc.data().object_value().internal_value.size(); + size_t count = doc.data().GetInternalValue().size(); HARD_ASSERT(count <= std::numeric_limits::max(), "Unable to encode specified document. Too many fields."); result.fields_count = static_cast(count); result.fields = MakeArray(count); int i = 0; - for (const auto& kv : doc.data().object_value().internal_value) { + for (const auto& kv : doc.data().GetInternalValue()) { result.fields[i].key = rpc_serializer_.EncodeString(kv.first); result.fields[i].value = rpc_serializer_.EncodeFieldValue(kv.second); i++; diff --git a/Firestore/core/src/firebase/firestore/model/document.cc b/Firestore/core/src/firebase/firestore/model/document.cc index 06deb50576b..da540c88324 100644 --- a/Firestore/core/src/firebase/firestore/model/document.cc +++ b/Firestore/core/src/firebase/firestore/model/document.cc @@ -24,7 +24,7 @@ namespace firebase { namespace firestore { namespace model { -Document::Document(FieldValue&& data, +Document::Document(ObjectValue&& data, DocumentKey key, SnapshotVersion version, DocumentState document_state) @@ -32,7 +32,6 @@ Document::Document(FieldValue&& data, data_(std::move(data)), document_state_(document_state) { set_type(Type::Document); - HARD_ASSERT(FieldValue::Type::Object == data.type()); } bool Document::Equals(const MaybeDocument& other) const { diff --git a/Firestore/core/src/firebase/firestore/model/document.h b/Firestore/core/src/firebase/firestore/model/document.h index 9afa327f64b..acd04212572 100644 --- a/Firestore/core/src/firebase/firestore/model/document.h +++ b/Firestore/core/src/firebase/firestore/model/document.h @@ -50,14 +50,14 @@ enum class DocumentState { class Document : public MaybeDocument { public: /** - * Construct a document. FieldValue must be passed by rvalue. + * Construct a document. ObjectValue must be passed by rvalue. */ - Document(FieldValue&& data, + Document(ObjectValue&& data, DocumentKey key, SnapshotVersion version, DocumentState document_state); - const FieldValue& data() const { + const ObjectValue& data() const { return data_; } @@ -81,7 +81,7 @@ class Document : public MaybeDocument { bool Equals(const MaybeDocument& other) const override; private: - FieldValue data_; // This is of type Object. + ObjectValue data_; DocumentState document_state_; }; diff --git a/Firestore/core/src/firebase/firestore/model/field_value.cc b/Firestore/core/src/firebase/firestore/model/field_value.cc index 53e197a9113..fee10451e9a 100644 --- a/Firestore/core/src/firebase/firestore/model/field_value.cc +++ b/Firestore/core/src/firebase/firestore/model/field_value.cc @@ -23,6 +23,7 @@ #include #include +#include "Firestore/core/src/firebase/firestore/immutable/sorted_map.h" #include "Firestore/core/src/firebase/firestore/util/comparison.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" #include "absl/memory/memory.h" @@ -91,8 +92,8 @@ FieldValue& FieldValue::operator=(const FieldValue& value) { } case Type::Object: { // copy-and-swap - ObjectValue::Map tmp = value.object_value_->internal_value; - std::swap(object_value_->internal_value, tmp); + Map tmp = *value.object_value_; + std::swap(*object_value_, tmp); break; } default: @@ -143,45 +144,41 @@ bool FieldValue::Comparable(Type lhs, Type rhs) { } } -FieldValue FieldValue::Set(const FieldPath& field_path, - const FieldValue& value) const { - HARD_ASSERT(type() == Type::Object, - "Cannot set field for non-object FieldValue"); +// TODO(rsgowman): Reorder this file to match its header. +ObjectValue ObjectValue::Set(const FieldPath& field_path, + const FieldValue& value) const { HARD_ASSERT(!field_path.empty(), "Cannot set field for empty path on FieldValue"); // Set the value by recursively calling on child object. const std::string& child_name = field_path.first_segment(); - const ObjectValue::Map& object_map = object_value_->internal_value; if (field_path.size() == 1) { return SetChild(child_name, value); } else { - FieldValue child; - const auto iter = object_map.find(child_name); - if (iter != object_map.end() && iter->second.type() == Type::Object) { - child = iter->second; - } else { - child = EmptyObject(); + ObjectValue child = ObjectValue::Empty(); + const auto iter = fv_.object_value_->find(child_name); + if (iter != fv_.object_value_->end() && + iter->second.type() == Type::Object) { + child = ObjectValue(iter->second); } - FieldValue new_child = child.Set(field_path.PopFirst(), value); - return SetChild(child_name, new_child); + ObjectValue new_child = child.Set(field_path.PopFirst(), value); + return SetChild(child_name, new_child.fv_); } } -FieldValue FieldValue::Delete(const FieldPath& field_path) const { - HARD_ASSERT(type() == Type::Object, - "Cannot delete field for non-object FieldValue"); +ObjectValue ObjectValue::Delete(const FieldPath& field_path) const { HARD_ASSERT(!field_path.empty(), "Cannot delete field for empty path on FieldValue"); // Delete the value by recursively calling on child object. const std::string& child_name = field_path.first_segment(); - const ObjectValue::Map& object_map = object_value_->internal_value; if (field_path.size() == 1) { - return FieldValue::FromMap(object_map.erase(child_name)); + return ObjectValue::FromMap(fv_.object_value_->erase(child_name)); } else { - const auto iter = object_map.find(child_name); - if (iter != object_map.end() && iter->second.type() == Type::Object) { - FieldValue new_child = iter->second.Delete(field_path.PopFirst()); - return SetChild(child_name, new_child); + const auto iter = fv_.object_value_->find(child_name); + if (iter != fv_.object_value_->end() && + iter->second.type() == Type::Object) { + ObjectValue new_child = + ObjectValue(iter->second).Delete(field_path.PopFirst()); + return SetChild(child_name, new_child.fv_); } else { // If the found value isn't an object, it cannot contain the remaining // segments of the path. We don't actually change a primitive value to @@ -191,17 +188,14 @@ FieldValue FieldValue::Delete(const FieldPath& field_path) const { } } -absl::optional FieldValue::Get(const FieldPath& field_path) const { - HARD_ASSERT(type() == Type::Object, - "Cannot get field for non-object FieldValue"); - const FieldValue* current = this; +absl::optional ObjectValue::Get(const FieldPath& field_path) const { + const FieldValue* current = &this->fv_; for (const auto& path : field_path) { if (current->type() != Type::Object) { return absl::nullopt; } - const ObjectValue::Map& object_map = current->object_value_->internal_value; - const auto iter = object_map.find(path); - if (iter == object_map.end()) { + const auto iter = current->object_value_->find(path); + if (iter == current->object_value_->end()) { return absl::nullopt; } else { current = &iter->second; @@ -210,12 +204,9 @@ absl::optional FieldValue::Get(const FieldPath& field_path) const { return *current; } -FieldValue FieldValue::SetChild(const std::string& child_name, - const FieldValue& value) const { - HARD_ASSERT(type() == Type::Object, - "Cannot set child for non-object FieldValue"); - return FieldValue::FromMap( - object_value_->internal_value.insert(child_name, value)); +ObjectValue ObjectValue::SetChild(const std::string& child_name, + const FieldValue& value) const { + return ObjectValue::FromMap(fv_.object_value_->insert(child_name, value)); } FieldValue FieldValue::Null() { @@ -239,7 +230,7 @@ FieldValue FieldValue::Nan() { } FieldValue FieldValue::EmptyObject() { - return FieldValue::FromMap(ObjectValue::Empty()); + return FieldValue::FromMap(FieldValue::Map()); } FieldValue FieldValue::FromInteger(int64_t value) { @@ -344,19 +335,19 @@ FieldValue FieldValue::FromArray(std::vector&& value) { return result; } -FieldValue FieldValue::FromMap(const ObjectValue::Map& value) { - ObjectValue::Map copy(value); +FieldValue FieldValue::FromMap(const FieldValue::Map& value) { + FieldValue::Map copy(value); return FromMap(std::move(copy)); } -FieldValue FieldValue::FromMap(ObjectValue::Map&& value) { +FieldValue FieldValue::FromMap(FieldValue::Map&& value) { FieldValue result; result.SwitchTo(Type::Object); - std::swap(result.object_value_->internal_value, value); + std::swap(*result.object_value_, value); return result; } -bool operator<(const ObjectValue::Map& lhs, const ObjectValue::Map& rhs) { +bool operator<(const FieldValue::Map& lhs, const FieldValue::Map& rhs) { return std::lexicographical_compare(lhs.begin(), lhs.end(), rhs.begin(), rhs.end()); } @@ -454,7 +445,7 @@ void FieldValue::SwitchTo(const Type type) { array_value_.~unique_ptr>(); break; case Type::Object: - object_value_.~unique_ptr(); + object_value_.~unique_ptr(); break; default: {} // The other types where there is nothing to worry about. } @@ -491,13 +482,20 @@ void FieldValue::SwitchTo(const Type type) { absl::make_unique>()); break; case Type::Object: - new (&object_value_) - std::unique_ptr(absl::make_unique()); + new (&object_value_) std::unique_ptr(absl::make_unique()); break; default: {} // The other types where there is nothing to worry about. } } +ObjectValue ObjectValue::FromMap(const FieldValue::Map& value) { + return ObjectValue(FieldValue::FromMap(value)); +} + +ObjectValue ObjectValue::FromMap(FieldValue::Map&& value) { + return ObjectValue(FieldValue::FromMap(std::move(value))); +} + } // namespace model } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/model/field_value.h b/Firestore/core/src/firebase/firestore/model/field_value.h index c6902d3dfd7..0aeb2d316e2 100644 --- a/Firestore/core/src/firebase/firestore/model/field_value.h +++ b/Firestore/core/src/firebase/firestore/model/field_value.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include "Firestore/core/include/firebase/firestore/geo_point.h" @@ -46,8 +47,6 @@ struct ReferenceValue { const DatabaseId* database_id; }; -struct ObjectValue; - /** * tagged-union class representing an immutable data value as stored in * Firestore. FieldValue represents all the different kinds of values @@ -55,6 +54,8 @@ struct ObjectValue; */ class FieldValue { public: + using Map = immutable::SortedMap; + /** * All the different kinds of values that can be stored in fields in * a document. The types of the same comparison order should be defined @@ -126,39 +127,6 @@ class FieldValue { return *string_value_; } - const ObjectValue& object_value() const { - HARD_ASSERT(tag_ == Type::Object); - return *object_value_; - } - - /** - * Returns a FieldValue with the field at the named path set to value. - * Any absent parent of the field will also be created accordingly. - * - * @param field_path The field path to set. Cannot be empty. - * @param value The value to set. - * @return A new FieldValue with the field set. - */ - FieldValue Set(const FieldPath& field_path, const FieldValue& value) const; - - /** - * Returns a FieldValue with the field path deleted. If there is no field at - * the specified path, the returned value is an identical copy. - * - * @param field_path The field path to remove. Cannot be empty. - * @return A new FieldValue with the field path removed. - */ - FieldValue Delete(const FieldPath& field_path) const; - - /** - * Returns the value at the given path or absl::nullopt. If the path is empty, - * an identical copy of the FieldValue is returned. - * - * @param field_path the path to search. - * @return The value at the path or absl::nullopt if it doesn't exist. - */ - absl::optional Get(const FieldPath& field_path) const; - /** factory methods. */ static FieldValue Null(); static FieldValue True(); @@ -183,14 +151,14 @@ class FieldValue { static FieldValue FromGeoPoint(const GeoPoint& value); static FieldValue FromArray(const std::vector& value); static FieldValue FromArray(std::vector&& value); - static FieldValue FromMap( - const immutable::SortedMap& value); - static FieldValue FromMap( - immutable::SortedMap&& value); + static FieldValue FromMap(const Map& value); + static FieldValue FromMap(Map&& value); friend bool operator<(const FieldValue& lhs, const FieldValue& rhs); private: + friend class ObjectValue; + explicit FieldValue(bool value) : tag_(Type::Boolean), boolean_value_(value) { } @@ -199,9 +167,6 @@ class FieldValue { */ void SwitchTo(Type type); - FieldValue SetChild(const std::string& child_name, - const FieldValue& value) const; - Type tag_ = Type::Null; union { // There is no null type as tag_ alone is enough for Null FieldValue. @@ -216,27 +181,72 @@ class FieldValue { std::unique_ptr reference_value_; std::unique_ptr geo_point_value_; std::unique_ptr> array_value_; - std::unique_ptr object_value_; + std::unique_ptr object_value_; }; }; -// TODO(rsgowman): Expand this to roughly match the java class -// c.g.f.f.model.value.ObjectValue. Probably move it to a similar namespace as -// well. (FieldValue itself is also in the value package in java.) Also do the -// same with the other FooValue values that FieldValue can return. -struct ObjectValue { - // TODO(rsgowman): These will eventually be private. We do want the serializer - // to be able to directly access these (possibly implying 'friend' usage, or a - // getInternalValue() like java has.) - using Map = immutable::SortedMap; - Map internal_value; +/** A structured object value stored in Firestore. */ +class ObjectValue { + public: + explicit ObjectValue(FieldValue fv) : fv_(std::move(fv)) { + HARD_ASSERT(fv_.type() == FieldValue::Type::Object); + } - static ObjectValue::Map Empty() { - return Map(); + static ObjectValue Empty() { + return ObjectValue(FieldValue::EmptyObject()); } + + static ObjectValue FromMap(const FieldValue::Map& value); + static ObjectValue FromMap(FieldValue::Map&& value); + + /** + * Returns the value at the given path or absl::nullopt. If the path is empty, + * an identical copy of the FieldValue is returned. + * + * @param field_path the path to search. + * @return The value at the path or absl::nullopt if it doesn't exist. + */ + absl::optional Get(const FieldPath& field_path) const; + + /** + * Returns a FieldValue with the field at the named path set to value. + * Any absent parent of the field will also be created accordingly. + * + * @param field_path The field path to set. Cannot be empty. + * @param value The value to set. + * @return A new FieldValue with the field set. + */ + ObjectValue Set(const FieldPath& field_path, const FieldValue& value) const; + + /** + * Returns a FieldValue with the field path deleted. If there is no field at + * the specified path, the returned value is an identical copy. + * + * @param field_path The field path to remove. Cannot be empty. + * @return A new FieldValue with the field path removed. + */ + ObjectValue Delete(const FieldPath& field_path) const; + + // TODO(rsgowman): Add Value() method? + // + // Java has a value() method which returns a (non-immutable) java.util.Map, + // which is a copy of the immutable map, but with some fields (such as server + // timestamps) optionally resolved. Do we need the same here? + + const FieldValue::Map& GetInternalValue() const { + return *fv_.object_value_; + } + + private: + friend bool operator<(const ObjectValue& lhs, const ObjectValue& rhs); + + ObjectValue SetChild(const std::string& child_name, + const FieldValue& value) const; + + FieldValue fv_; }; -bool operator<(const ObjectValue::Map& lhs, const ObjectValue::Map& rhs); +bool operator<(const FieldValue::Map& lhs, const FieldValue::Map& rhs); /** Compares against another FieldValue. */ bool operator<(const FieldValue& lhs, const FieldValue& rhs); @@ -263,7 +273,7 @@ inline bool operator==(const FieldValue& lhs, const FieldValue& rhs) { /** Compares against another ObjectValue. */ inline bool operator<(const ObjectValue& lhs, const ObjectValue& rhs) { - return lhs.internal_value < rhs.internal_value; + return lhs.fv_ < rhs.fv_; } inline bool operator>(const ObjectValue& lhs, const ObjectValue& rhs) { diff --git a/Firestore/core/src/firebase/firestore/model/mutation.cc b/Firestore/core/src/firebase/firestore/model/mutation.cc index d2ef4ebc3d3..e863b34520a 100644 --- a/Firestore/core/src/firebase/firestore/model/mutation.cc +++ b/Firestore/core/src/firebase/firestore/model/mutation.cc @@ -21,6 +21,7 @@ #include "Firestore/core/src/firebase/firestore/model/document.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/model/no_document.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" @@ -54,12 +55,10 @@ bool Mutation::equal_to(const Mutation& other) const { } SetMutation::SetMutation(DocumentKey&& key, - FieldValue&& value, + ObjectValue&& value, Precondition&& precondition) : Mutation(std::move(key), std::move(precondition)), value_(std::move(value)) { - // TODO(rsgowman): convert param to ObjectValue instead of FieldValue? - HARD_ASSERT(value_.type() == FieldValue::Type::Object); } MaybeDocumentPtr SetMutation::ApplyToRemoteDocument( @@ -74,7 +73,7 @@ MaybeDocumentPtr SetMutation::ApplyToRemoteDocument( // the server has accepted the mutation so the precondition must have held. const SnapshotVersion& version = mutation_result.version(); - return absl::make_unique(FieldValue(value_), key(), version, + return absl::make_unique(ObjectValue(value_), key(), version, DocumentState::kCommittedMutations); } @@ -89,7 +88,7 @@ MaybeDocumentPtr SetMutation::ApplyToLocalView( } SnapshotVersion version = GetPostMutationVersion(maybe_doc.get()); - return absl::make_unique(FieldValue(value_), key(), version, + return absl::make_unique(ObjectValue(value_), key(), version, DocumentState::kLocalMutations); } @@ -99,14 +98,12 @@ bool SetMutation::equal_to(const Mutation& other) const { } PatchMutation::PatchMutation(DocumentKey&& key, - FieldValue&& value, + ObjectValue&& value, FieldMask&& mask, Precondition&& precondition) : Mutation(std::move(key), std::move(precondition)), value_(std::move(value)), mask_(std::move(mask)) { - // TODO(rsgowman): convert param to ObjectValue instead of FieldValue? - HARD_ASSERT(value_.type() == FieldValue::Type::Object); } MaybeDocumentPtr PatchMutation::ApplyToRemoteDocument( @@ -131,7 +128,7 @@ MaybeDocumentPtr PatchMutation::ApplyToRemoteDocument( } const SnapshotVersion& version = mutation_result.version(); - FieldValue new_data = PatchDocument(maybe_doc.get()); + ObjectValue new_data = PatchDocument(maybe_doc.get()); return absl::make_unique(std::move(new_data), key(), version, DocumentState::kCommittedMutations); } @@ -147,21 +144,20 @@ MaybeDocumentPtr PatchMutation::ApplyToLocalView( } SnapshotVersion version = GetPostMutationVersion(maybe_doc.get()); - FieldValue new_data = PatchDocument(maybe_doc.get()); + ObjectValue new_data = PatchDocument(maybe_doc.get()); return absl::make_unique(std::move(new_data), key(), version, DocumentState::kLocalMutations); } -FieldValue PatchMutation::PatchDocument(const MaybeDocument* maybe_doc) const { +ObjectValue PatchMutation::PatchDocument(const MaybeDocument* maybe_doc) const { if (maybe_doc && maybe_doc->type() == MaybeDocument::Type::Document) { return PatchObject(static_cast(maybe_doc)->data()); } else { - return PatchObject(FieldValue::EmptyObject()); + return PatchObject(ObjectValue::Empty()); } } -FieldValue PatchMutation::PatchObject(FieldValue obj) const { - HARD_ASSERT(obj.type() == FieldValue::Type::Object); +ObjectValue PatchMutation::PatchObject(ObjectValue obj) const { for (const FieldPath& path : mask_) { if (!path.empty()) { absl::optional new_value = value_.Get(path); diff --git a/Firestore/core/src/firebase/firestore/model/mutation.h b/Firestore/core/src/firebase/firestore/model/mutation.h index 08b4c35aeef..8e4ad562405 100644 --- a/Firestore/core/src/firebase/firestore/model/mutation.h +++ b/Firestore/core/src/firebase/firestore/model/mutation.h @@ -46,7 +46,7 @@ class MutationResult { public: MutationResult( SnapshotVersion&& version, - const std::shared_ptr>& transform_results) + const std::shared_ptr>& transform_results) : version_(std::move(version)), transform_results_(std::move(transform_results)) { } @@ -67,19 +67,19 @@ class MutationResult { /** * The resulting fields returned from the backend after a TransformMutation - * has been committed. Contains one FieldValue for each FieldTransform + * has been committed. Contains one ObjectValue for each FieldTransform * that was in the mutation. * * Will be null if the mutation was not a TransformMutation. */ - const std::shared_ptr>& transform_results() + const std::shared_ptr>& transform_results() const { return transform_results_; } private: const SnapshotVersion version_; - const std::shared_ptr> transform_results_; + const std::shared_ptr> transform_results_; }; /** @@ -217,7 +217,7 @@ inline bool operator!=(const Mutation& lhs, const Mutation& rhs) { class SetMutation : public Mutation { public: SetMutation(DocumentKey&& key, - FieldValue&& value, + ObjectValue&& value, Precondition&& precondition); Type type() const override { @@ -234,7 +234,7 @@ class SetMutation : public Mutation { const Timestamp& local_write_time) const override; /** Returns the object value to use when setting the document. */ - const FieldValue& value() const { + const ObjectValue& value() const { return value_; } @@ -242,7 +242,7 @@ class SetMutation : public Mutation { bool equal_to(const Mutation& other) const override; private: - const FieldValue value_; + const ObjectValue value_; }; /** @@ -261,7 +261,7 @@ class SetMutation : public Mutation { class PatchMutation : public Mutation { public: PatchMutation(DocumentKey&& key, - FieldValue&& value, + ObjectValue&& value, FieldMask&& mask, Precondition&& precondition); @@ -281,7 +281,7 @@ class PatchMutation : public Mutation { /** * Returns the fields and associated values to use when patching the document. */ - const FieldValue& value() const { + const ObjectValue& value() const { return value_; } @@ -297,10 +297,10 @@ class PatchMutation : public Mutation { bool equal_to(const Mutation& other) const override; private: - FieldValue PatchDocument(const MaybeDocument* maybe_doc) const; - FieldValue PatchObject(FieldValue obj) const; + ObjectValue PatchDocument(const MaybeDocument* maybe_doc) const; + ObjectValue PatchObject(ObjectValue obj) const; - const FieldValue value_; + const ObjectValue value_; const FieldMask mask_; }; diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index fc727da2ef0..06703e2e310 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -32,6 +32,7 @@ #include "Firestore/core/include/firebase/firestore/timestamp.h" #include "Firestore/core/src/firebase/firestore/model/document.h" #include "Firestore/core/src/firebase/firestore/model/field_path.h" +#include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/model/no_document.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" #include "Firestore/core/src/firebase/firestore/nanopb/reader.h" @@ -101,8 +102,8 @@ std::vector Serializer::DecodeBytes(const pb_bytes_array_t* bytes) { namespace { -ObjectValue::Map DecodeMapValue(Reader* reader, - const google_firestore_v1_MapValue& map_value); +FieldValue::Map DecodeMapValue(Reader* reader, + const google_firestore_v1_MapValue& map_value); // There's no f:f::model equivalent of StructuredQuery, so we'll create our // own struct for decoding. We could use nanopb's struct, but it's slightly @@ -119,7 +120,7 @@ struct StructuredQuery { // TODO(rsgowman): other fields }; -ObjectValue::Map::value_type DecodeFieldsEntry( +FieldValue::Map::value_type DecodeFieldsEntry( Reader* reader, const google_firestore_v1_Document_FieldsEntry& fields) { std::string key = Serializer::DecodeString(fields.key); FieldValue value = Serializer::DecodeFieldValue(reader, fields.value); @@ -130,33 +131,32 @@ ObjectValue::Map::value_type DecodeFieldsEntry( return {}; } - return ObjectValue::Map::value_type{std::move(key), std::move(value)}; + return FieldValue::Map::value_type{std::move(key), std::move(value)}; } -ObjectValue::Map DecodeFields( +FieldValue::Map DecodeFields( Reader* reader, size_t count, const google_firestore_v1_Document_FieldsEntry* fields) { - ObjectValue::Map result; + FieldValue::Map result; for (size_t i = 0; i < count; i++) { - ObjectValue::Map::value_type kv = DecodeFieldsEntry(reader, fields[i]); + FieldValue::Map::value_type kv = DecodeFieldsEntry(reader, fields[i]); result = result.insert(std::move(kv.first), std::move(kv.second)); } return result; } -google_firestore_v1_MapValue EncodeMapValue( - const ObjectValue::Map& object_value_map) { +google_firestore_v1_MapValue EncodeMapValue(const ObjectValue& object_value) { google_firestore_v1_MapValue result{}; - size_t count = object_value_map.size(); + size_t count = object_value.GetInternalValue().size(); result.fields_count = static_cast(count); result.fields = MakeArray(count); int i = 0; - for (const auto& kv : object_value_map) { + for (const auto& kv : object_value.GetInternalValue()) { result.fields[i].key = Serializer::EncodeString(kv.first); result.fields[i].value = Serializer::EncodeFieldValue(kv.second); i++; @@ -165,9 +165,9 @@ google_firestore_v1_MapValue EncodeMapValue( return result; } -ObjectValue::Map DecodeMapValue(Reader* reader, - const google_firestore_v1_MapValue& map_value) { - ObjectValue::Map result; +FieldValue::Map DecodeMapValue(Reader* reader, + const google_firestore_v1_MapValue& map_value) { + FieldValue::Map result; for (size_t i = 0; i < map_value.fields_count; i++) { std::string key = Serializer::DecodeString(map_value.fields[i].key); @@ -310,8 +310,7 @@ google_firestore_v1_Value Serializer::EncodeFieldValue( case FieldValue::Type::Object: result.which_value_type = google_firestore_v1_Value_map_value_tag; - result.map_value = - EncodeMapValue(field_value.object_value().internal_value); + result.map_value = EncodeMapValue(ObjectValue(field_value)); break; default: @@ -416,11 +415,11 @@ google_firestore_v1_Document Serializer::EncodeDocument( result.name = EncodeString(EncodeKey(key)); // Encode Document.fields (unless it's empty) - size_t count = object_value.internal_value.size(); + size_t count = object_value.GetInternalValue().size(); result.fields_count = static_cast(count); result.fields = MakeArray(count); int i = 0; - for (const auto& kv : object_value.internal_value) { + for (const auto& kv : object_value.GetInternalValue()) { result.fields[i].key = EncodeString(kv.first); result.fields[i].value = EncodeFieldValue(kv.second); i++; @@ -457,7 +456,7 @@ std::unique_ptr Serializer::DecodeFoundDocument( "Tried to deserialize a found document from a missing document."); DocumentKey key = DecodeKey(reader, DecodeString(response.found.name)); - ObjectValue::Map value = + FieldValue::Map value = DecodeFields(reader, response.found.fields_count, response.found.fields); SnapshotVersion version = DecodeSnapshotVersion(reader, response.found.update_time); @@ -466,7 +465,7 @@ std::unique_ptr Serializer::DecodeFoundDocument( reader->Fail("Got a document response with no snapshot version"); } - return absl::make_unique(FieldValue::FromMap(std::move(value)), + return absl::make_unique(ObjectValue::FromMap(std::move(value)), std::move(key), std::move(version), DocumentState::kSynced); } @@ -492,12 +491,12 @@ std::unique_ptr Serializer::DecodeMissingDocument( std::unique_ptr Serializer::DecodeDocument( Reader* reader, const google_firestore_v1_Document& proto) const { - ObjectValue::Map fields_internal = + FieldValue::Map fields_internal = DecodeFields(reader, proto.fields_count, proto.fields); SnapshotVersion version = DecodeSnapshotVersion(reader, proto.update_time); return absl::make_unique( - FieldValue::FromMap(std::move(fields_internal)), + ObjectValue::FromMap(std::move(fields_internal)), DecodeKey(reader, DecodeString(proto.name)), std::move(version), DocumentState::kSynced); } @@ -514,16 +513,14 @@ google_firestore_v1_Write Serializer::EncodeMutation( case Mutation::Type::kSet: { result.which_operation = google_firestore_v1_Write_update_tag; result.update = EncodeDocument( - mutation.key(), - static_cast(mutation).value().object_value()); + mutation.key(), static_cast(mutation).value()); return result; } case Mutation::Type::kPatch: { result.which_operation = google_firestore_v1_Write_update_tag; auto patch_mutation = static_cast(mutation); - result.update = - EncodeDocument(mutation.key(), patch_mutation.value().object_value()); + result.update = EncodeDocument(mutation.key(), patch_mutation.value()); result.update_mask = EncodeDocumentMask(patch_mutation.mask()); return result; } @@ -565,7 +562,7 @@ std::unique_ptr Serializer::DecodeMutation( switch (mutation.which_operation) { case google_firestore_v1_Write_update_tag: { DocumentKey key = DecodeKey(reader, DecodeString(mutation.update.name)); - FieldValue value = FieldValue::FromMap(DecodeFields( + ObjectValue value = ObjectValue::FromMap(DecodeFields( reader, mutation.update.fields_count, mutation.update.fields)); FieldMask mask = DecodeDocumentMask(mutation.update_mask); if (mask.size() > 0) { diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.h b/Firestore/core/src/firebase/firestore/remote/serializer.h index 086db7d191f..9fb5246f2f8 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.h +++ b/Firestore/core/src/firebase/firestore/remote/serializer.h @@ -194,11 +194,6 @@ class Serializer { std::unique_ptr DecodeDocument( nanopb::Reader* reader, const google_firestore_v1_Document& proto) const; - static void EncodeObjectMap(const model::ObjectValue::Map& object_value_map, - uint32_t map_tag, - uint32_t key_tag, - uint32_t value_tag); - static google_protobuf_Timestamp EncodeVersion( const model::SnapshotVersion& version); @@ -223,10 +218,6 @@ class Serializer { nanopb::Reader* reader, const google_firestore_v1_BatchGetDocumentsResponse& response) const; - static void EncodeFieldsEntry(const model::ObjectValue::Map::value_type& kv, - uint32_t key_tag, - uint32_t value_tag); - std::string EncodeQueryPath(const model::ResourcePath& path) const; const model::DatabaseId& database_id_; diff --git a/Firestore/core/test/firebase/firestore/local/local_serializer_test.cc b/Firestore/core/test/firebase/firestore/local/local_serializer_test.cc index f7d739c461f..dac54256407 100644 --- a/Firestore/core/test/firebase/firestore/local/local_serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/local/local_serializer_test.cc @@ -58,6 +58,7 @@ using model::MaybeDocument; using model::Mutation; using model::MutationBatch; using model::NoDocument; +using model::ObjectValue; using model::PatchMutation; using model::Precondition; using model::SetMutation; @@ -258,8 +259,8 @@ TEST_F(LocalSerializerTest, EncodesMutationBatch) { {"num", FieldValue::FromInteger(1)}}); std::unique_ptr patch = absl::make_unique( Key("bar/baz"), - FieldValue::FromMap({{"a", FieldValue::FromString("b")}, - {"num", FieldValue::FromInteger(1)}}), + ObjectValue::FromMap({{"a", FieldValue::FromString("b")}, + {"num", FieldValue::FromInteger(1)}}), FieldMask({FieldPath({"a"})}), Precondition::Exists(true)); std::unique_ptr del = testutil::DeleteMutation("baz/quux"); diff --git a/Firestore/core/test/firebase/firestore/model/document_test.cc b/Firestore/core/test/firebase/firestore/model/document_test.cc index 0aaba50abbb..ae09b75825d 100644 --- a/Firestore/core/test/firebase/firestore/model/document_test.cc +++ b/Firestore/core/test/firebase/firestore/model/document_test.cc @@ -16,6 +16,7 @@ #include "Firestore/core/src/firebase/firestore/model/document.h" +#include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/model/unknown_document.h" #include "absl/strings/string_view.h" @@ -32,7 +33,7 @@ inline Document MakeDocument(const absl::string_view data, const Timestamp& timestamp, DocumentState document_state) { return Document( - FieldValue::FromMap({{"field", FieldValue::FromString(data.data())}}), + ObjectValue::FromMap({{"field", FieldValue::FromString(data.data())}}), DocumentKey::FromPathString(path.data()), SnapshotVersion(timestamp), document_state); } @@ -43,7 +44,7 @@ TEST(Document, Getter) { const Document& doc = MakeDocument("foo", "i/am/a/path", Timestamp(123, 456), DocumentState::kLocalMutations); EXPECT_EQ(MaybeDocument::Type::Document, doc.type()); - EXPECT_EQ(FieldValue::FromMap({{"field", FieldValue::FromString("foo")}}), + EXPECT_EQ(ObjectValue::FromMap({{"field", FieldValue::FromString("foo")}}), doc.data()); EXPECT_EQ(DocumentKey::FromPathString("i/am/a/path"), doc.key()); EXPECT_EQ(SnapshotVersion(Timestamp(123, 456)), doc.version()); @@ -74,11 +75,11 @@ TEST(Document, Comparison) { // Document and MaybeDocument will not equal. In particular, Document and // NoDocument will not equal, which I won't test here. - EXPECT_NE(Document(FieldValue(FieldValue::EmptyObject()), - DocumentKey::FromPathString("same/path"), - SnapshotVersion(Timestamp()), DocumentState::kSynced), - UnknownDocument(DocumentKey::FromPathString("same/path"), - SnapshotVersion(Timestamp()))); + EXPECT_NE( + Document(ObjectValue::Empty(), DocumentKey::FromPathString("same/path"), + SnapshotVersion(Timestamp()), DocumentState::kSynced), + UnknownDocument(DocumentKey::FromPathString("same/path"), + SnapshotVersion(Timestamp()))); } } // namespace model diff --git a/Firestore/core/test/firebase/firestore/model/field_value_test.cc b/Firestore/core/test/firebase/firestore/model/field_value_test.cc index fa3902d52f4..b1d1f8b70b5 100644 --- a/Firestore/core/test/firebase/firestore/model/field_value_test.cc +++ b/Firestore/core/test/firebase/firestore/model/field_value_test.cc @@ -189,19 +189,16 @@ TEST(FieldValue, ArrayType) { } TEST(FieldValue, ObjectType) { - const FieldValue empty = FieldValue::FromMap(ObjectValue::Empty()); - ObjectValue::Map object{{"null", FieldValue::Null()}, - {"true", FieldValue::True()}, - {"false", FieldValue::False()}}; + const ObjectValue empty = ObjectValue::Empty(); + FieldValue::Map object{{"null", FieldValue::Null()}, + {"true", FieldValue::True()}, + {"false", FieldValue::False()}}; // copy the map - const FieldValue small = FieldValue::FromMap(object); - ObjectValue::Map another_object{{"null", FieldValue::Null()}, - {"true", FieldValue::False()}}; + const ObjectValue small = ObjectValue::FromMap(object); + FieldValue::Map another_object{{"null", FieldValue::Null()}, + {"true", FieldValue::False()}}; // move the array - const FieldValue large = FieldValue::FromMap(std::move(another_object)); - EXPECT_EQ(Type::Object, empty.type()); - EXPECT_EQ(Type::Object, small.type()); - EXPECT_EQ(Type::Object, large.type()); + const ObjectValue large = ObjectValue::FromMap(std::move(another_object)); EXPECT_TRUE(empty < small); EXPECT_FALSE(small < empty); EXPECT_FALSE(small < small); @@ -334,18 +331,18 @@ TEST(FieldValue, Copy) { clone = null_value; EXPECT_EQ(FieldValue::Null(), clone); - const FieldValue object_value = FieldValue::FromMap(ObjectValue::Map{ - {"true", FieldValue::True()}, {"false", FieldValue::False()}}); + const FieldValue object_value = FieldValue::FromMap( + {{"true", FieldValue::True()}, {"false", FieldValue::False()}}); clone = object_value; - EXPECT_EQ(FieldValue::FromMap(ObjectValue::Map{ - {"true", FieldValue::True()}, {"false", FieldValue::False()}}), + EXPECT_EQ(FieldValue::FromMap( + {{"true", FieldValue::True()}, {"false", FieldValue::False()}}), clone); - EXPECT_EQ(FieldValue::FromMap(ObjectValue::Map{ - {"true", FieldValue::True()}, {"false", FieldValue::False()}}), + EXPECT_EQ(FieldValue::FromMap( + {{"true", FieldValue::True()}, {"false", FieldValue::False()}}), object_value); clone = *&clone; - EXPECT_EQ(FieldValue::FromMap(ObjectValue::Map{ - {"true", FieldValue::True()}, {"false", FieldValue::False()}}), + EXPECT_EQ(FieldValue::FromMap( + {{"true", FieldValue::True()}, {"false", FieldValue::False()}}), clone); clone = null_value; EXPECT_EQ(FieldValue::Null(), clone); @@ -425,11 +422,11 @@ TEST(FieldValue, Move) { clone = FieldValue::Null(); EXPECT_EQ(FieldValue::Null(), clone); - FieldValue object_value = FieldValue::FromMap(ObjectValue::Map{ - {"true", FieldValue::True()}, {"false", FieldValue::False()}}); + FieldValue object_value = FieldValue::FromMap( + {{"true", FieldValue::True()}, {"false", FieldValue::False()}}); clone = std::move(object_value); - EXPECT_EQ(FieldValue::FromMap(ObjectValue::Map{ - {"true", FieldValue::True()}, {"false", FieldValue::False()}}), + EXPECT_EQ(FieldValue::FromMap( + {{"true", FieldValue::True()}, {"false", FieldValue::False()}}), clone); clone = FieldValue::Null(); EXPECT_EQ(FieldValue::Null(), clone); @@ -489,13 +486,13 @@ TEST(FieldValue, CompareWithOperator) { TEST(FieldValue, Set) { // Set a field in an object. - const FieldValue value = FieldValue::FromMap({ + const ObjectValue value = ObjectValue::FromMap({ {"a", FieldValue::FromString("A")}, {"b", FieldValue::FromMap({ {"ba", FieldValue::FromString("BA")}, })}, }); - const FieldValue expected = FieldValue::FromMap({ + const ObjectValue expected = ObjectValue::FromMap({ {"a", FieldValue::FromString("A")}, {"b", FieldValue::FromMap({ {"ba", FieldValue::FromString("BA")}, @@ -508,10 +505,10 @@ TEST(FieldValue, Set) { TEST(FieldValue, SetRecursive) { // Set a field in a new object. - const FieldValue value = FieldValue::FromMap({ + const ObjectValue value = ObjectValue::FromMap({ {"a", FieldValue::FromString("A")}, }); - const FieldValue expected = FieldValue::FromMap({ + const ObjectValue expected = ObjectValue::FromMap({ {"a", FieldValue::FromString("A")}, {"b", FieldValue::FromMap({ {"bb", FieldValue::FromString("BB")}, @@ -522,14 +519,14 @@ TEST(FieldValue, SetRecursive) { } TEST(FieldValue, Delete) { - const FieldValue value = FieldValue::FromMap({ + const ObjectValue value = ObjectValue::FromMap({ {"a", FieldValue::FromString("A")}, {"b", FieldValue::FromMap({ {"ba", FieldValue::FromString("BA")}, {"bb", FieldValue::FromString("BB")}, })}, }); - const FieldValue expected = FieldValue::FromMap({ + const ObjectValue expected = ObjectValue::FromMap({ {"a", FieldValue::FromString("A")}, {"b", FieldValue::FromMap({ {"ba", FieldValue::FromString("BA")}, @@ -539,7 +536,7 @@ TEST(FieldValue, Delete) { } TEST(FieldValue, DeleteNothing) { - const FieldValue value = FieldValue::FromMap({ + const ObjectValue value = ObjectValue::FromMap({ {"a", FieldValue::FromString("A")}, {"b", FieldValue::FromMap({ {"ba", FieldValue::FromString("BA")}, @@ -550,7 +547,7 @@ TEST(FieldValue, DeleteNothing) { } TEST(FieldValue, Get) { - const FieldValue value = FieldValue::FromMap({ + const ObjectValue value = ObjectValue::FromMap({ {"a", FieldValue::FromString("A")}, {"b", FieldValue::FromMap({ {"ba", FieldValue::FromString("BA")}, @@ -563,7 +560,7 @@ TEST(FieldValue, Get) { } TEST(FieldValue, GetNothing) { - const FieldValue value = FieldValue::FromMap({ + const ObjectValue value = ObjectValue::FromMap({ {"a", FieldValue::FromString("A")}, {"b", FieldValue::FromMap({ {"ba", FieldValue::FromString("BA")}, diff --git a/Firestore/core/test/firebase/firestore/model/mutation_test.cc b/Firestore/core/test/firebase/firestore/model/mutation_test.cc index b688511d134..849ab616b93 100644 --- a/Firestore/core/test/firebase/firestore/model/mutation_test.cc +++ b/Firestore/core/test/firebase/firestore/model/mutation_test.cc @@ -114,7 +114,7 @@ TEST(Mutation, DeletesValuesFromTheFieldMask) { {"baz", FieldValue::FromString("baz-value")}})}})); std::unique_ptr patch = - PatchMutation("collection/key", ObjectValue::Empty(), {Field("foo.bar")}); + PatchMutation("collection/key", FieldValue::Map(), {Field("foo.bar")}); MaybeDocumentPtr patch_doc = patch->ApplyToLocalView(base_doc, base_doc.get(), Timestamp::Now()); diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc index a7221e68890..307a36e9e33 100644 --- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc @@ -36,6 +36,7 @@ #include "Firestore/Protos/cpp/google/firestore/v1/firestore.pb.h" #include "Firestore/core/include/firebase/firestore/firestore_errors.h" #include "Firestore/core/include/firebase/firestore/timestamp.h" +#include "Firestore/core/src/firebase/firestore/model/field_path.h" #include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/nanopb/reader.h" @@ -195,11 +196,10 @@ class SerializerTest : public ::testing::Test { std::vector EncodeDocument(Serializer* serializer, const DocumentKey& key, - const FieldValue& value) { + const ObjectValue& value) { std::vector bytes; Writer writer = Writer::Wrap(&bytes); - google_firestore_v1_Document proto = - serializer->EncodeDocument(key, value.object_value()); + google_firestore_v1_Document proto = serializer->EncodeDocument(key, value); writer.WriteNanopbMessage(google_firestore_v1_Document_fields, &proto); serializer->FreeNanopbMessage(google_firestore_v1_Document_fields, &proto); return bytes; @@ -316,7 +316,7 @@ class SerializerTest : public ::testing::Test { void ExpectSerializationRoundTrip( const DocumentKey& key, - const FieldValue& value, + const ObjectValue& value, const SnapshotVersion& update_time, const v1::BatchGetDocumentsResponse& proto) { std::vector bytes = EncodeDocument(&serializer, key, value); @@ -349,7 +349,7 @@ class SerializerTest : public ::testing::Test { void ExpectDeserializationRoundTrip( const DocumentKey& key, - const absl::optional value, + const absl::optional value, const SnapshotVersion& version, // either update_time or read_time const v1::BatchGetDocumentsResponse& proto) { size_t size = proto.ByteSizeLong(); @@ -797,7 +797,7 @@ TEST_F(SerializerTest, BadKey) { TEST_F(SerializerTest, EncodesEmptyDocument) { DocumentKey key = DocumentKey::FromPathString("path/to/the/doc"); - FieldValue empty_value = FieldValue::EmptyObject(); + ObjectValue empty_value = ObjectValue::Empty(); SnapshotVersion update_time = SnapshotVersion{{1234, 5678}}; v1::BatchGetDocumentsResponse proto; @@ -817,7 +817,7 @@ TEST_F(SerializerTest, EncodesEmptyDocument) { TEST_F(SerializerTest, EncodesNonEmptyDocument) { DocumentKey key = DocumentKey::FromPathString("path/to/the/doc"); - FieldValue fields = FieldValue::FromMap({ + ObjectValue fields = ObjectValue::FromMap({ {"foo", FieldValue::FromString("bar")}, {"two", FieldValue::FromInteger(2)}, {"nested", FieldValue::FromMap({ diff --git a/Firestore/core/test/firebase/firestore/testutil/testutil.cc b/Firestore/core/test/firebase/firestore/testutil/testutil.cc index e24185408b8..e08678b932b 100644 --- a/Firestore/core/test/firebase/firestore/testutil/testutil.cc +++ b/Firestore/core/test/firebase/firestore/testutil/testutil.cc @@ -24,15 +24,17 @@ namespace testutil { std::unique_ptr PatchMutation( absl::string_view path, - const model::ObjectValue::Map& values, + const model::FieldValue::Map& values, // TODO(rsgowman): Investigate changing update_mask to a set. const std::vector* update_mask) { - model::FieldValue object_value = model::FieldValue::EmptyObject(); + model::ObjectValue object_value = model::ObjectValue::Empty(); std::set object_mask; for (const auto& kv : values) { model::FieldPath field_path = Field(kv.first); object_mask.insert(field_path); + // TODO(rsgowman): This will abort if kv.second.string_value.type() != + // String if (kv.second.string_value() != kDeleteSentinel) { object_value = object_value.Set(field_path, kv.second); } diff --git a/Firestore/core/test/firebase/firestore/testutil/testutil.h b/Firestore/core/test/firebase/firestore/testutil/testutil.h index 0568ef7530b..07a1c8b8e81 100644 --- a/Firestore/core/test/firebase/firestore/testutil/testutil.h +++ b/Firestore/core/test/firebase/firestore/testutil/testutil.h @@ -80,9 +80,9 @@ inline model::SnapshotVersion Version(int64_t version) { inline model::Document Doc( absl::string_view key, int64_t version = 0, - const model::ObjectValue::Map& data = model::ObjectValue::Empty(), + const model::FieldValue::Map& data = model::FieldValue::Map(), model::DocumentState document_state = model::DocumentState::kSynced) { - return model::Document{model::FieldValue::FromMap(data), Key(key), + return model::Document{model::ObjectValue::FromMap(data), Key(key), Version(version), document_state}; } @@ -141,20 +141,20 @@ inline core::Query Query(absl::string_view path) { inline std::unique_ptr SetMutation( absl::string_view path, - const model::ObjectValue::Map& values = model::ObjectValue::Empty()) { + const model::FieldValue::Map& values = model::FieldValue::Map()) { return absl::make_unique( - Key(path), model::FieldValue::FromMap(values), + Key(path), model::ObjectValue::FromMap(values), model::Precondition::None()); } std::unique_ptr PatchMutation( absl::string_view path, - const model::ObjectValue::Map& values = model::ObjectValue::Empty(), + const model::FieldValue::Map& values = model::FieldValue::Map(), const std::vector* update_mask = nullptr); inline std::unique_ptr PatchMutation( absl::string_view path, - const model::ObjectValue::Map& values, + const model::FieldValue::Map& values, const std::vector& update_mask) { return PatchMutation(path, values, &update_mask); }