From 13aeb61de4fac4c0239bcf44a98a7d3aa9203963 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Tue, 27 Feb 2018 13:50:50 -0500 Subject: Eliminate TypedValue and serialize direct from FieldValue to bytes. (#860) This will likely only apply for proto messages that use 'oneof's. (Because we need to serialize these manually.) The problem was that as we were adding additional types, TypeValue was evolving into a parallel implementation of the FieldValue union. When serializing/deserializing oneofs we need to supply a custom value to serialize and a custom function to do the work. There's no value in translating from FieldValue to TypeValue just to store as this custom object. We might as well just store the FieldValue model directly and write the custom serializer/deserializer to use the model directly. --- .../firebase/firestore/remote/serializer_test.cc | 80 ++++------------------ 1 file changed, 13 insertions(+), 67 deletions(-) (limited to 'Firestore/core/test') diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc index 7607911..9ce60f5 100644 --- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc @@ -63,60 +63,27 @@ class SerializerTest : public ::testing::Test { Serializer serializer; void ExpectRoundTrip(const FieldValue& model, - const Serializer::TypedValue& proto, + const std::vector& bytes, FieldValue::Type type) { EXPECT_EQ(type, model.type()); - EXPECT_EQ(type, proto.type); - Serializer::TypedValue actual_proto = serializer.EncodeFieldValue(model); - EXPECT_EQ(type, actual_proto.type); - EXPECT_EQ(proto, actual_proto); - EXPECT_EQ(model, serializer.DecodeFieldValue(proto)); - } - - void ExpectRoundTrip(const Serializer::TypedValue& proto, - std::vector bytes, - FieldValue::Type type) { - EXPECT_EQ(type, proto.type); std::vector actual_bytes; - Serializer::EncodeTypedValue(proto, &actual_bytes); + serializer.EncodeFieldValue(model, &actual_bytes); EXPECT_EQ(bytes, actual_bytes); - Serializer::TypedValue actual_proto = Serializer::DecodeTypedValue(bytes); - EXPECT_EQ(type, actual_proto.type); - EXPECT_EQ(proto, actual_proto); + FieldValue actual_model = serializer.DecodeFieldValue(bytes); + EXPECT_EQ(type, actual_model.type()); + EXPECT_EQ(model, actual_model); } }; -TEST_F(SerializerTest, EncodesNullModelToProto) { +TEST_F(SerializerTest, EncodesNullModelToBytes) { FieldValue model = FieldValue::NullValue(); - Serializer::TypedValue proto{FieldValue::Type::Null, - google_firestore_v1beta1_Value_init_default}; - // sanity check (the _init_default above should set this to _NULL_VALUE) - EXPECT_EQ(google_protobuf_NullValue_NULL_VALUE, proto.value.null_value); - ExpectRoundTrip(model, proto, FieldValue::Type::Null); -} - -TEST_F(SerializerTest, EncodesNullProtoToBytes) { - Serializer::TypedValue proto{FieldValue::Type::Null, - google_firestore_v1beta1_Value_init_default}; - // sanity check (the _init_default above should set this to _NULL_VALUE) - EXPECT_EQ(google_protobuf_NullValue_NULL_VALUE, proto.value.null_value); // TEXT_FORMAT_PROTO: 'null_value: NULL_VALUE' std::vector bytes{0x58, 0x00}; - ExpectRoundTrip(proto, bytes, FieldValue::Type::Null); + ExpectRoundTrip(model, bytes, FieldValue::Type::Null); } -TEST_F(SerializerTest, EncodesBoolModelToProto) { - for (bool test : {true, false}) { - FieldValue model = FieldValue::BooleanValue(test); - Serializer::TypedValue proto{FieldValue::Type::Boolean, - google_firestore_v1beta1_Value_init_default}; - proto.value.boolean_value = test; - ExpectRoundTrip(model, proto, FieldValue::Type::Boolean); - } -} - -TEST_F(SerializerTest, EncodesBoolProtoToBytes) { +TEST_F(SerializerTest, EncodesBoolModelToBytes) { struct TestCase { bool value; std::vector bytes; @@ -128,31 +95,12 @@ TEST_F(SerializerTest, EncodesBoolProtoToBytes) { {false, {0x08, 0x00}}}; for (const TestCase& test : cases) { - Serializer::TypedValue proto{FieldValue::Type::Boolean, - google_firestore_v1beta1_Value_init_default}; - proto.value.boolean_value = test.value; - ExpectRoundTrip(proto, test.bytes, FieldValue::Type::Boolean); - } -} - -TEST_F(SerializerTest, EncodesIntegersModelToProto) { - std::vector testCases{0, - 1, - -1, - 100, - -100, - std::numeric_limits::min(), - std::numeric_limits::max()}; - for (int64_t test : testCases) { - FieldValue model = FieldValue::IntegerValue(test); - Serializer::TypedValue proto{FieldValue::Type::Integer, - google_firestore_v1beta1_Value_init_default}; - proto.value.integer_value = test; - ExpectRoundTrip(model, proto, FieldValue::Type::Integer); + FieldValue model = FieldValue::BooleanValue(test.value); + ExpectRoundTrip(model, test.bytes, FieldValue::Type::Boolean); } } -TEST_F(SerializerTest, EncodesIntegersProtoToBytes) { +TEST_F(SerializerTest, EncodesIntegersModelToBytes) { // NB: because we're calculating the bytes via protoc (instead of // libprotobuf) we have to look at values from numeric_limits ahead of // time. So these test cases make the following assumptions about @@ -198,10 +146,8 @@ TEST_F(SerializerTest, EncodesIntegersProtoToBytes) { {0x10, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}}}; for (const TestCase& test : cases) { - Serializer::TypedValue proto{FieldValue::Type::Integer, - google_firestore_v1beta1_Value_init_default}; - proto.value.integer_value = test.value; - ExpectRoundTrip(proto, test.bytes, FieldValue::Type::Integer); + FieldValue model = FieldValue::IntegerValue(test.value); + ExpectRoundTrip(model, test.bytes, FieldValue::Type::Integer); } } -- cgit v1.2.3