Serialize (and deserialize) int64 values (firebase#818) (firebase#829)

rsgowman · web-flow · commit 6ce954a791a7 · 2018-02-22T09:42:40.000-05:00
diff --git a/Firestore/core/src/firebase/firestore/model/field_value.cc b/Firestore/core/src/firebase/firestore/model/field_value.cc
@@ -45,9 +45,9 @@ namespace {
  */
 bool Comparable(Type lhs, Type rhs) {
   switch (lhs) {
-    case Type::Long:
+    case Type::Integer:
     case Type::Double:
-      return rhs == Type::Long || rhs == Type::Double;
+      return rhs == Type::Integer || rhs == Type::Double;
     case Type::Timestamp:
     case Type::ServerTimestamp:
       return rhs == Type::Timestamp || rhs == Type::ServerTimestamp;
@@ -78,7 +78,7 @@ FieldValue& FieldValue::operator=(const FieldValue& value) {
     case Type::Boolean:
       boolean_value_ = value.boolean_value_;
       break;
-    case Type::Long:
+    case Type::Integer:
       integer_value_ = value.integer_value_;
       break;
     case Type::Double:
@@ -179,7 +179,7 @@ const FieldValue& FieldValue::NanValue() {
 
 FieldValue FieldValue::IntegerValue(int64_t value) {
   FieldValue result;
-  result.SwitchTo(Type::Long);
+  result.SwitchTo(Type::Integer);
   result.integer_value_ = value;
   return result;
 }
@@ -304,8 +304,8 @@ bool operator<(const FieldValue& lhs, const FieldValue& rhs) {
       return false;
     case Type::Boolean:
       return Comparator<bool>()(lhs.boolean_value_, rhs.boolean_value_);
-    case Type::Long:
-      if (rhs.type() == Type::Long) {
+    case Type::Integer:
+      if (rhs.type() == Type::Integer) {
         return Comparator<int64_t>()(lhs.integer_value_, rhs.integer_value_);
       } else {
         return util::CompareMixedNumber(rhs.double_value_,
diff --git a/Firestore/core/src/firebase/firestore/model/field_value.h b/Firestore/core/src/firebase/firestore/model/field_value.h
@@ -28,6 +28,7 @@
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
 #include "Firestore/core/src/firebase/firestore/model/timestamp.h"
+#include "Firestore/core/src/firebase/firestore/util/firebase_assert.h"
 
 namespace firebase {
 namespace firestore {
@@ -63,7 +64,7 @@ class FieldValue {
   enum class Type {
     Null,     // Null
     Boolean,  // Boolean
-    Long,     // Number type starts here
+    Integer,  // Number type starts here
     Double,
     Timestamp,  // Timestamp type starts here
     ServerTimestamp,
@@ -95,6 +96,11 @@ class FieldValue {
     return tag_;
   }
 
+  int64_t integer_value() const {
+    FIREBASE_ASSERT(tag_ == Type::Integer);
+    return integer_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
@@ -98,6 +98,15 @@ bool DecodeBool(pb_istream_t* stream) {
   }
 }
 
+void EncodeInteger(pb_ostream_t* stream, int64_t integer_value) {
+  return EncodeVarint(stream, google_firestore_v1beta1_Value_integer_value_tag,
+                      integer_value);
+}
+
+int64_t DecodeInteger(pb_istream_t* stream) {
+  return DecodeVarint(stream);
+}
+
 }  // namespace
 
 using firebase::firestore::model::FieldValue;
@@ -118,6 +127,9 @@ Serializer::TypedValue Serializer::EncodeFieldValue(
         proto_value.value.boolean_value = false;
       }
       break;
+    case FieldValue::Type::Integer:
+      proto_value.value.integer_value = field_value.integer_value();
+      break;
     default:
       // TODO(rsgowman): implement the other types
       abort();
@@ -141,6 +153,10 @@ void Serializer::EncodeTypedValue(const TypedValue& value,
       EncodeBool(&stream, value.value.boolean_value);
       break;
 
+    case FieldValue::Type::Integer:
+      EncodeInteger(&stream, value.value.integer_value);
+      break;
+
     default:
       // TODO(rsgowman): implement the other types
       abort();
@@ -156,6 +172,8 @@ FieldValue Serializer::DecodeFieldValue(
       return FieldValue::NullValue();
     case FieldValue::Type::Boolean:
       return FieldValue::BooleanValue(value_proto.value.boolean_value);
+    case FieldValue::Type::Integer:
+      return FieldValue::IntegerValue(value_proto.value.integer_value);
     default:
       // TODO(rsgowman): implement the other types
       abort();
@@ -185,6 +203,10 @@ Serializer::TypedValue Serializer::DecodeTypedValue(const uint8_t* bytes,
       result.type = FieldValue::Type::Boolean;
       result.value.boolean_value = DecodeBool(&stream);
       break;
+    case google_firestore_v1beta1_Value_integer_value_tag:
+      result.type = FieldValue::Type::Integer;
+      result.value.integer_value = DecodeInteger(&stream);
+      break;
 
     default:
       // TODO(rsgowman): figure out error handling
@@ -209,6 +231,8 @@ bool operator==(const Serializer::TypedValue& lhs,
       return true;
     case FieldValue::Type::Boolean:
       return lhs.value.boolean_value == rhs.value.boolean_value;
+    case FieldValue::Type::Integer:
+      return lhs.value.integer_value == rhs.value.integer_value;
     default:
       // TODO(rsgowman): implement the other types
       abort();
diff --git a/Firestore/core/test/firebase/firestore/model/field_value_test.cc b/Firestore/core/test/firebase/firestore/model/field_value_test.cc
@@ -57,7 +57,7 @@ TEST(FieldValue, NumberType) {
   const FieldValue integer_value = FieldValue::IntegerValue(10L);
   const FieldValue double_value = FieldValue::DoubleValue(10.1);
   EXPECT_EQ(Type::Double, nan_value.type());
-  EXPECT_EQ(Type::Long, integer_value.type());
+  EXPECT_EQ(Type::Integer, integer_value.type());
   EXPECT_EQ(Type::Double, double_value.type());
   EXPECT_TRUE(nan_value < integer_value);
   EXPECT_TRUE(nan_value < double_value);
diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
@@ -38,6 +38,7 @@
 
 #include <pb.h>
 #include <pb_encode.h>
+#include <limits>
 #include <vector>
 
 #include "Firestore/core/src/firebase/firestore/model/field_value.h"
@@ -134,6 +135,76 @@ TEST_F(SerializerTest, EncodesBoolProtoToBytes) {
   }
 }
 
+TEST_F(SerializerTest, EncodesIntegersModelToProto) {
+  std::vector<int64_t> testCases{0,
+                                 1,
+                                 -1,
+                                 100,
+                                 -100,
+                                 std::numeric_limits<int64_t>::min(),
+                                 std::numeric_limits<int64_t>::max()};
+  for (int64_t test : testCases) {
+    FieldValue model = FieldValue::IntegerValue(test);
+    Serializer::TypedValue proto{FieldValue::Type::Integer,
+                                 google_firestore_v1beta1_Value_init_default};
+    proto.value.integer_value = test;
+    ExpectRoundTrip(model, proto, FieldValue::Type::Integer);
+  }
+}
+
+TEST_F(SerializerTest, EncodesIntegersProtoToBytes) {
+  // NB: because we're calculating the bytes via protoc (instead of
+  // libprotobuf) we have to look at values from numeric_limits<T> ahead of
+  // time. So these test cases make the following assumptions about
+  // numeric_limits: (linking libprotobuf is starting to sound like a better
+  // idea. :)
+  //   -9223372036854775808
+  //     == -8000000000000000
+  //     == numeric_limits<int64_t>::min()
+  //   9223372036854775807
+  //     == 7FFFFFFFFFFFFFFF
+  //     == numeric_limits<int64_t>::max()
+  //
+  // For now, lets at least verify these values:
+  EXPECT_EQ(-9223372036854775807 - 1, std::numeric_limits<int64_t>::min());
+  EXPECT_EQ(9223372036854775807, std::numeric_limits<int64_t>::max());
+
+  struct TestCase {
+    int64_t value;
+    std::vector<uint8_t> bytes;
+  };
+
+  std::vector<TestCase> cases{
+      // TEXT_FORMAT_PROTO: 'integer_value: 0'
+      TestCase{0, {0x10, 0x00}},
+      // TEXT_FORMAT_PROTO: 'integer_value: 1'
+      TestCase{1, {0x10, 0x01}},
+      // TEXT_FORMAT_PROTO: 'integer_value: -1'
+      TestCase{
+          -1,
+          {0x10, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01}},
+      // TEXT_FORMAT_PROTO: 'integer_value: 100'
+      TestCase{100, {0x10, 0x64}},
+      // TEXT_FORMAT_PROTO: 'integer_value: -100'
+      TestCase{
+          -100,
+          {0x10, 0x9c, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01}},
+      // TEXT_FORMAT_PROTO: 'integer_value: -9223372036854775808'
+      TestCase{
+          -9223372036854775807 - 1,
+          {0x10, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x01}},
+      // TEXT_FORMAT_PROTO: 'integer_value: 9223372036854775807'
+      TestCase{9223372036854775807,
+               {0x10, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f}}};
+
+  for (const TestCase& test : cases) {
+    Serializer::TypedValue proto{FieldValue::Type::Integer,
+                                 google_firestore_v1beta1_Value_init_default};
+    proto.value.integer_value = test.value;
+    ExpectRoundTrip(proto, test.bytes, FieldValue::Type::Integer);
+  }
+}
+
 // TODO(rsgowman): Test [en|de]coding multiple protos into the same output
 // vector.
 
