Add ObjectValue::Map alias (#967)

3 files changed, 66 insertions, 32 deletions

diff --git a/Firestore/core/src/firebase/firestore/model/field_value.cc b/Firestore/core/src/firebase/firestore/model/field_value.cc
index a38a676..2c1af53 100644
--- a/Firestore/core/src/firebase/firestore/model/field_value.cc
+++ b/Firestore/core/src/firebase/firestore/model/field_value.cc
@@ -113,8 +113,8 @@ FieldValue& FieldValue::operator=(const FieldValue& value) {
     }
     case Type::Object: {
       // copy-and-swap
-      std::map<std::string, FieldValue> tmp = value.object_value_;
-      std::swap(object_value_, tmp);
+      ObjectValue::Map tmp = value.object_value_.internal_value;
+      std::swap(object_value_.internal_value, tmp);
       break;
     }
     default:
@@ -280,16 +280,15 @@ FieldValue FieldValue::ArrayValue(std::vector<FieldValue>&& value) {
   return result;
 }
 
-FieldValue FieldValue::ObjectValue(
-    const std::map<std::string, FieldValue>& value) {
-  std::map<std::string, FieldValue> copy(value);
-  return ObjectValue(std::move(copy));
+FieldValue FieldValue::ObjectValueFromMap(const ObjectValue::Map& value) {
+  ObjectValue::Map copy(value);
+  return ObjectValueFromMap(std::move(copy));
 }
 
-FieldValue FieldValue::ObjectValue(std::map<std::string, FieldValue>&& value) {
+FieldValue FieldValue::ObjectValueFromMap(ObjectValue::Map&& value) {
   FieldValue result;
   result.SwitchTo(Type::Object);
-  std::swap(result.object_value_, value);
+  std::swap(result.object_value_.internal_value, value);
   return result;
 }
 
@@ -386,7 +385,7 @@ void FieldValue::SwitchTo(const Type type) {
       array_value_.~vector();
       break;
     case Type::Object:
-      object_value_.~map();
+      object_value_.internal_value.~map();
       break;
     default: {}  // The other types where there is nothing to worry about.
   }
@@ -417,7 +416,7 @@ void FieldValue::SwitchTo(const Type type) {
       new (&array_value_) std::vector<FieldValue>();
       break;
     case Type::Object:
-      new (&object_value_) std::map<std::string, FieldValue>();
+      new (&object_value_) ObjectValue{};
       break;
     default: {}  // The other types where there is nothing to worry about.
   }
diff --git a/Firestore/core/src/firebase/firestore/model/field_value.h b/Firestore/core/src/firebase/firestore/model/field_value.h
index 9111ffb..c42d533 100644
--- a/Firestore/core/src/firebase/firestore/model/field_value.h
+++ b/Firestore/core/src/firebase/firestore/model/field_value.h
@@ -47,6 +47,19 @@ struct ReferenceValue {
   const DatabaseId* database_id;
 };
 
+// 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.
+class FieldValue;
+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 = std::map<std::string, FieldValue>;
+  Map internal_value;
+};
+
 /**
  * tagged-union class representing an immutable data value as stored in
  * Firestore. FieldValue represents all the different kinds of values
@@ -111,9 +124,9 @@ class FieldValue {
     return string_value_;
   }
 
-  const std::map<std::string, FieldValue>& object_value() const {
+  const ObjectValue object_value() const {
     FIREBASE_ASSERT(tag_ == Type::Object);
-    return object_value_;
+    return ObjectValue{object_value_};
   }
 
   /** factory methods. */
@@ -139,8 +152,8 @@ class FieldValue {
   static FieldValue GeoPointValue(const GeoPoint& value);
   static FieldValue ArrayValue(const std::vector<FieldValue>& value);
   static FieldValue ArrayValue(std::vector<FieldValue>&& value);
-  static FieldValue ObjectValue(const std::map<std::string, FieldValue>& value);
-  static FieldValue ObjectValue(std::map<std::string, FieldValue>&& value);
+  static FieldValue ObjectValueFromMap(const ObjectValue::Map& value);
+  static FieldValue ObjectValueFromMap(ObjectValue::Map&& value);
 
   friend bool operator<(const FieldValue& lhs, const FieldValue& rhs);
 
@@ -167,7 +180,7 @@ class FieldValue {
     firebase::firestore::model::ReferenceValue reference_value_;
     GeoPoint geo_point_value_;
     std::vector<FieldValue> array_value_;
-    std::map<std::string, FieldValue> object_value_;
+    ObjectValue object_value_;
   };
 };
 
@@ -194,6 +207,31 @@ inline bool operator==(const FieldValue& lhs, const FieldValue& rhs) {
   return !(lhs != rhs);
 }
 
+/** Compares against another ObjectValue. */
+inline bool operator<(const ObjectValue& lhs, const ObjectValue& rhs) {
+  return lhs.internal_value < rhs.internal_value;
+}
+
+inline bool operator>(const ObjectValue& lhs, const ObjectValue& rhs) {
+  return rhs < lhs;
+}
+
+inline bool operator>=(const ObjectValue& lhs, const ObjectValue& rhs) {
+  return !(lhs < rhs);
+}
+
+inline bool operator<=(const ObjectValue& lhs, const ObjectValue& rhs) {
+  return !(lhs > rhs);
+}
+
+inline bool operator!=(const ObjectValue& lhs, const ObjectValue& rhs) {
+  return lhs < rhs || lhs > rhs;
+}
+
+inline bool operator==(const ObjectValue& lhs, const ObjectValue& rhs) {
+  return !(lhs != rhs);
+}
+
 }  // namespace model
 }  // namespace firestore
 }  // namespace firebase
diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc
index bab5856..5483e1f 100644
--- a/Firestore/core/src/firebase/firestore/remote/serializer.cc
+++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc
@@ -31,22 +31,20 @@ namespace firestore {
 namespace remote {
 
 using firebase::firestore::model::FieldValue;
+using firebase::firestore::model::ObjectValue;
 
 namespace {
 
 class Writer;
 
-void EncodeObject(Writer* writer,
-                  const std::map<std::string, FieldValue>& object_value);
+void EncodeObject(Writer* writer, const ObjectValue& object_value);
 
-std::map<std::string, FieldValue> DecodeObject(pb_istream_t* stream);
+ObjectValue DecodeObject(pb_istream_t* stream);
 
 /**
  * Docs TODO(rsgowman). But currently, this just wraps the underlying nanopb
  * pb_ostream_t.
  */
-// TODO(rsgowman): Define ObjectT alias (in field_value.h) and use it
-// throughout.
 class Writer {
  public:
   /**
@@ -371,7 +369,8 @@ FieldValue DecodeFieldValueImpl(pb_istream_t* stream) {
     case google_firestore_v1beta1_Value_string_value_tag:
       return FieldValue::StringValue(DecodeString(stream));
     case google_firestore_v1beta1_Value_map_value_tag:
-      return FieldValue::ObjectValue(DecodeObject(stream));
+      return FieldValue::ObjectValueFromMap(
+          DecodeObject(stream).internal_value);
 
     default:
       // TODO(rsgowman): figure out error handling
@@ -474,8 +473,7 @@ FieldValue DecodeNestedFieldValue(pb_istream_t* stream) {
  *
  * @param kv The individual key/value pair to write.
  */
-void EncodeFieldsEntry(Writer* writer,
-                       const std::pair<std::string, FieldValue>& kv) {
+void EncodeFieldsEntry(Writer* writer, const ObjectValue::Map::value_type& kv) {
   // Write the key (string)
   writer->WriteTag(PB_WT_STRING,
                    google_firestore_v1beta1_MapValue_FieldsEntry_key_tag);
@@ -488,7 +486,7 @@ void EncodeFieldsEntry(Writer* writer,
       [&kv](Writer* writer) { EncodeFieldValueImpl(writer, kv.second); });
 }
 
-std::pair<std::string, FieldValue> DecodeFieldsEntry(pb_istream_t* stream) {
+ObjectValue::Map::value_type DecodeFieldsEntry(pb_istream_t* stream) {
   pb_wire_type_t wire_type;
   uint32_t tag;
   bool eof;
@@ -513,11 +511,10 @@ std::pair<std::string, FieldValue> DecodeFieldsEntry(pb_istream_t* stream) {
   return {key, value};
 }
 
-void EncodeObject(Writer* writer,
-                  const std::map<std::string, FieldValue>& object_value) {
+void EncodeObject(Writer* writer, const ObjectValue& object_value) {
   writer->WriteNestedMessage([&object_value](Writer* writer) {
     // Write each FieldsEntry (i.e. key-value pair.)
-    for (const auto& kv : object_value) {
+    for (const auto& kv : object_value.internal_value) {
       writer->WriteTag(PB_WT_STRING,
                        google_firestore_v1beta1_MapValue_FieldsEntry_key_tag);
       writer->WriteNestedMessage(
@@ -528,18 +525,18 @@ void EncodeObject(Writer* writer,
   });
 }
 
-std::map<std::string, FieldValue> DecodeObject(pb_istream_t* stream) {
+ObjectValue DecodeObject(pb_istream_t* stream) {
   google_firestore_v1beta1_MapValue map_value =
       google_firestore_v1beta1_MapValue_init_zero;
-  std::map<std::string, FieldValue> result;
+  ObjectValue::Map result;
   // NB: c-style callbacks can't use *capturing* lambdas, so we'll pass in the
   // object_value via the arg field (and therefore need to do a bunch of
   // casting).
   map_value.fields.funcs.decode = [](pb_istream_t* stream, const pb_field_t*,
                                      void** arg) -> bool {
-    auto& result = *static_cast<std::map<std::string, FieldValue>*>(*arg);
+    auto& result = *static_cast<ObjectValue::Map*>(*arg);
 
-    std::pair<std::string, FieldValue> fv = DecodeFieldsEntry(stream);
+    ObjectValue::Map::value_type fv = DecodeFieldsEntry(stream);
 
     // Sanity check: ensure that this key doesn't already exist in the map.
     // TODO(rsgowman): figure out error handling: We can do better than a failed
@@ -560,7 +557,7 @@ std::map<std::string, FieldValue> DecodeObject(pb_istream_t* stream) {
     abort();
   }
 
-  return result;
+  return ObjectValue{result};
 }
 
 }  // namespace