diff options
3 files changed, 44 insertions, 191 deletions
diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index 474d890..02013fa 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -102,33 +102,7 @@ int64_t DecodeInteger(pb_istream_t* stream) { using firebase::firestore::model::FieldValue; -Serializer::TypedValue Serializer::EncodeFieldValue( - const FieldValue& field_value) { - Serializer::TypedValue proto_value{ - field_value.type(), google_firestore_v1beta1_Value_init_default}; - switch (field_value.type()) { - case FieldValue::Type::Null: - proto_value.value.null_value = google_protobuf_NullValue_NULL_VALUE; - break; - case FieldValue::Type::Boolean: - if (field_value == FieldValue::TrueValue()) { - proto_value.value.boolean_value = true; - } else { - FIREBASE_DEV_ASSERT(field_value == FieldValue::FalseValue()); - proto_value.value.boolean_value = false; - } - break; - case FieldValue::Type::Integer: - proto_value.value.integer_value = field_value.integer_value(); - break; - default: - // TODO(rsgowman): implement the other types - abort(); - } - return proto_value; -} - -void Serializer::EncodeTypedValue(const TypedValue& value, +void Serializer::EncodeFieldValue(const FieldValue& field_value, std::vector<uint8_t>* out_bytes) { // TODO(rsgowman): how large should the output buffer be? Do some // investigation to see if we can get nanopb to tell us how much space it's @@ -139,7 +113,7 @@ void Serializer::EncodeTypedValue(const TypedValue& value, // TODO(rsgowman): some refactoring is in order... but will wait until after a // non-varint, non-fixed-size (i.e. string) type is present before doing so. bool status = false; - switch (value.type) { + switch (field_value.type()) { case FieldValue::Type::Null: status = pb_encode_tag(&stream, PB_WT_VARINT, google_firestore_v1beta1_Value_null_value_tag); @@ -157,7 +131,12 @@ void Serializer::EncodeTypedValue(const TypedValue& value, // TODO(rsgowman): figure out error handling abort(); } - EncodeBool(&stream, value.value.boolean_value); + if (field_value == FieldValue::TrueValue()) { + EncodeBool(&stream, true); + } else { + FIREBASE_DEV_ASSERT(field_value == FieldValue::FalseValue()); + EncodeBool(&stream, false); + } break; case FieldValue::Type::Integer: @@ -167,7 +146,7 @@ void Serializer::EncodeTypedValue(const TypedValue& value, // TODO(rsgowman): figure out error handling abort(); } - EncodeInteger(&stream, value.value.integer_value); + EncodeInteger(&stream, field_value.integer_value()); break; default: @@ -178,23 +157,7 @@ void Serializer::EncodeTypedValue(const TypedValue& value, out_bytes->insert(out_bytes->end(), buf, buf + stream.bytes_written); } -FieldValue Serializer::DecodeFieldValue( - const Serializer::TypedValue& value_proto) { - switch (value_proto.type) { - case FieldValue::Type::Null: - return FieldValue::NullValue(); - case FieldValue::Type::Boolean: - return FieldValue::BooleanValue(value_proto.value.boolean_value); - case FieldValue::Type::Integer: - return FieldValue::IntegerValue(value_proto.value.integer_value); - default: - // TODO(rsgowman): implement the other types - abort(); - } -} - -Serializer::TypedValue Serializer::DecodeTypedValue(const uint8_t* bytes, - size_t length) { +FieldValue Serializer::DecodeFieldValue(const uint8_t* bytes, size_t length) { pb_istream_t stream = pb_istream_from_buffer(bytes, length); pb_wire_type_t wire_type; uint32_t tag; @@ -205,51 +168,19 @@ Serializer::TypedValue Serializer::DecodeTypedValue(const uint8_t* bytes, abort(); } - Serializer::TypedValue result{FieldValue::Type::Null, - google_firestore_v1beta1_Value_init_default}; switch (tag) { case google_firestore_v1beta1_Value_null_value_tag: - result.type = FieldValue::Type::Null; DecodeNull(&stream); - break; + return FieldValue::NullValue(); case google_firestore_v1beta1_Value_boolean_value_tag: - result.type = FieldValue::Type::Boolean; - result.value.boolean_value = DecodeBool(&stream); - break; + return FieldValue::BooleanValue(DecodeBool(&stream)); case google_firestore_v1beta1_Value_integer_value_tag: - result.type = FieldValue::Type::Integer; - result.value.integer_value = DecodeInteger(&stream); - break; + return FieldValue::IntegerValue(DecodeInteger(&stream)); default: // TODO(rsgowman): figure out error handling abort(); } - - return result; -} - -bool operator==(const Serializer::TypedValue& lhs, - const Serializer::TypedValue& rhs) { - if (lhs.type != rhs.type) { - return false; - } - - switch (lhs.type) { - case FieldValue::Type::Null: - FIREBASE_DEV_ASSERT(lhs.value.null_value == - google_protobuf_NullValue_NULL_VALUE); - FIREBASE_DEV_ASSERT(rhs.value.null_value == - google_protobuf_NullValue_NULL_VALUE); - return true; - case FieldValue::Type::Boolean: - return lhs.value.boolean_value == rhs.value.boolean_value; - case FieldValue::Type::Integer: - return lhs.value.integer_value == rhs.value.integer_value; - default: - // TODO(rsgowman): implement the other types - abort(); - } } } // namespace remote diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.h b/Firestore/core/src/firebase/firestore/remote/serializer.h index af65255..635ffc3 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.h +++ b/Firestore/core/src/firebase/firestore/remote/serializer.h @@ -33,25 +33,17 @@ namespace remote { * @brief Converts internal model objects to their equivalent protocol buffer * form, and protocol buffer objects to their equivalent bytes. * - * Methods starting with "Encode" either convert from a model object to a - * protocol buffer or from a protocol buffer to bytes, and methods starting with - * "Decode" either convert from a protocol buffer to a model object or from - * bytes to a protocol buffer. - * + * Methods starting with "Encode" convert from a model object to a protocol + * buffer (or directly to bytes in cases where the proto uses a 'oneof', due to + * limitations in nanopb), and methods starting with "Decode" convert from a + * protocol buffer to a model object (or from bytes directly to a model + * objects.) */ // TODO(rsgowman): Original docs also has this: "Throws an exception if a // protocol buffer is missing a critical field or has a value we can't // interpret." Adjust for C++. class Serializer { public: - /** - * @brief Wraps (nanopb) google_firestore_v1beta1_Value with type information. - */ - struct TypedValue { - firebase::firestore::model::FieldValue::Type type; - google_firestore_v1beta1_Value value; - }; - Serializer() { } // TODO(rsgowman): We eventually need the DatabaseId, but can't add it just @@ -68,52 +60,39 @@ class Serializer { //} /** - * Converts the FieldValue model passed into the Value proto equivalent. + * Converts the FieldValue model passed into bytes. * * @param field_value the model to convert. - * @return the proto representation of the model. - */ - static Serializer::TypedValue EncodeFieldValue( - const firebase::firestore::model::FieldValue& field_value); - - /** - * @brief Converts the value proto passed into bytes. - * * @param[out] out_bytes A buffer to place the output. The bytes will be * appended to this vector. */ // TODO(rsgowman): error handling, incl return code. - static void EncodeTypedValue(const TypedValue& value, - std::vector<uint8_t>* out_bytes); + static void EncodeFieldValue( + const firebase::firestore::model::FieldValue& field_value, + std::vector<uint8_t>* out_bytes); /** - * Converts from the proto Value format to the model FieldValue format - * - * @return The model equivalent of the proto data. - */ - static firebase::firestore::model::FieldValue DecodeFieldValue( - const Serializer::TypedValue& value_proto); - - /** - * @brief Converts from bytes to the nanopb proto. + * @brief Converts from bytes to the model FieldValue format. * * @param bytes The bytes to convert. It's assumed that exactly all of the * bytes will be used by this conversion. - * @return The (nanopb) proto equivalent of the bytes. + * @return The model equivalent of the bytes. */ // TODO(rsgowman): error handling. - static TypedValue DecodeTypedValue(const uint8_t* bytes, size_t length); + static firebase::firestore::model::FieldValue DecodeFieldValue( + const uint8_t* bytes, size_t length); /** - * @brief Converts from bytes to the nanopb proto. + * @brief Converts from bytes to the model FieldValue format. * * @param bytes The bytes to convert. It's assumed that exactly all of the * bytes will be used by this conversion. - * @return The (nanopb) proto equivalent of the bytes. + * @return The model equivalent of the bytes. */ // TODO(rsgowman): error handling. - static TypedValue DecodeTypedValue(const std::vector<uint8_t>& bytes) { - return DecodeTypedValue(bytes.data(), bytes.size()); + static firebase::firestore::model::FieldValue DecodeFieldValue( + const std::vector<uint8_t>& bytes) { + return DecodeFieldValue(bytes.data(), bytes.size()); } private: @@ -121,9 +100,6 @@ class Serializer { // const firebase::firestore::model::DatabaseId& database_id_; }; -bool operator==(const Serializer::TypedValue& lhs, - const Serializer::TypedValue& rhs); - } // namespace remote } // namespace firestore } // namespace firebase 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<uint8_t>& 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<uint8_t> bytes, - FieldValue::Type type) { - EXPECT_EQ(type, proto.type); std::vector<uint8_t> 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<uint8_t> 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<uint8_t> 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<int64_t> testCases{0, - 1, - -1, - 100, - -100, - std::numeric_limits<int64_t>::min(), - std::numeric_limits<int64_t>::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<T> 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); } } |