diff options
6 files changed, 102 insertions, 70 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 diff --git a/Firestore/core/test/firebase/firestore/model/document_test.cc b/Firestore/core/test/firebase/firestore/model/document_test.cc index 6b72360..066c875 100644 --- a/Firestore/core/test/firebase/firestore/model/document_test.cc +++ b/Firestore/core/test/firebase/firestore/model/document_test.cc @@ -29,7 +29,7 @@ inline Document MakeDocument(const absl::string_view data, const absl::string_view path, const Timestamp& timestamp, bool has_local_mutations) { - return Document(FieldValue::ObjectValue( + return Document(FieldValue::ObjectValueFromMap( {{"field", FieldValue::StringValue(data.data())}}), DocumentKey::FromPathString(path.data()), SnapshotVersion(timestamp), has_local_mutations); @@ -41,9 +41,9 @@ TEST(Document, Getter) { const Document& doc = MakeDocument("foo", "i/am/a/path", Timestamp(123, 456), true); EXPECT_EQ(MaybeDocument::Type::Document, doc.type()); - EXPECT_EQ( - FieldValue::ObjectValue({{"field", FieldValue::StringValue("foo")}}), - doc.data()); + EXPECT_EQ(FieldValue::ObjectValueFromMap( + {{"field", FieldValue::StringValue("foo")}}), + doc.data()); EXPECT_EQ(DocumentKey::FromPathString("i/am/a/path"), doc.key()); EXPECT_EQ(SnapshotVersion(Timestamp(123, 456)), doc.version()); EXPECT_TRUE(doc.has_local_mutations()); @@ -64,7 +64,7 @@ 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::ObjectValue({}), + EXPECT_NE(Document(FieldValue::ObjectValueFromMap({}), DocumentKey::FromPathString("same/path"), SnapshotVersion(Timestamp()), false), MaybeDocument(DocumentKey::FromPathString("same/path"), 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 93879f9..40be2d5 100644 --- a/Firestore/core/test/firebase/firestore/model/field_value_test.cc +++ b/Firestore/core/test/firebase/firestore/model/field_value_test.cc @@ -189,17 +189,17 @@ TEST(FieldValue, ArrayType) { } TEST(FieldValue, ObjectType) { - const FieldValue empty = - FieldValue::ObjectValue(std::map<std::string, FieldValue>{}); - std::map<std::string, FieldValue> object{{"null", FieldValue::NullValue()}, - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}; + const FieldValue empty = FieldValue::ObjectValueFromMap({}); + ObjectValue::Map object{{"null", FieldValue::NullValue()}, + {"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}; // copy the map - const FieldValue small = FieldValue::ObjectValue(object); - std::map<std::string, FieldValue> another_object{ - {"null", FieldValue::NullValue()}, {"true", FieldValue::FalseValue()}}; + const FieldValue small = FieldValue::ObjectValueFromMap(object); + ObjectValue::Map another_object{{"null", FieldValue::NullValue()}, + {"true", FieldValue::FalseValue()}}; // move the array - const FieldValue large = FieldValue::ObjectValue(std::move(another_object)); + const FieldValue large = + FieldValue::ObjectValueFromMap(std::move(another_object)); EXPECT_EQ(Type::Object, empty.type()); EXPECT_EQ(Type::Object, small.type()); EXPECT_EQ(Type::Object, large.type()); @@ -335,22 +335,22 @@ TEST(FieldValue, Copy) { clone = null_value; EXPECT_EQ(FieldValue::NullValue(), clone); - const FieldValue object_value = FieldValue::ObjectValue( - std::map<std::string, FieldValue>{{"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}); + const FieldValue object_value = FieldValue::ObjectValueFromMap( + ObjectValue::Map{{"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}); clone = object_value; - EXPECT_EQ(FieldValue::ObjectValue(std::map<std::string, FieldValue>{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}), + EXPECT_EQ(FieldValue::ObjectValueFromMap( + ObjectValue::Map{{"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}), clone); - EXPECT_EQ(FieldValue::ObjectValue(std::map<std::string, FieldValue>{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}), + EXPECT_EQ(FieldValue::ObjectValueFromMap( + ObjectValue::Map{{"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}), object_value); clone = clone; - EXPECT_EQ(FieldValue::ObjectValue(std::map<std::string, FieldValue>{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}), + EXPECT_EQ(FieldValue::ObjectValueFromMap( + ObjectValue::Map{{"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}), clone); clone = null_value; EXPECT_EQ(FieldValue::NullValue(), clone); @@ -430,13 +430,12 @@ TEST(FieldValue, Move) { clone = FieldValue::NullValue(); EXPECT_EQ(FieldValue::NullValue(), clone); - FieldValue object_value = FieldValue::ObjectValue( - std::map<std::string, FieldValue>{{"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}); + FieldValue object_value = FieldValue::ObjectValueFromMap(ObjectValue::Map{ + {"true", FieldValue::TrueValue()}, {"false", FieldValue::FalseValue()}}); clone = std::move(object_value); - EXPECT_EQ(FieldValue::ObjectValue(std::map<std::string, FieldValue>{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}), + EXPECT_EQ(FieldValue::ObjectValueFromMap( + ObjectValue::Map{{"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}), clone); clone = FieldValue::NullValue(); EXPECT_EQ(FieldValue::NullValue(), clone); @@ -455,8 +454,7 @@ TEST(FieldValue, CompareMixedType) { const FieldValue geo_point_value = FieldValue::GeoPointValue({1, 2}); const FieldValue array_value = FieldValue::ArrayValue(std::vector<FieldValue>()); - const FieldValue object_value = - FieldValue::ObjectValue(std::map<std::string, FieldValue>()); + const FieldValue object_value = FieldValue::ObjectValueFromMap({}); EXPECT_TRUE(null_value < true_value); EXPECT_TRUE(true_value < number_value); EXPECT_TRUE(number_value < timestamp_value); diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc index e2456d5..862ac97 100644 --- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc @@ -194,7 +194,7 @@ TEST_F(SerializerTest, WritesStringModelToBytes) { } TEST_F(SerializerTest, WritesEmptyMapToBytes) { - FieldValue model = FieldValue::ObjectValue({}); + FieldValue model = FieldValue::ObjectValueFromMap({}); // TEXT_FORMAT_PROTO: 'map_value: {}' std::vector<uint8_t> bytes{0x32, 0x00}; ExpectRoundTrip(model, bytes, FieldValue::Type::Object); @@ -206,7 +206,7 @@ TEST_F(SerializerTest, WritesNestedObjectsToBytes) { // TODO(rsgowman): link libprotobuf to the test suite and eliminate the // above. - FieldValue model = FieldValue::ObjectValue( + FieldValue model = FieldValue::ObjectValueFromMap( {{"b", FieldValue::TrueValue()}, // TODO(rsgowman): add doubles (once they're supported) // {"d", FieldValue::DoubleValue(std::numeric_limits<double>::max())}, @@ -215,10 +215,10 @@ TEST_F(SerializerTest, WritesNestedObjectsToBytes) { {"s", FieldValue::StringValue("foo")}, // TODO(rsgowman): add arrays (once they're supported) // {"a", [2, "bar", {"b", false}]}, - {"o", FieldValue::ObjectValue( + {"o", FieldValue::ObjectValueFromMap( {{"d", FieldValue::IntegerValue(100)}, {"nested", - FieldValue::ObjectValue( + FieldValue::ObjectValueFromMap( {{"e", FieldValue::IntegerValue( std::numeric_limits<int64_t>::max())}})}})}}); |