From 2e4855911c436b608eb6e1372ac26ca7165eec4e Mon Sep 17 00:00:00 2001 From: rsgowman Date: Fri, 22 Jun 2018 09:07:23 -0400 Subject: Refactored a few methods from anon namespace to remote serializer (#1435) --- .../src/firebase/firestore/remote/serializer.cc | 232 ++++++++++----------- .../src/firebase/firestore/remote/serializer.h | 11 + 2 files changed, 116 insertions(+), 127 deletions(-) (limited to 'Firestore/core') diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index fc8b312..044b3db 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -62,13 +62,6 @@ using firebase::firestore::util::StatusOr; namespace { -void EncodeMapValue(Writer* writer, const ObjectValue& object_value); -void EncodeObjectMapImpl(Writer* writer, - const ObjectValue::Map& object_value_map, - uint32_t map_tag, - uint32_t key_tag, - uint32_t value_tag); - ObjectValue::Map DecodeMapValue(Reader* reader); void EncodeTimestamp(Writer* writer, const Timestamp& timestamp_value) { @@ -110,60 +103,6 @@ Timestamp DecodeTimestamp(Reader* reader) { return Timestamp{timestamp_proto.seconds, timestamp_proto.nanos}; } -// Named '..Impl' so as to not conflict with Serializer::EncodeFieldValue. -// TODO(rsgowman): Refactor to use a helper class that wraps the stream struct. -// This will help with error handling, and should eliminate the issue of two -// 'EncodeFieldValue' methods. -void EncodeFieldValueImpl(Writer* writer, const FieldValue& field_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. - switch (field_value.type()) { - case FieldValue::Type::Null: - writer->WriteTag( - {PB_WT_VARINT, google_firestore_v1beta1_Value_null_value_tag}); - writer->WriteNull(); - break; - - case FieldValue::Type::Boolean: - writer->WriteTag( - {PB_WT_VARINT, google_firestore_v1beta1_Value_boolean_value_tag}); - writer->WriteBool(field_value.boolean_value()); - break; - - case FieldValue::Type::Integer: - writer->WriteTag( - {PB_WT_VARINT, google_firestore_v1beta1_Value_integer_value_tag}); - writer->WriteInteger(field_value.integer_value()); - break; - - case FieldValue::Type::String: - writer->WriteTag( - {PB_WT_STRING, google_firestore_v1beta1_Value_string_value_tag}); - writer->WriteString(field_value.string_value()); - break; - - case FieldValue::Type::Timestamp: - writer->WriteTag( - {PB_WT_STRING, google_firestore_v1beta1_Value_timestamp_value_tag}); - writer->WriteNestedMessage([&field_value](Writer* writer) { - EncodeTimestamp(writer, field_value.timestamp_value()); - }); - break; - - case FieldValue::Type::Object: - writer->WriteTag( - {PB_WT_STRING, google_firestore_v1beta1_Value_map_value_tag}); - writer->WriteNestedMessage([&field_value](Writer* writer) { - EncodeMapValue(writer, field_value.object_value()); - }); - break; - - default: - // TODO(rsgowman): implement the other types - abort(); - } -} - FieldValue DecodeFieldValueImpl(Reader* reader) { if (!reader->status().ok()) return FieldValue::NullValue(); @@ -265,40 +204,6 @@ FieldValue DecodeFieldValueImpl(Reader* reader) { return result; } -/** - * Encodes a 'FieldsEntry' object, within a FieldValue's map_value type. - * - * In protobuf, maps are implemented as a repeated set of key/values. For - * instance, this: - * message Foo { - * map fields = 1; - * } - * would be written (in proto text format) as: - * { - * fields: {key:"key string 1", value:{}} - * fields: {key:"key string 2", value:{}} - * ... - * } - * - * This method writes an individual entry from that list. It is expected that - * this method will be called once for each entry in the map. - * - * @param kv The individual key/value pair to write. - */ -void EncodeFieldsEntry(Writer* writer, - const ObjectValue::Map::value_type& kv, - uint32_t key_tag, - uint32_t value_tag) { - // Write the key (string) - writer->WriteTag({PB_WT_STRING, key_tag}); - writer->WriteString(kv.first); - - // Write the value (FieldValue) - writer->WriteTag({PB_WT_STRING, value_tag}); - writer->WriteNestedMessage( - [&kv](Writer* writer) { EncodeFieldValueImpl(writer, kv.second); }); -} - ObjectValue::Map::value_type DecodeFieldsEntry(Reader* reader, uint32_t key_tag, uint32_t value_tag) { @@ -336,27 +241,6 @@ ObjectValue::Map::value_type DecodeDocumentFieldsEntry(Reader* reader) { google_firestore_v1beta1_Document_FieldsEntry_value_tag); } -void EncodeObjectMapImpl(Writer* writer, - const ObjectValue::Map& object_value_map, - uint32_t map_tag, - uint32_t key_tag, - uint32_t value_tag) { - // Write each FieldsEntry (i.e. key-value pair.) - for (const auto& kv : object_value_map) { - writer->WriteTag({PB_WT_STRING, map_tag}); - writer->WriteNestedMessage([&kv, &key_tag, &value_tag](Writer* writer) { - return EncodeFieldsEntry(writer, kv, key_tag, value_tag); - }); - } -} - -void EncodeMapValue(Writer* writer, const ObjectValue& object_value) { - EncodeObjectMapImpl(writer, object_value.internal_value, - google_firestore_v1beta1_MapValue_fields_tag, - google_firestore_v1beta1_MapValue_FieldsEntry_key_tag, - google_firestore_v1beta1_MapValue_FieldsEntry_value_tag); -} - ObjectValue::Map DecodeMapValue(Reader* reader) { ObjectValue::Map result; if (!reader->status().ok()) return result; @@ -454,10 +338,61 @@ ResourcePath ExtractLocalPathFromResourceName( Status Serializer::EncodeFieldValue(const FieldValue& field_value, std::vector* out_bytes) { Writer writer = Writer::Wrap(out_bytes); - EncodeFieldValueImpl(&writer, field_value); + EncodeFieldValue(&writer, field_value); return writer.status(); } +void Serializer::EncodeFieldValue(Writer* writer, + const FieldValue& field_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. + switch (field_value.type()) { + case FieldValue::Type::Null: + writer->WriteTag( + {PB_WT_VARINT, google_firestore_v1beta1_Value_null_value_tag}); + writer->WriteNull(); + break; + + case FieldValue::Type::Boolean: + writer->WriteTag( + {PB_WT_VARINT, google_firestore_v1beta1_Value_boolean_value_tag}); + writer->WriteBool(field_value.boolean_value()); + break; + + case FieldValue::Type::Integer: + writer->WriteTag( + {PB_WT_VARINT, google_firestore_v1beta1_Value_integer_value_tag}); + writer->WriteInteger(field_value.integer_value()); + break; + + case FieldValue::Type::String: + writer->WriteTag( + {PB_WT_STRING, google_firestore_v1beta1_Value_string_value_tag}); + writer->WriteString(field_value.string_value()); + break; + + case FieldValue::Type::Timestamp: + writer->WriteTag( + {PB_WT_STRING, google_firestore_v1beta1_Value_timestamp_value_tag}); + writer->WriteNestedMessage([&field_value](Writer* writer) { + EncodeTimestamp(writer, field_value.timestamp_value()); + }); + break; + + case FieldValue::Type::Object: + writer->WriteTag( + {PB_WT_STRING, google_firestore_v1beta1_Value_map_value_tag}); + writer->WriteNestedMessage([&field_value](Writer* writer) { + EncodeMapValue(writer, field_value.object_value()); + }); + break; + + default: + // TODO(rsgowman): implement the other types + abort(); + } +} + StatusOr Serializer::DecodeFieldValue(const uint8_t* bytes, size_t length) { Reader reader = Reader::Wrap(bytes, length); @@ -499,11 +434,10 @@ void Serializer::EncodeDocument(Writer* writer, // Encode Document.fields (unless it's empty) if (!object_value.internal_value.empty()) { - EncodeObjectMapImpl( - writer, object_value.internal_value, - google_firestore_v1beta1_Document_fields_tag, - google_firestore_v1beta1_Document_FieldsEntry_key_tag, - google_firestore_v1beta1_Document_FieldsEntry_value_tag); + EncodeObjectMap(writer, object_value.internal_value, + google_firestore_v1beta1_Document_fields_tag, + google_firestore_v1beta1_Document_FieldsEntry_key_tag, + google_firestore_v1beta1_Document_FieldsEntry_value_tag); } // Skip Document.create_time and Document.update_time, since they're @@ -668,17 +602,27 @@ std::unique_ptr Serializer::DecodeDocument(Reader* reader) const { /*has_local_modifications=*/false); } +void Serializer::EncodeMapValue(Writer* writer, + const ObjectValue& object_value) { + EncodeObjectMap(writer, object_value.internal_value, + google_firestore_v1beta1_MapValue_fields_tag, + google_firestore_v1beta1_MapValue_FieldsEntry_key_tag, + google_firestore_v1beta1_MapValue_FieldsEntry_value_tag); +} + void Serializer::EncodeObjectMap( nanopb::Writer* writer, const model::ObjectValue::Map& object_value_map, uint32_t map_tag, uint32_t key_tag, uint32_t value_tag) { - // TODO(rsgowman): Move the implementation of EncodeObjectMapImpl here and - // eliminate that function. The only reason I'm (temporarily) keeping it, is - // that performing that refactoring now will cause a cascade of things that - // need to move into the local serializer class (as private functions). - EncodeObjectMapImpl(writer, object_value_map, map_tag, key_tag, value_tag); + // Write each FieldsEntry (i.e. key-value pair.) + for (const auto& kv : object_value_map) { + writer->WriteTag({PB_WT_STRING, map_tag}); + writer->WriteNestedMessage([&kv, &key_tag, &value_tag](Writer* writer) { + return EncodeFieldsEntry(writer, kv, key_tag, value_tag); + }); + } } void Serializer::EncodeVersion(nanopb::Writer* writer, @@ -686,6 +630,40 @@ void Serializer::EncodeVersion(nanopb::Writer* writer, EncodeTimestamp(writer, version.timestamp()); } +/** + * Encodes a 'FieldsEntry' object, within a FieldValue's map_value type. + * + * In protobuf, maps are implemented as a repeated set of key/values. For + * instance, this: + * message Foo { + * map fields = 1; + * } + * would be written (in proto text format) as: + * { + * fields: {key:"key string 1", value:{}} + * fields: {key:"key string 2", value:{}} + * ... + * } + * + * This method writes an individual entry from that list. It is expected that + * this method will be called once for each entry in the map. + * + * @param kv The individual key/value pair to write. + */ +void Serializer::EncodeFieldsEntry(Writer* writer, + const ObjectValue::Map::value_type& kv, + uint32_t key_tag, + uint32_t value_tag) { + // Write the key (string) + writer->WriteTag({PB_WT_STRING, key_tag}); + writer->WriteString(kv.first); + + // Write the value (FieldValue) + writer->WriteTag({PB_WT_STRING, value_tag}); + writer->WriteNestedMessage( + [&kv](Writer* writer) { EncodeFieldValue(writer, kv.second); }); +} + } // namespace remote } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.h b/Firestore/core/src/firebase/firestore/remote/serializer.h index 3b72693..5fd6fbc 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.h +++ b/Firestore/core/src/firebase/firestore/remote/serializer.h @@ -181,6 +181,17 @@ class Serializer { std::unique_ptr DecodeBatchGetDocumentsResponse( nanopb::Reader* reader) const; + static void EncodeMapValue(nanopb::Writer* writer, + const model::ObjectValue& object_value); + + static void EncodeFieldsEntry(nanopb::Writer* writer, + const model::ObjectValue::Map::value_type& kv, + uint32_t key_tag, + uint32_t value_tag); + + static void EncodeFieldValue(nanopb::Writer* writer, + const model::FieldValue& field_value); + const firebase::firestore::model::DatabaseId& database_id_; }; -- cgit v1.2.3