From 520a0c0a84c34897aae10564ed48ec3efc508fbf Mon Sep 17 00:00:00 2001 From: rsgowman Date: Tue, 15 May 2018 17:02:08 -0400 Subject: [De]serialize empty Document instances (#1262) * [De]serialize empty Document instances Still TODO: - non-empty - NoDocument - ErrorHandling --- .../core/src/firebase/firestore/nanopb/reader.h | 9 +- .../src/firebase/firestore/remote/serializer.cc | 144 +++++++++++++++++++++ .../src/firebase/firestore/remote/serializer.h | 56 +++++++- 3 files changed, 206 insertions(+), 3 deletions(-) (limited to 'Firestore/core/src/firebase/firestore') diff --git a/Firestore/core/src/firebase/firestore/nanopb/reader.h b/Firestore/core/src/firebase/firestore/nanopb/reader.h index 7d77a4d..093e20b 100644 --- a/Firestore/core/src/firebase/firestore/nanopb/reader.h +++ b/Firestore/core/src/firebase/firestore/nanopb/reader.h @@ -80,6 +80,11 @@ class Reader { * * Call this method when reading a nested message. Provide a function to read * the message itself. + * + * @param read_message_fn Function to read the submessage. Note that this + * function is expected to check the Reader's status (via + * Reader::status().ok()) and if not ok, to return a placeholder/invalid + * value. */ template T ReadNestedMessage(const std::function& read_message_fn); @@ -130,14 +135,14 @@ T Reader::ReadNestedMessage(const std::function& read_message_fn) { // Implementation note: This is roughly modeled on pb_decode_delimited, // adjusted to account for the oneof in FieldValue. - if (!status_.ok()) return T(); + if (!status_.ok()) return read_message_fn(this); pb_istream_t raw_substream; if (!pb_make_string_substream(&stream_, &raw_substream)) { status_ = util::Status(FirestoreErrorCode::DataLoss, PB_GET_ERROR(&stream_)); pb_close_string_substream(&stream_, &raw_substream); - return T(); + return read_message_fn(this); } Reader substream(raw_substream); diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index 0a1c168..6240c21 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -25,14 +25,19 @@ #include #include "Firestore/Protos/nanopb/google/firestore/v1beta1/document.pb.h" +#include "Firestore/Protos/nanopb/google/firestore/v1beta1/firestore.pb.h" #include "Firestore/core/include/firebase/firestore/firestore_errors.h" #include "Firestore/core/include/firebase/firestore/timestamp.h" +#include "Firestore/core/src/firebase/firestore/model/document.h" +#include "Firestore/core/src/firebase/firestore/model/no_document.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" +#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/nanopb/reader.h" #include "Firestore/core/src/firebase/firestore/nanopb/tag.h" #include "Firestore/core/src/firebase/firestore/nanopb/writer.h" #include "Firestore/core/src/firebase/firestore/timestamp_internal.h" #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" +#include "absl/memory/memory.h" namespace firebase { namespace firestore { @@ -41,10 +46,14 @@ namespace remote { using firebase::Timestamp; using firebase::TimestampInternal; using firebase::firestore::model::DatabaseId; +using firebase::firestore::model::Document; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldValue; +using firebase::firestore::model::MaybeDocument; +using firebase::firestore::model::NoDocument; using firebase::firestore::model::ObjectValue; using firebase::firestore::model::ResourcePath; +using firebase::firestore::model::SnapshotVersion; using firebase::firestore::nanopb::Reader; using firebase::firestore::nanopb::Tag; using firebase::firestore::nanopb::Writer; @@ -67,6 +76,8 @@ void EncodeTimestamp(Writer* writer, const Timestamp& timestamp_value) { } Timestamp DecodeTimestamp(Reader* reader) { + if (!reader->status().ok()) return {}; + google_protobuf_Timestamp timestamp_proto = google_protobuf_Timestamp_init_zero; reader->ReadNanopbMessage(google_protobuf_Timestamp_fields, ×tamp_proto); @@ -147,6 +158,8 @@ void EncodeFieldValueImpl(Writer* writer, const FieldValue& field_value) { } FieldValue DecodeFieldValueImpl(Reader* reader) { + if (!reader->status().ok()) return FieldValue::NullValue(); + Tag tag = reader->ReadTag(); if (!reader->status().ok()) return FieldValue::NullValue(); @@ -255,6 +268,8 @@ void EncodeFieldsEntry(Writer* writer, const ObjectValue::Map::value_type& kv) { } ObjectValue::Map::value_type DecodeFieldsEntry(Reader* reader) { + if (!reader->status().ok()) return {}; + Tag tag = reader->ReadTag(); if (!reader->status().ok()) return {}; @@ -413,6 +428,135 @@ DocumentKey Serializer::DecodeKey(absl::string_view name) const { return DocumentKey{ExtractLocalPathFromResourceName(resource)}; } +util::Status Serializer::EncodeDocument(const DocumentKey& key, + const ObjectValue& value, + std::vector* out_bytes) const { + Writer writer = Writer::Wrap(out_bytes); + EncodeDocument(&writer, key, value); + return writer.status(); +} + +void Serializer::EncodeDocument(Writer* writer, + const DocumentKey& key, + const ObjectValue& object_value) const { + // Encode Document.name + writer->WriteTag({PB_WT_STRING, google_firestore_v1beta1_Document_name_tag}); + writer->WriteString(EncodeKey(key)); + + // Encode Document.fields (unless it's empty) + if (!object_value.internal_value.empty()) { + writer->WriteTag( + {PB_WT_STRING, google_firestore_v1beta1_Document_fields_tag}); + EncodeObject(writer, object_value); + } + + // Skip Document.create_time and Document.update_time, since they're + // output-only fields. +} + +util::StatusOr> +Serializer::DecodeMaybeDocument(const uint8_t* bytes, size_t length) const { + Reader reader = Reader::Wrap(bytes, length); + std::unique_ptr maybeDoc = + DecodeBatchGetDocumentsResponse(&reader); + + if (reader.status().ok()) { + return maybeDoc; + } else { + return reader.status(); + } +} + +std::unique_ptr Serializer::DecodeBatchGetDocumentsResponse( + Reader* reader) const { + Tag tag = reader->ReadTag(); + if (!reader->status().ok()) return nullptr; + + // Ensure the tag matches the wire type + switch (tag.field_number) { + case google_firestore_v1beta1_BatchGetDocumentsResponse_found_tag: + case google_firestore_v1beta1_BatchGetDocumentsResponse_missing_tag: + if (tag.wire_type != PB_WT_STRING) { + reader->set_status( + Status(FirestoreErrorCode::DataLoss, + "Input proto bytes cannot be parsed (mismatch between " + "the wiretype and the field number (tag))")); + } + break; + + default: + reader->set_status(Status( + FirestoreErrorCode::DataLoss, + "Input proto bytes cannot be parsed (invalid field number (tag))")); + } + + if (!reader->status().ok()) return nullptr; + + switch (tag.field_number) { + case google_firestore_v1beta1_BatchGetDocumentsResponse_found_tag: + return reader->ReadNestedMessage>( + [this](Reader* reader) -> std::unique_ptr { + return DecodeDocument(reader); + }); + case google_firestore_v1beta1_BatchGetDocumentsResponse_missing_tag: + // TODO(rsgowman): Right now, we only support Document (and don't support + // NoDocument). That should change in the next PR or so. + abort(); + default: + // This indicates an internal error as we've already ensured that this is + // a valid field_number. + FIREBASE_ASSERT_MESSAGE( + false, + "Somehow got an unexpected field number (tag) after verifying that " + "the field number was expected."); + } +} + +std::unique_ptr Serializer::DecodeDocument(Reader* reader) const { + if (!reader->status().ok()) return nullptr; + + std::string name; + FieldValue fields = FieldValue::ObjectValueFromMap({}); + SnapshotVersion version = SnapshotVersion::None(); + + while (reader->bytes_left()) { + Tag tag = reader->ReadTag(); + if (!reader->status().ok()) return nullptr; + FIREBASE_ASSERT(tag.wire_type == PB_WT_STRING); + switch (tag.field_number) { + case google_firestore_v1beta1_Document_name_tag: + name = reader->ReadString(); + break; + case google_firestore_v1beta1_Document_fields_tag: + // TODO(rsgowman): Rather than overwriting, we should instead merge with + // the existing FieldValue (if any). + fields = DecodeFieldValueImpl(reader); + break; + case google_firestore_v1beta1_Document_create_time_tag: + // This field is ignored by the client sdk, but we still need to extract + // it. + reader->ReadNestedMessage(DecodeTimestamp); + break; + case google_firestore_v1beta1_Document_update_time_tag: + // TODO(rsgowman): Rather than overwriting, we should instead merge with + // the existing SnapshotVersion (if any). Less relevant here, since it's + // just two numbers which are both expected to be present, but if the + // proto evolves that might change. + version = SnapshotVersion{ + reader->ReadNestedMessage(DecodeTimestamp)}; + break; + default: + // TODO(rsgowman): Error handling. (Invalid tags should fail to decode, + // but shouldn't cause a crash.) + abort(); + } + } + + return absl::make_unique(std::move(fields), DecodeKey(name), + version, + /*has_local_modifications=*/false); +} + } // 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 86aa6e2..e79c238 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.h +++ b/Firestore/core/src/firebase/firestore/remote/serializer.h @@ -19,12 +19,17 @@ #include #include +#include #include #include #include "Firestore/core/src/firebase/firestore/model/database_id.h" +#include "Firestore/core/src/firebase/firestore/model/document.h" #include "Firestore/core/src/firebase/firestore/model/document_key.h" #include "Firestore/core/src/firebase/firestore/model/field_value.h" +#include "Firestore/core/src/firebase/firestore/model/maybe_document.h" +#include "Firestore/core/src/firebase/firestore/nanopb/reader.h" +#include "Firestore/core/src/firebase/firestore/nanopb/writer.h" #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" #include "Firestore/core/src/firebase/firestore/util/status.h" #include "Firestore/core/src/firebase/firestore/util/statusor.h" @@ -59,7 +64,7 @@ class Serializer { } /** - * Converts the FieldValue model passed into bytes. + * @brief Converts the FieldValue model passed into bytes. * * @param field_value the model to convert. * @param[out] out_bytes A buffer to place the output. The bytes will be @@ -111,7 +116,56 @@ class Serializer { firebase::firestore::model::DocumentKey DecodeKey( absl::string_view name) const; + /** + * @brief Converts the Document (i.e. key/value) into bytes. + * + * @param[out] out_bytes A buffer to place the output. The bytes will be + * appended to this vector. + * @return A Status, which if not ok(), indicates what went wrong. Note that + * errors during encoding generally indicate a serious/fatal error. + */ + // TODO(rsgowman): Similar to above, if we never support any output except to + // a vector, it may make sense to have Serializer own the vector and provide + // an accessor rather than asking the user to create it first. + util::Status EncodeDocument( + const firebase::firestore::model::DocumentKey& key, + const firebase::firestore::model::ObjectValue& value, + std::vector* out_bytes) const; + + /** + * @brief Converts from bytes to the model Document format. + * + * @param bytes The bytes to convert. These bytes must represent a + * BatchGetDocumentsResponse. It's assumed that exactly all of the bytes will + * be used by this conversion. + * @return The model equivalent of the bytes or a Status indicating + * what went wrong. + */ + util::StatusOr> DecodeMaybeDocument( + const uint8_t* bytes, size_t length) const; + + /** + * @brief Converts from bytes to the model Document format. + * + * @param bytes The bytes to convert. These bytes must represent a + * BatchGetDocumentsResponse. It's assumed that exactly all of the bytes will + * be used by this conversion. + * @return The model equivalent of the bytes or a Status indicating + * what went wrong. + */ + util::StatusOr> DecodeMaybeDocument( + const std::vector& bytes) const { + return DecodeMaybeDocument(bytes.data(), bytes.size()); + } + private: + void EncodeDocument(nanopb::Writer* writer, + const model::DocumentKey& key, + const model::ObjectValue& object_value) const; + std::unique_ptr DecodeBatchGetDocumentsResponse( + nanopb::Reader* reader) const; + std::unique_ptr DecodeDocument(nanopb::Reader* reader) const; + const firebase::firestore::model::DatabaseId& database_id_; }; -- cgit v1.2.3