[De]serialize Timestamps via the remote Serializer (#1233)

Author: rsgowman

Firestore/core/src/firebase/firestore/CMakeLists.txt

@@ -18,6 +18,7 @@ cc_library(
   SOURCES
     geo_point.cc
     timestamp.cc
+    timestamp_internal.h
   DEPENDS
     firebase_firestore_util
 )

Firestore/core/src/firebase/firestore/remote/serializer.cc

@@ -25,13 +25,17 @@
 #include <utility>
 
 #include "Firestore/Protos/nanopb/google/firestore/v1beta1/document.pb.h"
+#include "Firestore/core/include/firebase/firestore/timestamp.h"
 #include "Firestore/core/src/firebase/firestore/model/resource_path.h"
+#include "Firestore/core/src/firebase/firestore/timestamp_internal.h"
 #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h"
 
 namespace firebase {
 namespace firestore {
 namespace remote {
 
+using firebase::Timestamp;
+using firebase::TimestampInternal;
 using firebase::firestore::model::DatabaseId;
 using firebase::firestore::model::DocumentKey;
 using firebase::firestore::model::FieldValue;
@@ -95,6 +99,15 @@ class Writer {
    */
   void WriteTag(Tag tag);
 
+  /**
+   * Writes a nanopb message to the output stream.
+   *
+   * This essentially wraps calls to nanopb's `pb_encode()` method. If we didn't
+   * use `oneof`s in our protos, this would be the primary way of encoding
+   * messages.
+   */
+  void WriteNanopbMessage(const pb_field_t fields[], const void* src_struct);
+
   void WriteSize(size_t size);
   void WriteNull();
   void WriteBool(bool bool_value);
@@ -179,6 +192,15 @@ class Reader {
    */
   Tag ReadTag();
 
+  /**
+   * Reads a nanopb message from the input stream.
+   *
+   * This essentially wraps calls to nanopb's pb_decode() method. If we didn't
+   * use `oneof`s in our protos, this would be the primary way of decoding
+   * messages.
+   */
+  void ReadNanopbMessage(const pb_field_t fields[], void* dest_struct);
+
   void ReadNull();
   bool ReadBool();
   int64_t ReadInteger();
@@ -299,6 +321,23 @@ Tag Reader::ReadTag() {
   return tag;
 }
 
+void Writer::WriteNanopbMessage(const pb_field_t fields[],
+                                const void* src_struct) {
+  if (!status_.ok()) return;
+
+  if (!pb_encode(&stream_, fields, src_struct)) {
+    FIREBASE_ASSERT_MESSAGE(false, PB_GET_ERROR(&stream_));
+  }
+}
+
+void Reader::ReadNanopbMessage(const pb_field_t fields[], void* dest_struct) {
+  if (!status_.ok()) return;
+
+  if (!pb_decode(&stream_, fields, dest_struct)) {
+    status_ = Status(FirestoreErrorCode::DataLoss, PB_GET_ERROR(&stream_));
+  }
+}
+
 void Writer::WriteSize(size_t size) {
   return WriteVarint(size);
 }
@@ -415,6 +454,43 @@ std::string Reader::ReadString() {
   return result;
 }
 
+void EncodeTimestamp(Writer* writer, const Timestamp& timestamp_value) {
+  google_protobuf_Timestamp timestamp_proto =
+      google_protobuf_Timestamp_init_zero;
+  timestamp_proto.seconds = timestamp_value.seconds();
+  timestamp_proto.nanos = timestamp_value.nanoseconds();
+  writer->WriteNanopbMessage(google_protobuf_Timestamp_fields,
+                             &timestamp_proto);
+}
+
+Timestamp DecodeTimestamp(Reader* reader) {
+  google_protobuf_Timestamp timestamp_proto =
+      google_protobuf_Timestamp_init_zero;
+  reader->ReadNanopbMessage(google_protobuf_Timestamp_fields, &timestamp_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};
+}
+
 // Named '..Impl' so as to not conflict with Serializer::EncodeFieldValue.
 // TODO(rsgowman): Refactor to use a helper class that wraps the stream struct.
 // This will help with error handling, and should eliminate the issue of two
@@ -447,6 +523,14 @@ void EncodeFieldValueImpl(Writer* writer, const FieldValue& field_value) {
       writer->WriteString(field_value.string_value());
       break;
 
+    case FieldValue::Type::Timestamp:
+      writer->WriteTag(
+          {PB_WT_STRING, google_firestore_v1beta1_Value_timestamp_value_tag});
+      writer->WriteNestedMessage([&field_value](Writer* writer) {
+        EncodeTimestamp(writer, field_value.timestamp_value());
+      });
+      break;
+
     case FieldValue::Type::Object:
       writer->WriteTag(
           {PB_WT_STRING, google_firestore_v1beta1_Value_map_value_tag});
@@ -477,6 +561,7 @@ FieldValue DecodeFieldValueImpl(Reader* reader) {
       break;
 
     case google_firestore_v1beta1_Value_string_value_tag:
+    case google_firestore_v1beta1_Value_timestamp_value_tag:
     case google_firestore_v1beta1_Value_map_value_tag:
       if (tag.wire_type != PB_WT_STRING) {
         reader->set_status(
@@ -517,6 +602,9 @@ FieldValue DecodeFieldValueImpl(Reader* reader) {
       return FieldValue::IntegerValue(reader->ReadInteger());
     case google_firestore_v1beta1_Value_string_value_tag:
       return FieldValue::StringValue(reader->ReadString());
+    case google_firestore_v1beta1_Value_timestamp_value_tag:
+      return FieldValue::TimestampValue(
+          reader->ReadNestedMessage<Timestamp>(DecodeTimestamp));
     case google_firestore_v1beta1_Value_map_value_tag:
       return FieldValue::ObjectValueFromMap(DecodeObject(reader));
 

Firestore/core/src/firebase/firestore/timestamp_internal.h

@@ -0,0 +1,49 @@
+/*
+ * Copyright 2018 Google
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_TIMESTAMP_INTERNAL_H_
+#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_TIMESTAMP_INTERNAL_H_
+
+#include "Firestore/core/include/firebase/firestore/timestamp.h"
+
+namespace firebase {
+
+/**
+ * Details about the Timestamp class which are useful internally, but we don't
+ * want to expose publicly.
+ */
+class TimestampInternal {
+ public:
+  /**
+   * Represents the maximum allowable time that the Timestamp class handles,
+   * specifically 9999-12-31T23:59:59.999999999Z.
+   */
+  static firebase::Timestamp Max() {
+    return {253402300800L - 1, 999999999};
+  }
+
+  /**
+   * Represents the minimum allowable time that the Timestamp class handles,
+   * specifically 0001-01-01T00:00:00Z.
+   */
+  static firebase::Timestamp Min() {
+    return {-62135596800L, 0};
+  }
+};
+
+}  // namespace firebase
+
+#endif  // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_TIMESTAMP_INTERNAL_H_

Firestore/core/test/firebase/firestore/remote/serializer_test.cc

@@ -34,14 +34,18 @@
 
 #include "Firestore/Protos/cpp/google/firestore/v1beta1/document.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/field_value.h"
+#include "Firestore/core/src/firebase/firestore/timestamp_internal.h"
 #include "Firestore/core/src/firebase/firestore/util/status.h"
 #include "Firestore/core/src/firebase/firestore/util/statusor.h"
 #include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
 #include "google/protobuf/stubs/common.h"
 #include "google/protobuf/util/message_differencer.h"
 #include "gtest/gtest.h"
 
+using firebase::Timestamp;
+using firebase::TimestampInternal;
 using firebase::firestore::FirestoreErrorCode;
 using firebase::firestore::model::DatabaseId;
 using firebase::firestore::model::FieldValue;
@@ -179,6 +183,15 @@ class SerializerTest : public ::testing::Test {
     return proto;
   }
 
+  google::firestore::v1beta1::Value ValueProto(const Timestamp& ts) {
+    std::vector<uint8_t> bytes =
+        EncodeFieldValue(&serializer, FieldValue::TimestampValue(ts));
+    google::firestore::v1beta1::Value proto;
+    bool ok = proto.ParseFromArray(bytes.data(), bytes.size());
+    EXPECT_TRUE(ok);
+    return proto;
+  }
+
  private:
   void ExpectSerializationRoundTrip(
       const FieldValue& model,
@@ -259,6 +272,23 @@ TEST_F(SerializerTest, EncodesString) {
   }
 }
 
+TEST_F(SerializerTest, EncodesTimestamps) {
+  std::vector<Timestamp> cases{
+      {},  // epoch
+      {1234, 0},
+      {1234, 999999999},
+      {-1234, 0},
+      {-1234, 999999999},
+      TimestampInternal::Max(),
+      TimestampInternal::Min(),
+  };
+
+  for (const Timestamp& ts_value : cases) {
+    FieldValue model = FieldValue::TimestampValue(ts_value);
+    ExpectRoundTrip(model, ValueProto(ts_value), FieldValue::Type::Timestamp);
+  }
+}
+
 TEST_F(SerializerTest, EncodesEmptyMap) {
   FieldValue model = FieldValue::ObjectValueFromMap({});
 
@@ -366,6 +396,28 @@ TEST_F(SerializerTest, BadStringValue) {
       Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
 }
 
+TEST_F(SerializerTest, BadTimestampValue_TooLarge) {
+  std::vector<uint8_t> bytes = EncodeFieldValue(
+      &serializer, FieldValue::TimestampValue(TimestampInternal::Max()));
+
+  // Add some time, which should push us above the maximum allowed timestamp.
+  Mutate(&bytes[4], 0x82, 0x83);
+
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
+TEST_F(SerializerTest, BadTimestampValue_TooSmall) {
+  std::vector<uint8_t> bytes = EncodeFieldValue(
+      &serializer, FieldValue::TimestampValue(TimestampInternal::Min()));
+
+  // Remove some time, which should push us below the minimum allowed timestamp.
+  Mutate(&bytes[4], 0x92, 0x91);
+
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
 TEST_F(SerializerTest, BadTag) {
   std::vector<uint8_t> bytes =
       EncodeFieldValue(&serializer, FieldValue::NullValue());