From a3f792f3f093e913be5823cb4df9dfeac7612a52 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Tue, 3 Jul 2018 15:59:01 -0400 Subject: Add support for NoDocument in the local serializer (#1484) --- .../firebase/firestore/local/local_serializer.cc | 70 ++++++- .../firebase/firestore/local/local_serializer.h | 8 + .../src/firebase/firestore/remote/serializer.cc | 220 +++++++++++---------- .../src/firebase/firestore/remote/serializer.h | 3 + .../firestore/local/local_serializer_test.cc | 15 ++ 5 files changed, 202 insertions(+), 114 deletions(-) (limited to 'Firestore') diff --git a/Firestore/core/src/firebase/firestore/local/local_serializer.cc b/Firestore/core/src/firebase/firestore/local/local_serializer.cc index d8e039b..c549604 100644 --- a/Firestore/core/src/firebase/firestore/local/local_serializer.cc +++ b/Firestore/core/src/firebase/firestore/local/local_serializer.cc @@ -17,12 +17,14 @@ #include "Firestore/core/src/firebase/firestore/local/local_serializer.h" #include +#include #include #include "Firestore/Protos/nanopb/firestore/local/maybe_document.nanopb.h" #include "Firestore/Protos/nanopb/google/firestore/v1beta1/document.nanopb.h" #include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/model/no_document.h" +#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/nanopb/tag.h" #include "Firestore/core/src/firebase/firestore/util/hard_assert.h" @@ -31,6 +33,7 @@ namespace firestore { namespace local { using firebase::firestore::model::ObjectValue; +using firebase::firestore::model::SnapshotVersion; using firebase::firestore::nanopb::Reader; using firebase::firestore::nanopb::Tag; using firebase::firestore::nanopb::Writer; @@ -56,8 +59,13 @@ void LocalSerializer::EncodeMaybeDocument( return; case model::MaybeDocument::Type::NoDocument: - // TODO(rsgowman) - abort(); + writer->WriteTag( + {PB_WT_STRING, firestore_client_MaybeDocument_no_document_tag}); + writer->WriteNestedMessage([&](Writer* writer) { + EncodeNoDocument(writer, + static_cast(maybe_doc)); + }); + return; case model::MaybeDocument::Type::Unknown: // TODO(rsgowman) @@ -90,7 +98,7 @@ std::unique_ptr LocalSerializer::DecodeMaybeDocument( // claim that if both are set on the wire, the last one wins. no_document = nullptr; - // TODO(rsgowman): If multiple '_document' values are found, we should + // TODO(rsgowman): If multiple 'document' values are found, we should // merge them (rather than using the last one.) document = reader->ReadNestedMessage>( [&](Reader* reader) -> std::unique_ptr { @@ -106,8 +114,12 @@ std::unique_ptr LocalSerializer::DecodeMaybeDocument( // claim that if both are set on the wire, the last one wins. document = nullptr; - // TODO(rsgowman): Parse the no_document field. - abort(); + // TODO(rsgowman): If multiple 'no_document' values are found, we should + // merge them (rather than using the last one.) + no_document = + reader->ReadNestedMessage>( + [&](Reader* reader) { return DecodeNoDocument(reader); }); + break; break; @@ -155,6 +167,19 @@ void LocalSerializer::EncodeDocument(Writer* writer, // Ignore Document.create_time. (We don't use this in our on-disk protos.) } +void LocalSerializer::EncodeNoDocument(Writer* writer, + const model::NoDocument& no_doc) const { + // Encode NoDocument.name + writer->WriteTag({PB_WT_STRING, firestore_client_NoDocument_name_tag}); + writer->WriteString(rpc_serializer_.EncodeKey(no_doc.key())); + + // Encode NoDocument.read_time + writer->WriteTag({PB_WT_STRING, firestore_client_NoDocument_read_time_tag}); + writer->WriteNestedMessage([&](Writer* writer) { + rpc_serializer_.EncodeVersion(writer, no_doc.version()); + }); +} + util::StatusOr> LocalSerializer::DecodeMaybeDocument(const uint8_t* bytes, size_t length) const { @@ -168,6 +193,41 @@ LocalSerializer::DecodeMaybeDocument(const uint8_t* bytes, } } +std::unique_ptr LocalSerializer::DecodeNoDocument( + Reader* reader) const { + if (!reader->status().ok()) return nullptr; + + std::string name; + SnapshotVersion version = SnapshotVersion::None(); + + while (reader->bytes_left()) { + Tag tag = reader->ReadTag(); + if (!reader->status().ok()) return nullptr; + + // Ensure the tag matches the wire type + switch (tag.field_number) { + case firestore_client_NoDocument_name_tag: + if (!reader->RequireWireType(PB_WT_STRING, tag)) return nullptr; + name = reader->ReadString(); + break; + + case firestore_client_NoDocument_read_time_tag: + if (!reader->RequireWireType(PB_WT_STRING, tag)) return nullptr; + version = SnapshotVersion{reader->ReadNestedMessage( + rpc_serializer_.DecodeTimestamp)}; + break; + + default: + // Unknown tag. According to the proto spec, we need to ignore these. + reader->SkipField(tag); + break; + } + } + + return absl::make_unique(rpc_serializer_.DecodeKey(name), + version); +} + } // namespace local } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/local/local_serializer.h b/Firestore/core/src/firebase/firestore/local/local_serializer.h index c52e324..5c94a34 100644 --- a/Firestore/core/src/firebase/firestore/local/local_serializer.h +++ b/Firestore/core/src/firebase/firestore/local/local_serializer.h @@ -20,7 +20,9 @@ #include #include +#include "Firestore/core/src/firebase/firestore/model/document.h" #include "Firestore/core/src/firebase/firestore/model/maybe_document.h" +#include "Firestore/core/src/firebase/firestore/model/no_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/remote/serializer.h" @@ -99,6 +101,12 @@ class LocalSerializer { */ void EncodeDocument(nanopb::Writer* writer, const model::Document& doc) const; + void EncodeNoDocument(nanopb::Writer* writer, + const model::NoDocument& no_doc) const; + + std::unique_ptr DecodeNoDocument( + nanopb::Reader* reader) const; + const remote::Serializer& rpc_serializer_; }; diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index ce133e6..28d9907 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -73,113 +73,6 @@ void EncodeTimestamp(Writer* writer, const Timestamp& timestamp_value) { ×tamp_proto); } -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); - - // The Timestamp ctor will assert if we provide values outside the valid - // range. However, since we're decoding, a single corrupt byte could cause - // this to occur, so we'll verify the ranges before passing them in since we'd - // rather not abort in these situations. - if (timestamp_proto.seconds < TimestampInternal::Min().seconds()) { - reader->set_status(Status( - FirestoreErrorCode::DataLoss, - "Invalid message: timestamp beyond the earliest supported date")); - return {}; - } else if (TimestampInternal::Max().seconds() < timestamp_proto.seconds) { - reader->set_status( - Status(FirestoreErrorCode::DataLoss, - "Invalid message: timestamp behond the latest supported date")); - return {}; - } else if (timestamp_proto.nanos < 0 || timestamp_proto.nanos > 999999999) { - reader->set_status(Status( - FirestoreErrorCode::DataLoss, - "Invalid message: timestamp nanos must be between 0 and 999999999")); - return {}; - } - return Timestamp{timestamp_proto.seconds, timestamp_proto.nanos}; -} - -FieldValue DecodeFieldValueImpl(Reader* reader) { - if (!reader->status().ok()) return FieldValue::NullValue(); - - // There needs to be at least one entry in the FieldValue. - if (reader->bytes_left() == 0) { - reader->set_status(Status(FirestoreErrorCode::DataLoss, - "Input Value proto missing contents")); - return FieldValue::NullValue(); - } - - FieldValue result = FieldValue::NullValue(); - - while (reader->bytes_left()) { - Tag tag = reader->ReadTag(); - if (!reader->status().ok()) return FieldValue::NullValue(); - - // Ensure the tag matches the wire type - switch (tag.field_number) { - case google_firestore_v1beta1_Value_null_value_tag: - if (!reader->RequireWireType(PB_WT_VARINT, tag)) - return FieldValue::NullValue(); - reader->ReadNull(); - result = FieldValue::NullValue(); - break; - - case google_firestore_v1beta1_Value_boolean_value_tag: - if (!reader->RequireWireType(PB_WT_VARINT, tag)) - return FieldValue::NullValue(); - result = FieldValue::BooleanValue(reader->ReadBool()); - break; - - case google_firestore_v1beta1_Value_integer_value_tag: - if (!reader->RequireWireType(PB_WT_VARINT, tag)) - return FieldValue::NullValue(); - result = FieldValue::IntegerValue(reader->ReadInteger()); - break; - - case google_firestore_v1beta1_Value_string_value_tag: - if (!reader->RequireWireType(PB_WT_STRING, tag)) - return FieldValue::NullValue(); - result = FieldValue::StringValue(reader->ReadString()); - break; - - case google_firestore_v1beta1_Value_timestamp_value_tag: - if (!reader->RequireWireType(PB_WT_STRING, tag)) - return FieldValue::NullValue(); - result = FieldValue::TimestampValue( - reader->ReadNestedMessage(DecodeTimestamp)); - break; - - case google_firestore_v1beta1_Value_map_value_tag: - if (!reader->RequireWireType(PB_WT_STRING, tag)) - return FieldValue::NullValue(); - // TODO(rsgowman): We should merge the existing map (if any) with the - // newly parsed map. - result = FieldValue::ObjectValueFromMap( - reader->ReadNestedMessage(DecodeMapValue)); - break; - - case google_firestore_v1beta1_Value_double_value_tag: - case google_firestore_v1beta1_Value_bytes_value_tag: - case google_firestore_v1beta1_Value_reference_value_tag: - case google_firestore_v1beta1_Value_geo_point_value_tag: - case google_firestore_v1beta1_Value_array_value_tag: - // TODO(b/74243929): Implement remaining types. - HARD_FAIL("Unhandled message field number (tag): %i.", - tag.field_number); - - default: - // Unknown tag. According to the proto spec, we need to ignore these. - reader->SkipField(tag); - } - } - - return result; -} - ObjectValue::Map::value_type DecodeFieldsEntry(Reader* reader, uint32_t key_tag, uint32_t value_tag) { @@ -200,7 +93,9 @@ ObjectValue::Map::value_type DecodeFieldsEntry(Reader* reader, HARD_ASSERT(tag.wire_type == PB_WT_STRING); FieldValue value = - reader->ReadNestedMessage(DecodeFieldValueImpl); + reader->ReadNestedMessage([](Reader* reader) -> FieldValue { + return Serializer::DecodeFieldValue(reader); + }); return ObjectValue::Map::value_type{key, value}; } @@ -372,7 +267,7 @@ void Serializer::EncodeFieldValue(Writer* writer, StatusOr Serializer::DecodeFieldValue(const uint8_t* bytes, size_t length) { Reader reader = Reader::Wrap(bytes, length); - FieldValue fv = DecodeFieldValueImpl(&reader); + FieldValue fv = DecodeFieldValue(&reader); if (reader.status().ok()) { return fv; } else { @@ -380,6 +275,83 @@ StatusOr Serializer::DecodeFieldValue(const uint8_t* bytes, } } +FieldValue Serializer::DecodeFieldValue(Reader* reader) { + if (!reader->status().ok()) return FieldValue::NullValue(); + + // There needs to be at least one entry in the FieldValue. + if (reader->bytes_left() == 0) { + reader->set_status(Status(FirestoreErrorCode::DataLoss, + "Input Value proto missing contents")); + return FieldValue::NullValue(); + } + + FieldValue result = FieldValue::NullValue(); + + while (reader->bytes_left()) { + Tag tag = reader->ReadTag(); + if (!reader->status().ok()) return FieldValue::NullValue(); + + // Ensure the tag matches the wire type + switch (tag.field_number) { + case google_firestore_v1beta1_Value_null_value_tag: + if (!reader->RequireWireType(PB_WT_VARINT, tag)) + return FieldValue::NullValue(); + reader->ReadNull(); + result = FieldValue::NullValue(); + break; + + case google_firestore_v1beta1_Value_boolean_value_tag: + if (!reader->RequireWireType(PB_WT_VARINT, tag)) + return FieldValue::NullValue(); + result = FieldValue::BooleanValue(reader->ReadBool()); + break; + + case google_firestore_v1beta1_Value_integer_value_tag: + if (!reader->RequireWireType(PB_WT_VARINT, tag)) + return FieldValue::NullValue(); + result = FieldValue::IntegerValue(reader->ReadInteger()); + break; + + case google_firestore_v1beta1_Value_string_value_tag: + if (!reader->RequireWireType(PB_WT_STRING, tag)) + return FieldValue::NullValue(); + result = FieldValue::StringValue(reader->ReadString()); + break; + + case google_firestore_v1beta1_Value_timestamp_value_tag: + if (!reader->RequireWireType(PB_WT_STRING, tag)) + return FieldValue::NullValue(); + result = FieldValue::TimestampValue( + reader->ReadNestedMessage(DecodeTimestamp)); + break; + + case google_firestore_v1beta1_Value_map_value_tag: + if (!reader->RequireWireType(PB_WT_STRING, tag)) + return FieldValue::NullValue(); + // TODO(rsgowman): We should merge the existing map (if any) with the + // newly parsed map. + result = FieldValue::ObjectValueFromMap( + reader->ReadNestedMessage(DecodeMapValue)); + break; + + case google_firestore_v1beta1_Value_double_value_tag: + case google_firestore_v1beta1_Value_bytes_value_tag: + case google_firestore_v1beta1_Value_reference_value_tag: + case google_firestore_v1beta1_Value_geo_point_value_tag: + case google_firestore_v1beta1_Value_array_value_tag: + // TODO(b/74243929): Implement remaining types. + HARD_FAIL("Unhandled message field number (tag): %i.", + tag.field_number); + + default: + // Unknown tag. According to the proto spec, we need to ignore these. + reader->SkipField(tag); + } + } + + return result; +} + std::string Serializer::EncodeKey(const DocumentKey& key) const { return EncodeResourceName(database_id_, key.path()); } @@ -615,6 +587,36 @@ void Serializer::EncodeFieldsEntry(Writer* writer, [&kv](Writer* writer) { EncodeFieldValue(writer, kv.second); }); } +Timestamp Serializer::DecodeTimestamp(nanopb::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); + + // The Timestamp ctor will assert if we provide values outside the valid + // range. However, since we're decoding, a single corrupt byte could cause + // this to occur, so we'll verify the ranges before passing them in since we'd + // rather not abort in these situations. + if (timestamp_proto.seconds < TimestampInternal::Min().seconds()) { + reader->set_status(Status( + FirestoreErrorCode::DataLoss, + "Invalid message: timestamp beyond the earliest supported date")); + return {}; + } else if (TimestampInternal::Max().seconds() < timestamp_proto.seconds) { + reader->set_status( + Status(FirestoreErrorCode::DataLoss, + "Invalid message: timestamp behond the latest supported date")); + return {}; + } else if (timestamp_proto.nanos < 0 || timestamp_proto.nanos > 999999999) { + reader->set_status(Status( + FirestoreErrorCode::DataLoss, + "Invalid message: timestamp nanos must be between 0 and 999999999")); + return {}; + } + return Timestamp{timestamp_proto.seconds, timestamp_proto.nanos}; +} + } // 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 5fd6fbc..6564e2a 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.h +++ b/Firestore/core/src/firebase/firestore/remote/serializer.h @@ -174,6 +174,9 @@ class Serializer { static void EncodeVersion(nanopb::Writer* writer, const model::SnapshotVersion& version); + static Timestamp DecodeTimestamp(nanopb::Reader* reader); + static model::FieldValue DecodeFieldValue(nanopb::Reader* reader); + private: void EncodeDocument(nanopb::Writer* writer, const model::DocumentKey& key, diff --git a/Firestore/core/test/firebase/firestore/local/local_serializer_test.cc b/Firestore/core/test/firebase/firestore/local/local_serializer_test.cc index a4543ab..9b97038 100644 --- a/Firestore/core/test/firebase/firestore/local/local_serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/local/local_serializer_test.cc @@ -19,6 +19,7 @@ #include "Firestore/Protos/cpp/firestore/local/maybe_document.pb.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/model/no_document.h" #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h" #include "Firestore/core/src/firebase/firestore/remote/serializer.h" #include "Firestore/core/src/firebase/firestore/util/status.h" @@ -34,7 +35,9 @@ using firebase::firestore::model::DatabaseId; using firebase::firestore::model::Document; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::MaybeDocument; +using firebase::firestore::model::NoDocument; using firebase::firestore::model::SnapshotVersion; +using firebase::firestore::testutil::DeletedDoc; using firebase::firestore::testutil::Doc; using firebase::firestore::util::Status; using firebase::firestore::util::StatusOr; @@ -148,3 +151,15 @@ TEST_F(LocalSerializerTest, EncodesDocumentAsMaybeDocument) { ExpectRoundTrip(doc, maybe_doc_proto, doc.type()); } + +TEST_F(LocalSerializerTest, EncodesNoDocumentAsMaybeDocument) { + NoDocument no_doc = DeletedDoc("some/path", /*version=*/42); + + firestore::client::MaybeDocument maybe_doc_proto; + maybe_doc_proto.mutable_no_document()->set_name( + "projects/p/databases/d/documents/some/path"); + maybe_doc_proto.mutable_no_document()->mutable_read_time()->set_seconds(0); + maybe_doc_proto.mutable_no_document()->mutable_read_time()->set_nanos(42000); + + ExpectRoundTrip(no_doc, maybe_doc_proto, no_doc.type()); +} -- cgit v1.2.3