From 2ae36f1e9671b40723dd06462b4a416e4baa5a57 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Thu, 8 Mar 2018 11:53:37 -0500 Subject: [De]Serialize FieldValue map_values ("Objects") (#878) These can (recursively) contain other FieldValues. --- .../firebase/firestore/model/field_value_test.cc | 63 ++++++++--------- .../firebase/firestore/remote/serializer_test.cc | 82 ++++++++++++++++++++++ 2 files changed, 110 insertions(+), 35 deletions(-) (limited to 'Firestore/core/test') 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 5a64d59..93879f9 100644 --- a/Firestore/core/test/firebase/firestore/model/field_value_test.cc +++ b/Firestore/core/test/firebase/firestore/model/field_value_test.cc @@ -190,14 +190,13 @@ TEST(FieldValue, ArrayType) { TEST(FieldValue, ObjectType) { const FieldValue empty = - FieldValue::ObjectValue(std::map{}); - std::map object{ - {"null", FieldValue::NullValue()}, - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}; + FieldValue::ObjectValue(std::map{}); + std::map object{{"null", FieldValue::NullValue()}, + {"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}; // copy the map const FieldValue small = FieldValue::ObjectValue(object); - std::map another_object{ + std::map another_object{ {"null", FieldValue::NullValue()}, {"true", FieldValue::FalseValue()}}; // move the array const FieldValue large = FieldValue::ObjectValue(std::move(another_object)); @@ -336,27 +335,23 @@ TEST(FieldValue, Copy) { clone = null_value; EXPECT_EQ(FieldValue::NullValue(), clone); - const FieldValue object_value = - FieldValue::ObjectValue(std::map{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}); + const FieldValue object_value = FieldValue::ObjectValue( + std::map{{"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}); clone = object_value; - EXPECT_EQ( - FieldValue::ObjectValue(std::map{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}), - clone); - EXPECT_EQ( - FieldValue::ObjectValue(std::map{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}), - object_value); + EXPECT_EQ(FieldValue::ObjectValue(std::map{ + {"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}), + clone); + EXPECT_EQ(FieldValue::ObjectValue(std::map{ + {"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}), + object_value); clone = clone; - EXPECT_EQ( - FieldValue::ObjectValue(std::map{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}), - clone); + EXPECT_EQ(FieldValue::ObjectValue(std::map{ + {"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}), + clone); clone = null_value; EXPECT_EQ(FieldValue::NullValue(), clone); } @@ -435,16 +430,14 @@ TEST(FieldValue, Move) { clone = FieldValue::NullValue(); EXPECT_EQ(FieldValue::NullValue(), clone); - FieldValue object_value = - FieldValue::ObjectValue(std::map{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}); + FieldValue object_value = FieldValue::ObjectValue( + std::map{{"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}); clone = std::move(object_value); - EXPECT_EQ( - FieldValue::ObjectValue(std::map{ - {"true", FieldValue::TrueValue()}, - {"false", FieldValue::FalseValue()}}), - clone); + EXPECT_EQ(FieldValue::ObjectValue(std::map{ + {"true", FieldValue::TrueValue()}, + {"false", FieldValue::FalseValue()}}), + clone); clone = FieldValue::NullValue(); EXPECT_EQ(FieldValue::NullValue(), clone); } @@ -463,7 +456,7 @@ TEST(FieldValue, CompareMixedType) { const FieldValue array_value = FieldValue::ArrayValue(std::vector()); const FieldValue object_value = - FieldValue::ObjectValue(std::map()); + FieldValue::ObjectValue(std::map()); 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 8f89064..1e2f928 100644 --- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc @@ -116,6 +116,8 @@ TEST_F(SerializerTest, EncodesIntegersModelToBytes) { // For now, lets at least verify these values: EXPECT_EQ(-9223372036854775807 - 1, std::numeric_limits::min()); EXPECT_EQ(9223372036854775807, std::numeric_limits::max()); + // TODO(rsgowman): link libprotobuf to the test suite and eliminate the + // above. struct TestCase { int64_t value; @@ -191,6 +193,86 @@ TEST_F(SerializerTest, EncodesStringModelToBytes) { } } +TEST_F(SerializerTest, EncodesEmptyMapToBytes) { + FieldValue model = FieldValue::ObjectValue({}); + // TEXT_FORMAT_PROTO: 'map_value: {}' + std::vector bytes{0x32, 0x00}; + ExpectRoundTrip(model, bytes, FieldValue::Type::Object); +} + +TEST_F(SerializerTest, EncodesNestedObjectsToBytes) { + // As above, verify max int64_t value. + EXPECT_EQ(9223372036854775807, std::numeric_limits::max()); + // TODO(rsgowman): link libprotobuf to the test suite and eliminate the + // above. + + FieldValue model = FieldValue::ObjectValue( + {{"b", FieldValue::TrueValue()}, + // TODO(rsgowman): add doubles (once they're supported) + // {"d", FieldValue::DoubleValue(std::numeric_limits::max())}, + {"i", FieldValue::IntegerValue(1)}, + {"n", FieldValue::NullValue()}, + {"s", FieldValue::StringValue("foo")}, + // TODO(rsgowman): add arrays (once they're supported) + // {"a", [2, "bar", {"b", false}]}, + {"o", FieldValue::ObjectValue( + {{"d", FieldValue::IntegerValue(100)}, + {"nested", + FieldValue::ObjectValue( + {{"e", FieldValue::IntegerValue( + std::numeric_limits::max())}})}})}}); + + /* WARNING: "Wire format ordering and map iteration ordering of map values is + * undefined, so you cannot rely on your map items being in a particular + * order." + * - https://developers.google.com/protocol-buffers/docs/proto#maps-features + * + * In reality, the map items are serialized by protoc in whatever order you + * provide them in. Since FieldValue::ObjectValue is currently backed by a + * std::map (and not an unordered_map) this implies ~alpha ordering. So we + * need to provide the text format input in alpha ordering for things to match + * up. + * + * This is... not ideal. Nothing stops libprotobuf from changing this + * behaviour (since it's not guaranteed) nor does anything stop us from + * switching map->unordered_map in FieldValue. (Arguably, that would be + * better.) But the alternative is to not test the serializing to bytes, and + * instead just assume we got that right. A *better* solution is to serialize + * to bytes, and then deserialize with libprotobuf (rather than nanopb) and + * then do a second test of serializing via libprotobuf and deserializing via + * nanopb. In both cases, we would ignore the bytes themselves (since the + * ordering is not defined) and instead compare the input objects with the + * output objects. + * + * TODO(rsgowman): ^ + * + * TEXT_FORMAT_PROTO (with multi-line formatting to preserve sanity): + 'map_value: { + fields: {key:"b", value:{boolean_value: true}} + fields: {key:"i", value:{integer_value: 1}} + fields: {key:"n", value:{null_value: NULL_VALUE}} + fields: {key:"o", value:{map_value: { + fields: {key:"d", value:{integer_value: 100}} + fields: {key:"nested", value{map_value: { + fields: {key:"e", value:{integer_value: 9223372036854775807}} + }}} + }}} + fields: {key:"s", value:{string_value: "foo"}} + }' + */ + std::vector bytes{ + 0x32, 0x59, 0x0a, 0x07, 0x0a, 0x01, 0x62, 0x12, 0x02, 0x08, 0x01, 0x0a, + 0x07, 0x0a, 0x01, 0x69, 0x12, 0x02, 0x10, 0x01, 0x0a, 0x07, 0x0a, 0x01, + 0x6e, 0x12, 0x02, 0x58, 0x00, 0x0a, 0x2f, 0x0a, 0x01, 0x6f, 0x12, 0x2a, + 0x32, 0x28, 0x0a, 0x07, 0x0a, 0x01, 0x64, 0x12, 0x02, 0x10, 0x64, 0x0a, + 0x1d, 0x0a, 0x06, 0x6e, 0x65, 0x73, 0x74, 0x65, 0x64, 0x12, 0x13, 0x32, + 0x11, 0x0a, 0x0f, 0x0a, 0x01, 0x65, 0x12, 0x0a, 0x10, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f, 0x0a, 0x0b, 0x0a, 0x01, 0x73, 0x12, + 0x06, 0x8a, 0x01, 0x03, 0x66, 0x6f, 0x6f}; + + ExpectRoundTrip(model, bytes, FieldValue::Type::Object); +} + // TODO(rsgowman): Test [en|de]coding multiple protos into the same output // vector. -- cgit v1.2.3