Simplify Serializer [En|De]code*() methods to no longer [de]serialize manually. (#1816)

Now, we use the re-worked nanopb deserializer (that mallocs). This also
had the advantage of moving the C++ serialization code closer to the
other platforms.

Still TODO:
- Reorder the methods within the files to resemble the other platforms.

Author: rsgowman

FirebaseFirestore.podspec

@@ -55,7 +55,7 @@ Google Cloud Firestore is a NoSQL document database built for automatic scaling,
   s.dependency 'gRPC-C++', '~> 0.0.3'
   s.dependency 'leveldb-library', '~> 1.20'
   s.dependency 'Protobuf', '~> 3.1'
-  s.dependency 'nanopb', '~> 0.3.8'
+  s.dependency 'nanopb', '~> 0.3.901'
 
   s.frameworks = 'MobileCoreServices', 'SystemConfiguration'
   s.library = 'c++'
@@ -66,7 +66,7 @@ Google Cloud Firestore is a NoSQL document database built for automatic scaling,
       # The nanopb pod sets these defs, so we must too. (We *do* require 16bit
       # (or larger) fields, so we'd have to set at least PB_FIELD_16BIT
       # anyways.)
-      'PB_FIELD_32BIT=1 PB_NO_PACKED_STRUCTS=1',
+      'PB_FIELD_32BIT=1 PB_NO_PACKED_STRUCTS=1 PB_ENABLE_MALLOC=1',
     'HEADER_SEARCH_PATHS' =>
       '"${PODS_TARGET_SRCROOT}" ' +
       '"${PODS_TARGET_SRCROOT}/Firestore/third_party/abseil-cpp" ' +

Firestore/Example/Firestore.xcodeproj/project.pbxproj

@@ -2484,6 +2484,7 @@
 					"\"${PODS_ROOT}/GoogleTest/googlemock/include\"",
 					"\"${PODS_ROOT}/GoogleTest/googletest/include\"",
 					"\"${PODS_ROOT}/leveldb-library/include\"",
+					"\"${PODS_ROOT}/../../../Firestore/Protos/nanopb\"",
 					"\"${PODS_ROOT}/../../../Firestore/Protos/cpp\"",
 					"\"${PODS_ROOT}/ProtobufCpp/src\"",
 				);
@@ -2569,6 +2570,7 @@
 					"\"${PODS_ROOT}/GoogleTest/googlemock/include\"",
 					"\"${PODS_ROOT}/GoogleTest/googletest/include\"",
 					"\"${PODS_ROOT}/leveldb-library/include\"",
+					"\"${PODS_ROOT}/../../../Firestore/Protos/nanopb\"",
 					"\"${PODS_ROOT}/../../../Firestore/Protos/cpp\"",
 					"\"${PODS_ROOT}/ProtobufCpp/src\"",
 				);

Firestore/core/src/firebase/firestore/local/local_serializer.cc

@@ -27,8 +27,8 @@
 #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"
+#include "Firestore/core/src/firebase/firestore/util/string_format.h"
 
 namespace firebase {
 namespace firestore {
@@ -41,28 +41,27 @@ using model::NoDocument;
 using model::ObjectValue;
 using model::SnapshotVersion;
 using nanopb::Reader;
-using nanopb::Tag;
 using nanopb::Writer;
+using remote::MakeArray;
 using util::Status;
+using util::StringFormat;
+
+firestore_client_MaybeDocument LocalSerializer::EncodeMaybeDocument(
+    const MaybeDocument& maybe_doc) const {
+  firestore_client_MaybeDocument result{};
 
-void LocalSerializer::EncodeMaybeDocument(
-    Writer* writer, const MaybeDocument& maybe_doc) const {
   switch (maybe_doc.type()) {
     case MaybeDocument::Type::Document:
-      writer->WriteTag(
-          {PB_WT_STRING, firestore_client_MaybeDocument_document_tag});
-      writer->WriteNestedMessage([&](Writer* writer) {
-        EncodeDocument(writer, static_cast<const Document&>(maybe_doc));
-      });
-      return;
+      result.which_document_type = firestore_client_MaybeDocument_document_tag;
+      result.document = EncodeDocument(static_cast<const Document&>(maybe_doc));
+      return result;
 
     case MaybeDocument::Type::NoDocument:
-      writer->WriteTag(
-          {PB_WT_STRING, firestore_client_MaybeDocument_no_document_tag});
-      writer->WriteNestedMessage([&](Writer* writer) {
-        EncodeNoDocument(writer, static_cast<const NoDocument&>(maybe_doc));
-      });
-      return;
+      result.which_document_type =
+          firestore_client_MaybeDocument_no_document_tag;
+      result.no_document =
+          EncodeNoDocument(static_cast<const NoDocument&>(maybe_doc));
+      return result;
 
     case MaybeDocument::Type::Unknown:
       // TODO(rsgowman)
@@ -73,195 +72,137 @@ void LocalSerializer::EncodeMaybeDocument(
 }
 
 std::unique_ptr<MaybeDocument> LocalSerializer::DecodeMaybeDocument(
-    Reader* reader) const {
-  std::unique_ptr<MaybeDocument> result;
-
-  while (reader->good()) {
-    switch (reader->ReadTag()) {
-      case firestore_client_MaybeDocument_document_tag:
-        // TODO(rsgowman): If multiple 'document' values are found, we should
-        // merge them (rather than using the last one.)
-        result = reader->ReadNestedMessage<Document>(
-            rpc_serializer_, &remote::Serializer::DecodeDocument);
-        break;
-
-      case firestore_client_MaybeDocument_no_document_tag:
-        // TODO(rsgowman): If multiple 'no_document' values are found, we should
-        // merge them (rather than using the last one.)
-        result = reader->ReadNestedMessage<NoDocument>(
-            *this, &LocalSerializer::DecodeNoDocument);
-        break;
-
-      default:
-        reader->SkipUnknown();
-    }
-  }
+    Reader* reader, const firestore_client_MaybeDocument& proto) const {
+  if (!reader->status().ok()) return nullptr;
 
-  if (!result) {
-    reader->Fail(
-        "Invalid MaybeDocument message: Neither 'no_document' nor 'document' "
-        "fields set.");
-    return nullptr;
+  switch (proto.which_document_type) {
+    case firestore_client_MaybeDocument_document_tag:
+      return rpc_serializer_.DecodeDocument(reader, proto.document);
+
+    case firestore_client_MaybeDocument_no_document_tag:
+      return DecodeNoDocument(reader, proto.no_document);
+
+    default:
+      reader->Fail(
+          StringFormat("Invalid MaybeDocument document type: %s. Expected "
+                       "'no_document' (%s) or 'document' (%s)",
+                       proto.which_document_type,
+                       firestore_client_MaybeDocument_no_document_tag,
+                       firestore_client_MaybeDocument_document_tag));
+      return nullptr;
   }
-  return result;
+
+  UNREACHABLE();
 }
 
-void LocalSerializer::EncodeDocument(Writer* writer,
-                                     const Document& doc) const {
-  // Encode Document.name
-  writer->WriteTag({PB_WT_STRING, google_firestore_v1beta1_Document_name_tag});
-  writer->WriteString(rpc_serializer_.EncodeKey(doc.key()));
+google_firestore_v1beta1_Document LocalSerializer::EncodeDocument(
+    const Document& doc) const {
+  google_firestore_v1beta1_Document result{};
+
+  result.name =
+      rpc_serializer_.EncodeString(rpc_serializer_.EncodeKey(doc.key()));
 
   // Encode Document.fields (unless it's empty)
-  const ObjectValue& object_value = doc.data().object_value();
-  if (!object_value.internal_value.empty()) {
-    rpc_serializer_.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);
+  size_t count = doc.data().object_value().internal_value.size();
+  result.fields_count = count;
+  result.fields =
+      MakeArray<google_firestore_v1beta1_Document_FieldsEntry>(count);
+  int i = 0;
+  for (const auto& kv : doc.data().object_value().internal_value) {
+    result.fields[i].key = rpc_serializer_.EncodeString(kv.first);
+    result.fields[i].value = rpc_serializer_.EncodeFieldValue(kv.second);
+    i++;
   }
 
-  // Encode Document.update_time
-  writer->WriteTag(
-      {PB_WT_STRING, google_firestore_v1beta1_Document_update_time_tag});
-  writer->WriteNestedMessage([&](Writer* writer) {
-    rpc_serializer_.EncodeVersion(writer, doc.version());
-  });
+  result.update_time = rpc_serializer_.EncodeVersion(doc.version());
 
   // Ignore Document.create_time. (We don't use this in our on-disk protos.)
+
+  return result;
 }
 
-void LocalSerializer::EncodeNoDocument(Writer* writer,
-                                       const 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());
-  });
+firestore_client_NoDocument LocalSerializer::EncodeNoDocument(
+    const NoDocument& no_doc) const {
+  firestore_client_NoDocument result{};
+
+  result.name =
+      rpc_serializer_.EncodeString(rpc_serializer_.EncodeKey(no_doc.key()));
+  result.read_time = rpc_serializer_.EncodeVersion(no_doc.version());
+
+  return result;
 }
 
 std::unique_ptr<NoDocument> LocalSerializer::DecodeNoDocument(
-    Reader* reader) const {
-  std::string name;
-  absl::optional<SnapshotVersion> version = SnapshotVersion::None();
-
-  while (reader->good()) {
-    switch (reader->ReadTag()) {
-      case firestore_client_NoDocument_name_tag:
-        name = reader->ReadString();
-        break;
-
-      case firestore_client_NoDocument_read_time_tag:
-        version = reader->ReadNestedMessage<SnapshotVersion>(
-            rpc_serializer_.DecodeSnapshotVersion);
-        break;
-
-      default:
-        reader->SkipUnknown();
-        break;
-    }
-  }
-
+    Reader* reader, const firestore_client_NoDocument& proto) const {
   if (!reader->status().ok()) return nullptr;
-  return absl::make_unique<NoDocument>(rpc_serializer_.DecodeKey(name),
-                                       *std::move(version));
-}
 
-void LocalSerializer::EncodeQueryData(Writer* writer,
-                                      const QueryData& query_data) const {
-  writer->WriteTag({PB_WT_VARINT, firestore_client_Target_target_id_tag});
-  writer->WriteInteger(query_data.target_id());
+  SnapshotVersion version =
+      rpc_serializer_.DecodeSnapshotVersion(reader, proto.read_time);
 
-  writer->WriteTag(
-      {PB_WT_VARINT, firestore_client_Target_last_listen_sequence_number_tag});
-  writer->WriteInteger(query_data.sequence_number());
+  if (!reader->status().ok()) return nullptr;
+  return absl::make_unique<NoDocument>(
+      rpc_serializer_.DecodeKey(reader,
+                                rpc_serializer_.DecodeString(proto.name)),
+      std::move(version));
+}
 
-  writer->WriteTag(
-      {PB_WT_STRING, firestore_client_Target_snapshot_version_tag});
-  writer->WriteNestedMessage([&](Writer* writer) {
-    rpc_serializer_.EncodeTimestamp(writer,
-                                    query_data.snapshot_version().timestamp());
-  });
+firestore_client_Target LocalSerializer::EncodeQueryData(
+    const QueryData& query_data) const {
+  firestore_client_Target result{};
 
-  writer->WriteTag({PB_WT_STRING, firestore_client_Target_resume_token_tag});
-  writer->WriteBytes(query_data.resume_token());
+  result.target_id = query_data.target_id();
+  result.last_listen_sequence_number = query_data.sequence_number();
+  result.snapshot_version = rpc_serializer_.EncodeTimestamp(
+      query_data.snapshot_version().timestamp());
+  result.resume_token = rpc_serializer_.EncodeBytes(query_data.resume_token());
 
   const Query& query = query_data.query();
   if (query.IsDocumentQuery()) {
     // TODO(rsgowman): Implement. Probably like this (once EncodeDocumentsTarget
     // exists):
     /*
-    writer->WriteTag({PB_WT_STRING, firestore_client_Target_documents_tag});
-    writer->WriteNestedMessage([&](Writer* writer) {
-      rpc_serializer_.EncodeDocumentsTarget(writer, query);
-    });
+    result.which_target_type = firestore_client_Target_document_tag;
+    result.documents = rpc_serializer_.EncodeDocumentsTarget(query);
     */
     abort();
   } else {
-    writer->WriteTag({PB_WT_STRING, firestore_client_Target_query_tag});
-    writer->WriteNestedMessage([&](Writer* writer) {
-      rpc_serializer_.EncodeQueryTarget(writer, query);
-    });
+    result.which_target_type = firestore_client_Target_query_tag;
+    result.query = rpc_serializer_.EncodeQueryTarget(query);
   }
+
+  return result;
 }
 
-absl::optional<QueryData> LocalSerializer::DecodeQueryData(
-    Reader* reader) const {
-  model::TargetId target_id = 0;
-  model::ListenSequenceNumber sequence_number = 0;
-  absl::optional<SnapshotVersion> version = SnapshotVersion::None();
-  std::vector<uint8_t> resume_token;
-  absl::optional<Query> query = Query::Invalid();
-
-  while (reader->good()) {
-    switch (reader->ReadTag()) {
-      case firestore_client_Target_target_id_tag:
-        // TODO(rsgowman): How to handle truncation of integer types?
-        target_id = static_cast<model::TargetId>(reader->ReadInteger());
-        break;
-
-      case firestore_client_Target_last_listen_sequence_number_tag:
-        // TODO(rsgowman): How to handle truncation of integer types?
-        sequence_number =
-            static_cast<model::ListenSequenceNumber>(reader->ReadInteger());
-        break;
-
-      case firestore_client_Target_snapshot_version_tag:
-        version = reader->ReadNestedMessage<SnapshotVersion>(
-            rpc_serializer_.DecodeSnapshotVersion);
-        break;
-
-      case firestore_client_Target_resume_token_tag:
-        resume_token = reader->ReadBytes();
-        break;
-
-      case firestore_client_Target_query_tag:
-        // TODO(rsgowman): Clear 'documents' field (since query and documents
-        // are part of a 'oneof').
-        query =
-            reader->ReadNestedMessage<Query>(rpc_serializer_.DecodeQueryTarget);
-        break;
-
-      case firestore_client_Target_documents_tag:
-        // Clear 'query' field (since query and documents are part of a 'oneof')
-        query = Query::Invalid();
-        // TODO(rsgowman): Implement.
-        abort();
-
-      default:
-        reader->SkipUnknown();
-        break;
-    }
+QueryData LocalSerializer::DecodeQueryData(
+    Reader* reader, const firestore_client_Target& proto) const {
+  if (!reader->status().ok()) return QueryData::Invalid();
+
+  model::TargetId target_id = proto.target_id;
+  // TODO(rgowman): How to handle truncation of integer types?
+  model::ListenSequenceNumber sequence_number = static_cast<model::ListenSequenceNumber>(proto.last_listen_sequence_number);
+  SnapshotVersion version =
+      rpc_serializer_.DecodeSnapshotVersion(reader, proto.snapshot_version);
+  std::vector<uint8_t> resume_token =
+      rpc_serializer_.DecodeBytes(proto.resume_token);
+  Query query = Query::Invalid();
+
+  switch (proto.which_target_type) {
+    case firestore_client_Target_query_tag:
+      query = rpc_serializer_.DecodeQueryTarget(reader, proto.query);
+      break;
+
+    case firestore_client_Target_documents_tag:
+      // TODO(rsgowman): Implement.
+      abort();
+
+    default:
+      reader->Fail(
+          StringFormat("Unknown target_type: %s", proto.which_target_type));
   }
 
-  if (!reader->status().ok()) return absl::nullopt;
-  return QueryData(*std::move(query), target_id, sequence_number,
-                   QueryPurpose::kListen, *std::move(version),
+  if (!reader->status().ok()) return QueryData::Invalid();
+  return QueryData(std::move(query), target_id, sequence_number,
+                   QueryPurpose::kListen, std::move(version),
                    std::move(resume_token));
 }