Serialize (and deserialize) string values (#864)

3 files changed, 122 insertions, 1 deletions

diff --git a/Firestore/core/src/firebase/firestore/model/field_value.h b/Firestore/core/src/firebase/firestore/model/field_value.h
index 72b9481..cb219f5 100644
--- a/Firestore/core/src/firebase/firestore/model/field_value.h
+++ b/Firestore/core/src/firebase/firestore/model/field_value.h
@@ -106,6 +106,11 @@ class FieldValue {
     return integer_value_;
   }
 
+  const std::string& string_value() const {
+    FIREBASE_ASSERT(tag_ == Type::String);
+    return string_value_;
+  }
+
   /** factory methods. */
   static const FieldValue& NullValue();
   static const FieldValue& TrueValue();
diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc
index 79ed98d..b7ab891 100644
--- a/Firestore/core/src/firebase/firestore/remote/serializer.cc
+++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc
@@ -19,6 +19,8 @@
 #include <pb_decode.h>
 #include <pb_encode.h>
 
+#include <string>
+
 namespace firebase {
 namespace firestore {
 namespace remote {
@@ -98,6 +100,47 @@ int64_t DecodeInteger(pb_istream_t* stream) {
   return DecodeVarint(stream);
 }
 
+void EncodeString(pb_ostream_t* stream, const std::string& string_value) {
+  bool status = pb_encode_string(
+      stream, reinterpret_cast<const pb_byte_t*>(string_value.c_str()),
+      string_value.length());
+  if (!status) {
+    // TODO(rsgowman): figure out error handling
+    abort();
+  }
+}
+
+std::string DecodeString(pb_istream_t* stream) {
+  pb_istream_t substream;
+  bool status = pb_make_string_substream(stream, &substream);
+  if (!status) {
+    // TODO(rsgowman): figure out error handling
+    abort();
+  }
+
+  std::string result(substream.bytes_left, '\0');
+  status = pb_read(&substream, reinterpret_cast<pb_byte_t*>(&result[0]),
+                   substream.bytes_left);
+  if (!status) {
+    // TODO(rsgowman): figure out error handling
+    abort();
+  }
+
+  // NB: future versions of nanopb read the remaining characters out of the
+  // substream (and return false if that fails) as an additional safety
+  // check within pb_close_string_substream. Unfortunately, that's not present
+  // in the current version (0.38).  We'll make a stronger assertion and check
+  // to make sure there *are* no remaining characters in the substream.
+  if (substream.bytes_left != 0) {
+    // TODO(rsgowman): figure out error handling
+    abort();
+  }
+
+  pb_close_string_substream(stream, &substream);
+
+  return result;
+}
+
 }  // namespace
 
 using firebase::firestore::model::FieldValue;
@@ -144,6 +187,16 @@ void Serializer::EncodeFieldValue(const FieldValue& field_value,
       EncodeInteger(&stream, field_value.integer_value());
       break;
 
+    case FieldValue::Type::String:
+      status = pb_encode_tag(&stream, PB_WT_STRING,
+                             google_firestore_v1beta1_Value_string_value_tag);
+      if (!status) {
+        // TODO(rsgowman): figure out error handling
+        abort();
+      }
+      EncodeString(&stream, field_value.string_value());
+      break;
+
     default:
       // TODO(rsgowman): implement the other types
       abort();
@@ -158,11 +211,32 @@ FieldValue Serializer::DecodeFieldValue(const uint8_t* bytes, size_t length) {
   uint32_t tag;
   bool eof;
   bool status = pb_decode_tag(&stream, &wire_type, &tag, &eof);
-  if (!status || wire_type != PB_WT_VARINT) {
+  if (!status) {
     // TODO(rsgowman): figure out error handling
     abort();
   }
 
+  // Ensure the tag matches the wire type
+  // TODO(rsgowman): figure out error handling
+  switch (tag) {
+    case google_firestore_v1beta1_Value_null_value_tag:
+    case google_firestore_v1beta1_Value_boolean_value_tag:
+    case google_firestore_v1beta1_Value_integer_value_tag:
+      if (wire_type != PB_WT_VARINT) {
+        abort();
+      }
+      break;
+
+    case google_firestore_v1beta1_Value_string_value_tag:
+      if (wire_type != PB_WT_STRING) {
+        abort();
+      }
+      break;
+
+    default:
+      abort();
+  }
+
   switch (tag) {
     case google_firestore_v1beta1_Value_null_value_tag:
       DecodeNull(&stream);
@@ -171,6 +245,8 @@ FieldValue Serializer::DecodeFieldValue(const uint8_t* bytes, size_t length) {
       return FieldValue::BooleanValue(DecodeBool(&stream));
     case google_firestore_v1beta1_Value_integer_value_tag:
       return FieldValue::IntegerValue(DecodeInteger(&stream));
+    case google_firestore_v1beta1_Value_string_value_tag:
+      return FieldValue::StringValue(DecodeString(&stream));
 
     default:
       // TODO(rsgowman): figure out error handling
diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
index 9ce60f5..8f89064 100644
--- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
+++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
@@ -151,6 +151,46 @@ TEST_F(SerializerTest, EncodesIntegersModelToBytes) {
   }
 }
 
+TEST_F(SerializerTest, EncodesStringModelToBytes) {
+  struct TestCase {
+    std::string value;
+    std::vector<uint8_t> bytes;
+  };
+
+  std::vector<TestCase> cases{
+      // TEXT_FORMAT_PROTO: 'string_value: ""'
+      {"", {0x8a, 0x01, 0x00}},
+      // TEXT_FORMAT_PROTO: 'string_value: "a"'
+      {"a", {0x8a, 0x01, 0x01, 0x61}},
+      // TEXT_FORMAT_PROTO: 'string_value: "abc def"'
+      {"abc def", {0x8a, 0x01, 0x07, 0x61, 0x62, 0x63, 0x20, 0x64, 0x65, 0x66}},
+      // TEXT_FORMAT_PROTO: 'string_value: "æ"'
+      {"æ", {0x8a, 0x01, 0x02, 0xc3, 0xa6}},
+      // TEXT_FORMAT_PROTO: 'string_value: "\0\ud7ff\ue000\uffff"'
+      // Note: Each one of the three embedded universal character names
+      // (\u-escaped) maps to three chars, so the total length of the string
+      // literal is 10 (ignoring the terminating null), and the resulting string
+      // literal is the same as '\0\xed\x9f\xbf\xee\x80\x80\xef\xbf\xbf'". The
+      // size of 10 must be added, or else std::string will see the \0 at the
+      // start and assume that's the end of the string.
+      {{"\0\ud7ff\ue000\uffff", 10},
+       {0x8a, 0x01, 0x0a, 0x00, 0xed, 0x9f, 0xbf, 0xee, 0x80, 0x80, 0xef, 0xbf,
+        0xbf}},
+      {{"\0\xed\x9f\xbf\xee\x80\x80\xef\xbf\xbf", 10},
+       {0x8a, 0x01, 0x0a, 0x00, 0xed, 0x9f, 0xbf, 0xee, 0x80, 0x80, 0xef, 0xbf,
+        0xbf}},
+      // TEXT_FORMAT_PROTO: 'string_value: "(╯°□°）╯︵ ┻━┻"'
+      {"(╯°□°）╯︵ ┻━┻",
+       {0x8a, 0x01, 0x1e, 0x28, 0xe2, 0x95, 0xaf, 0xc2, 0xb0, 0xe2, 0x96,
+        0xa1, 0xc2, 0xb0, 0xef, 0xbc, 0x89, 0xe2, 0x95, 0xaf, 0xef, 0xb8,
+        0xb5, 0x20, 0xe2, 0x94, 0xbb, 0xe2, 0x94, 0x81, 0xe2, 0x94, 0xbb}}};
+
+  for (const TestCase& test : cases) {
+    FieldValue model = FieldValue::StringValue(test.value);
+    ExpectRoundTrip(model, test.bytes, FieldValue::Type::String);
+  }
+}
+
 // TODO(rsgowman): Test [en|de]coding multiple protos into the same output
 // vector.