From 7cddb97a607a773d8a5c70f4b73b4c132e1dc0e0 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Fri, 16 Feb 2018 10:07:38 -0500 Subject: Serialize (and deserialize) bool values (#791) --- .../src/firebase/firestore/remote/serializer.cc | 128 ++++++++++++++++++--- .../src/firebase/firestore/remote/serializer.h | 20 +--- .../firebase/firestore/remote/serializer_test.cc | 61 ++++++++-- 3 files changed, 165 insertions(+), 44 deletions(-) (limited to 'Firestore') diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index e503d09..c6c699f 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -23,6 +23,68 @@ namespace firebase { namespace firestore { namespace remote { +namespace { + +void EncodeUnsignedVarint(pb_ostream_t* stream, + uint32_t field_number, + uint64_t value) { + bool status = pb_encode_tag(stream, PB_WT_VARINT, field_number); + if (!status) { + // TODO(rsgowman): figure out error handling + abort(); + } + + status = pb_encode_varint(stream, value); + if (!status) { + // TODO(rsgowman): figure out error handling + abort(); + } +} + +uint64_t DecodeUnsignedVarint(pb_istream_t* stream) { + uint64_t varint_value; + bool status = pb_decode_varint(stream, &varint_value); + if (!status) { + // TODO(rsgowman): figure out error handling + abort(); + } + return varint_value; +} + +void EncodeNull(pb_ostream_t* stream) { + return EncodeUnsignedVarint(stream, + google_firestore_v1beta1_Value_null_value_tag, + google_protobuf_NullValue_NULL_VALUE); +} + +void DecodeNull(pb_istream_t* stream) { + uint64_t varint = DecodeUnsignedVarint(stream); + if (varint != google_protobuf_NullValue_NULL_VALUE) { + // TODO(rsgowman): figure out error handling + abort(); + } +} + +void EncodeBool(pb_ostream_t* stream, bool bool_value) { + return EncodeUnsignedVarint( + stream, google_firestore_v1beta1_Value_boolean_value_tag, bool_value); +} + +bool DecodeBool(pb_istream_t* stream) { + uint64_t varint = DecodeUnsignedVarint(stream); + switch (varint) { + case 0: + return false; + case 1: + return true; + default: + // TODO(rsgowman): figure out error handling + abort(); + } +} + +} // namespace + using firebase::firestore::model::FieldValue; Serializer::TypedValue Serializer::EncodeFieldValue( @@ -33,6 +95,14 @@ Serializer::TypedValue Serializer::EncodeFieldValue( 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; default: // TODO(rsgowman): implement the other types abort(); @@ -42,7 +112,6 @@ Serializer::TypedValue Serializer::EncodeFieldValue( 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. @@ -50,27 +119,19 @@ void Serializer::EncodeTypedValue(const TypedValue& value, 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); + EncodeNull(&stream); + break; + case FieldValue::Type::Boolean: + EncodeBool(&stream, value.value.boolean_value); break; default: // TODO(rsgowman): implement the other types abort(); } + + out_bytes->insert(out_bytes->end(), buf, buf + stream.bytes_written); } FieldValue Serializer::DecodeFieldValue( @@ -78,6 +139,8 @@ FieldValue Serializer::DecodeFieldValue( switch (value_proto.type) { case FieldValue::Type::Null: return FieldValue::NullValue(); + case FieldValue::Type::Boolean: + return FieldValue::BooleanValue(value_proto.value.boolean_value); default: // TODO(rsgowman): implement the other types abort(); @@ -96,14 +159,45 @@ 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: - return Serializer::TypedValue{ - FieldValue::Type::Null, google_firestore_v1beta1_Value_init_default}; + result.type = FieldValue::Type::Null; + DecodeNull(&stream); + break; + case google_firestore_v1beta1_Value_boolean_value_tag: + result.type = FieldValue::Type::Boolean; + result.value.boolean_value = DecodeBool(&stream); + break; + 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; + 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 518cff4..af65255 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.h +++ b/Firestore/core/src/firebase/firestore/remote/serializer.h @@ -121,24 +121,8 @@ class Serializer { // 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(); - } -} +bool operator==(const Serializer::TypedValue& lhs, + const Serializer::TypedValue& rhs); } // namespace remote } // namespace firestore diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc index 35f417e..79aba85 100644 --- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc @@ -14,6 +14,26 @@ * limitations under the License. */ +/* NB: proto bytes were created via: + echo 'TEXT_FORMAT_PROTO' \ + | ./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 \ + | hexdump -C + * where TEXT_FORMAT_PROTO is the text format of the protobuf. (go/textformat). + * + * Examples: + * - For null, TEXT_FORMAT_PROTO would be 'null_value: NULL_VALUE' and would + * yield the bytes: { 0x58, 0x00 }. + * - For true, TEXT_FORMAT_PROTO would be 'boolean_value: true' and would yield + * the bytes { 0x08, 0x01 }. + * + * All uses are documented below, so search for TEXT_FORMAT_PROTO to find more + * examples. + */ + #include "Firestore/core/src/firebase/firestore/remote/serializer.h" #include @@ -80,18 +100,41 @@ TEST_F(SerializerTest, EncodesNullProtoToBytes) { // 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 - */ + // TEXT_FORMAT_PROTO: 'null_value: NULL_VALUE' std::vector bytes{0x58, 0x00}; ExpectRoundTrip(proto, 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) { + struct TestCase { + bool value; + std::vector bytes; + }; + + std::vector cases{// TEXT_FORMAT_PROTO: 'boolean_value: true' + {true, {0x08, 0x01}}, + // TEXT_FORMAT_PROTO: 'boolean_value: false' + {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); + } +} + // TODO(rsgowman): Test [en|de]coding multiple protos into the same output // vector. + +// TODO(rsgowman): Death test for decoding invalid bytes. -- cgit v1.2.3