From fe19fca0e521e3765e4ecf6a87b0a3d622a52b92 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Tue, 13 Feb 2018 17:46:09 -0500 Subject: Serialize and deserialize null (#783) * Build (grpc's) nanopb with -DPB_FIELD_16BIT We require (at least) 16 bit fields. (By default, nanopb uses 8 bit fields, ie allowing up to 256 field tags.) Also note that this patch adds this to grpc's nanopb, rather than to our nanopb. We'll need to eventually either: a) we instruct grpc to use our nanopb b) we rely on grpc's nanopb instead of using our own. (^ marked as a TODO for now.) * Add some dependant protos Imported from protobuf. Nanopb requires these to be present (though anything using libprotobuf does not, as these are already built into that.) * Add generated nanopb protos based off of newly added proto definitions * Build the nanopb protos * Serialize and deserialize null --- .../src/firebase/firestore/remote/CMakeLists.txt | 2 + .../src/firebase/firestore/remote/serializer.cc | 88 +++++++++++++++- .../src/firebase/firestore/remote/serializer.h | 112 ++++++++++++++++++++- .../firebase/firestore/remote/serializer_test.cc | 71 ++++++++++++- 4 files changed, 262 insertions(+), 11 deletions(-) (limited to 'Firestore/core') diff --git a/Firestore/core/src/firebase/firestore/remote/CMakeLists.txt b/Firestore/core/src/firebase/firestore/remote/CMakeLists.txt index a218e3b..7f528fb 100644 --- a/Firestore/core/src/firebase/firestore/remote/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/remote/CMakeLists.txt @@ -20,6 +20,8 @@ cc_library( serializer.h serializer.cc DEPENDS + firebase_firestore_model + firebase_firestore_protos_nanopb grpc::grpc nanopb ) diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index d3cdd3f..e503d09 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -16,16 +16,94 @@ #include "Firestore/core/src/firebase/firestore/remote/serializer.h" -// TODO(rsgowman): These are (currently!) unnecessary includes. Adding for now -// to ensure we can find nanopb's generated header files. -#include "Firestore/Protos/nanopb/google/firestore/v1beta1/document.pb.h" -#include "Firestore/Protos/nanopb/google/protobuf/timestamp.pb.h" +#include +#include namespace firebase { namespace firestore { namespace remote { -Serializer::Serializer() { +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; + default: + // TODO(rsgowman): implement the other types + abort(); + } + return proto_value; +} + +void Serializer::EncodeTypedValue(const TypedValue& value, + std::vector* out_bytes) { + bool status; + // 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 + // going to need. + uint8_t buf[1024]; + pb_ostream_t stream = pb_ostream_from_buffer(buf, sizeof(buf)); + switch (value.type) { + case FieldValue::Type::Null: + status = pb_encode_tag(&stream, PB_WT_VARINT, + google_firestore_v1beta1_Value_null_value_tag); + if (!status) { + // TODO(rsgowman): figure out error handling + abort(); + } + + status = pb_encode_varint(&stream, value.value.null_value); + if (!status) { + // TODO(rsgowman): figure out error handling + abort(); + } + + out_bytes->insert(out_bytes->end(), buf, buf + stream.bytes_written); + + break; + + default: + // TODO(rsgowman): implement the other types + abort(); + } +} + +FieldValue Serializer::DecodeFieldValue( + const Serializer::TypedValue& value_proto) { + switch (value_proto.type) { + case FieldValue::Type::Null: + return FieldValue::NullValue(); + default: + // TODO(rsgowman): implement the other types + abort(); + } +} + +Serializer::TypedValue Serializer::DecodeTypedValue(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; + bool eof; + bool status = pb_decode_tag(&stream, &wire_type, &tag, &eof); + if (!status || wire_type != PB_WT_VARINT) { + // TODO(rsgowman): figure out error handling + abort(); + } + + switch (tag) { + case google_firestore_v1beta1_Value_null_value_tag: + return Serializer::TypedValue{ + FieldValue::Type::Null, google_firestore_v1beta1_Value_init_default}; + default: + // TODO(rsgowman): figure out error handling + abort(); + } } } // namespace remote diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.h b/Firestore/core/src/firebase/firestore/remote/serializer.h index 4dc6b9e..518cff4 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.h +++ b/Firestore/core/src/firebase/firestore/remote/serializer.h @@ -17,16 +17,26 @@ #ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_REMOTE_SERIALIZER_H_ #define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_REMOTE_SERIALIZER_H_ +#include +#include +#include + +#include "Firestore/Protos/nanopb/google/firestore/v1beta1/document.pb.h" +#include "Firestore/core/src/firebase/firestore/model/field_value.h" +#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" + namespace firebase { namespace firestore { namespace remote { /** * @brief Converts internal model objects to their equivalent protocol buffer - * form. + * form, and protocol buffer objects to their equivalent bytes. * - * Methods starting with "Encode" convert to a protocol buffer and methods - * starting with "Decode" convert from a protocol buffer. + * 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. * */ // TODO(rsgowman): Original docs also has this: "Throws an exception if a @@ -34,10 +44,102 @@ namespace remote { // interpret." Adjust for C++. class Serializer { public: - // TODO(rsgowman): Adjust ctor to accept a DatabaseID... once that exists. - Serializer(/*DatabaseID databaseId*/); + /** + * @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 + // yet since it's not used yet (which travis complains about). So for now, + // we'll create a parameterless ctor (above) that likely won't exist in the + // final version of this class. + ///** + // * @param database_id Must remain valid for the lifetime of this Serializer + // * object. + // */ + // explicit Serializer(const firebase::firestore::model::DatabaseId& + // database_id) + // : database_id_(database_id) { + //} + + /** + * Converts the FieldValue model passed into the Value proto equivalent. + * + * @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* 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. + * + * @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. + */ + // TODO(rsgowman): error handling. + static TypedValue DecodeTypedValue(const uint8_t* bytes, size_t length); + + /** + * @brief Converts from bytes to the nanopb proto. + * + * @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. + */ + // TODO(rsgowman): error handling. + static TypedValue DecodeTypedValue(const std::vector& bytes) { + return DecodeTypedValue(bytes.data(), bytes.size()); + } + + private: + // TODO(rsgowman): We don't need the database_id_ yet (but will eventually). + // const firebase::firestore::model::DatabaseId& database_id_; }; +inline bool operator==(const Serializer::TypedValue& lhs, + const Serializer::TypedValue& rhs) { + if (lhs.type != rhs.type) { + return false; + } + + switch (lhs.type) { + case firebase::firestore::model::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; + default: + // TODO(rsgowman): implement the other types + abort(); + } +} + } // 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 1be5a87..35f417e 100644 --- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc @@ -16,8 +16,15 @@ #include "Firestore/core/src/firebase/firestore/remote/serializer.h" -#include +#include #include +#include + +#include "Firestore/core/src/firebase/firestore/model/field_value.h" +#include "gtest/gtest.h" + +using firebase::firestore::model::FieldValue; +using firebase::firestore::remote::Serializer; TEST(Serializer, CanLinkToNanopb) { // This test doesn't actually do anything interesting as far as actually using @@ -26,3 +33,65 @@ TEST(Serializer, CanLinkToNanopb) { // the test. pb_ostream_from_buffer(NULL, 0); } + +// Fixture for running serializer tests. +class SerializerTest : public ::testing::Test { + public: + SerializerTest() : serializer(/*DatabaseId("p", "d")*/) { + } + Serializer serializer; + + void ExpectRoundTrip(const FieldValue& model, + const Serializer::TypedValue& proto, + 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); + EXPECT_EQ(bytes, actual_bytes); + Serializer::TypedValue actual_proto = Serializer::DecodeTypedValue(bytes); + EXPECT_EQ(type, actual_proto.type); + EXPECT_EQ(proto, actual_proto); + } +}; + +TEST_F(SerializerTest, EncodesNullModelToProto) { + 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); + + /* NB: proto bytes were created via: + echo 'null_value: NULL_VALUE' \ + | ./build/external/protobuf/src/protobuf-build/src/protoc \ + -I./Firestore/Protos/protos \ + -I./build/external/protobuf/src/protobuf/src/ \ + --encode=google.firestore.v1beta1.Value \ + google/firestore/v1beta1/document.proto \ + > output.bin + */ + std::vector bytes{0x58, 0x00}; + ExpectRoundTrip(proto, bytes, FieldValue::Type::Null); +} + +// TODO(rsgowman): Test [en|de]coding multiple protos into the same output +// vector. -- cgit v1.2.3