Create ObjectValue as a wrapper around a FieldValue which contains a map (#2527)

Author: rsgowman

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<pb_size_t>::max(),
               "Unable to encode specified document. Too many fields.");
   result.fields_count = static_cast<pb_size_t>(count);
   result.fields = MakeArray<google_firestore_v1_Document_FieldsEntry>(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++;

Firestore/core/src/firebase/firestore/model/document.cc

@@ -24,15 +24,14 @@ namespace firebase {
 namespace firestore {
 namespace model {
 
-Document::Document(FieldValue&& data,
+Document::Document(ObjectValue&& data,
                    DocumentKey key,
                    SnapshotVersion version,
                    DocumentState document_state)
     : MaybeDocument(std::move(key), std::move(version)),
       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 {

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_;
 };
 

Firestore/core/src/firebase/firestore/model/field_value.cc

@@ -23,6 +23,7 @@
 #include <utility>
 #include <vector>
 
+#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> 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<FieldValue> 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> 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<FieldValue>&& 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<std::vector<FieldValue>>();
       break;
     case Type::Object:
-      object_value_.~unique_ptr<ObjectValue>();
+      object_value_.~unique_ptr<Map>();
       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<std::vector<FieldValue>>());
       break;
     case Type::Object:
-      new (&object_value_)
-          std::unique_ptr<ObjectValue>(absl::make_unique<ObjectValue>());
+      new (&object_value_) std::unique_ptr<Map>(absl::make_unique<Map>());
       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