Migrate FSTServerTimestampValue to FSTDelegateValue (#3174)

* Migrate FSTServerTimetsampValue to FSTDelegateValue

* Make ServerTimestamp use FSTFieldValue for previous value

This makes it possible integrate the FSTServerTimestampValue changes
earlier.

Author: wilhuff

FirebaseFirestore.podspec

@@ -25,7 +25,7 @@ Google Cloud Firestore is a NoSQL document database built for automatic scaling,
   s.prefix_header_file = false
 
   s.source_files = [
-    'Firestore/Source/**/*',
+    'Firestore/Source/**/*.{h,m,mm}',
     'Firestore/Protos/nanopb/**/*.{h,cc}',
     'Firestore/Protos/objc/**/*.[hm]',
     'Firestore/core/include/**/*.{h,cc,mm}',

Firestore/CMakeLists.txt

@@ -13,6 +13,9 @@
 # limitations under the License.
 
 add_subdirectory(Example/Benchmarks)
+add_subdirectory(Source)
+add_subdirectory(third_party/Immutable)
+
 add_subdirectory(Protos)
 add_subdirectory(core)
 add_subdirectory(fuzzing)

Firestore/Example/Tests/Model/FSTFieldValueTests.mm

@@ -51,13 +51,12 @@
       // Server Timestamp values can't be parsed directly, so we have a couple predefined sentinel
       // strings that can be used instead.
       if ([value isEqual:@"server-timestamp-1"]) {
-        wrappedValue = [FSTServerTimestampValue
-            serverTimestampValueWithLocalWriteTime:testutil::MakeTimestamp(2016, 5, 20, 10, 20, 0)
-                                     previousValue:nil];
+        wrappedValue =
+            FieldValue::FromServerTimestamp(testutil::MakeTimestamp(2016, 5, 20, 10, 20, 0)).Wrap();
       } else if ([value isEqual:@"server-timestamp-2"]) {
-        wrappedValue = [FSTServerTimestampValue
-            serverTimestampValueWithLocalWriteTime:testutil::MakeTimestamp(2016, 10, 21, 15, 32, 0)
-                                     previousValue:nil];
+        wrappedValue =
+            FieldValue::FromServerTimestamp(testutil::MakeTimestamp(2016, 10, 21, 15, 32, 0))
+                .Wrap();
       } else if ([value isKindOfClass:[FSTDocumentKeyReference class]]) {
         // We directly convert these here so that the databaseIDs can be different.
         FSTDocumentKeyReference *reference = (FSTDocumentKeyReference *)value;
@@ -311,13 +310,10 @@ - (void)testValueEquality {
     @[ FSTTestFieldValue(date2) ],
     @[
       // NOTE: ServerTimestampValues can't be parsed via FSTTestFieldValue().
-      [FSTServerTimestampValue serverTimestampValueWithLocalWriteTime:MakeTimestamp(date1)
-                                                        previousValue:nil],
-      [FSTServerTimestampValue serverTimestampValueWithLocalWriteTime:MakeTimestamp(date1)
-                                                        previousValue:nil]
+      FieldValue::FromServerTimestamp(MakeTimestamp(date1)).Wrap(),
+      FieldValue::FromServerTimestamp(MakeTimestamp(date1)).Wrap()
     ],
-    @[ [FSTServerTimestampValue serverTimestampValueWithLocalWriteTime:MakeTimestamp(date2)
-                                                         previousValue:nil] ],
+    @[ FieldValue::FromServerTimestamp(MakeTimestamp(date2)).Wrap() ],
     @[ FSTTestFieldValue(FSTTestGeoPoint(0, 1)), FieldValue::FromGeoPoint(GeoPoint(0, 1)).Wrap() ],
     @[ FSTTestFieldValue(FSTTestGeoPoint(1, 0)) ],
     @[

Firestore/Example/Tests/Model/FSTMutationTests.mm

@@ -144,9 +144,7 @@ - (void)testAppliesLocalServerTimestampTransformToDocuments {
   FSTObjectValue *expectedData =
       FSTTestObjectValue(@{@"foo" : @{@"bar" : @"<server-timestamp>"}, @"baz" : @"baz-value"});
   expectedData =
-      [expectedData objectBySettingValue:[FSTServerTimestampValue
-                                             serverTimestampValueWithLocalWriteTime:_timestamp
-                                                                      previousValue:nil]
+      [expectedData objectBySettingValue:FieldValue::FromServerTimestamp(_timestamp).Wrap()
                                  forPath:testutil::Field("foo.bar")];
 
   FSTDocument *expectedDoc = [FSTDocument documentWithData:expectedData

Firestore/Source/API/CMakeLists.txt

@@ -0,0 +1,40 @@
+# Copyright 2019 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.
+
+# Public types to be used both internally and externally.
+if(APPLE)
+  cc_library(
+    firebase_firestore_objc_api
+    SOURCES
+      FIRGeoPoint+Internal.h
+      FIRGeoPoint.mm
+      FIRTimestamp.m
+      FIRTimestamp+Internal.h
+      converters.h
+      converters.mm
+    DEPENDS
+      firebase_firestore_api
+      firebase_firestore_util
+  )
+
+  target_include_directories(
+    firebase_firestore_objc_api
+    PUBLIC ${PROJECT_SOURCE_DIR}/Firestore/Source/Public
+  )
+
+  target_compile_options(
+    firebase_firestore_objc_api PRIVATE
+    -Wno-unused-parameter
+  )
+endif()

Firestore/Source/API/FIRFieldPath.mm

@@ -48,13 +48,13 @@ - (instancetype)initWithFields:(NSArray<NSString *> *)fieldNames {
     ThrowInvalidArgument("Invalid field path. Provided names must not be empty.");
   }
 
-  std::vector<std::string> field_names;
-  field_names.reserve(fieldNames.count);
-  for (int i = 0; i < fieldNames.count; ++i) {
-    field_names.emplace_back(util::MakeString(fieldNames[i]));
+  std::vector<std::string> converted;
+  converted.reserve(fieldNames.count);
+  for (NSString *fieldName in fieldNames) {
+    converted.emplace_back(util::MakeString(fieldName));
   }
 
-  return [self initPrivate:FieldPath::FromSegments(std::move(field_names))];
+  return [self initPrivate:FieldPath::FromSegments(std::move(converted))];
 }
 
 + (instancetype)documentID {

Firestore/Source/API/FIRFieldValue.mm

@@ -130,8 +130,7 @@ - (instancetype)initWithOperand:(NSNumber *)operand;
 @end
 
 @implementation FSTNumericIncrementFieldValue
-- (instancetype)initWithOperand:(NSNumber *)operand;
-{
+- (instancetype)initWithOperand:(NSNumber *)operand {
   if (self = [super initPrivate]) {
     _operand = operand;
   }

Firestore/Source/CMakeLists.txt

@@ -0,0 +1,16 @@
+# Copyright 2019 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.
+
+add_subdirectory(API)
+add_subdirectory(Model)

Firestore/Source/Model/CMakeLists.txt

@@ -0,0 +1,40 @@
+# Copyright 2019 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.
+
+if(APPLE)
+  cc_library(
+    firebase_firestore_objc_model
+    SOURCES
+      FSTDocument.h
+      FSTDocument.mm
+      FSTDocumentKey.h
+      FSTDocumentKey.mm
+      FSTFieldValue.h
+      FSTFieldValue.mm
+      FSTMutation.h
+      FSTMutation.mm
+      FSTMutationBatch.h
+      FSTMutationBatch.mm
+    DEPENDS
+      firebase_firestore_model
+      firebase_firestore_objc_api
+      firebase_firestore_objc_immutable
+      firebase_firestore_util
+  )
+
+  target_compile_options(
+    firebase_firestore_objc_model PRIVATE
+    -Wno-unused-parameter
+  )
+endif()

Firestore/Source/Model/FSTDocumentKey.mm

@@ -19,8 +19,6 @@
 #include <string>
 #include <utility>
 
-#import "Firestore/Source/Core/FSTFirestoreClient.h"
-
 #include "Firestore/core/src/firebase/firestore/model/document_key.h"
 #include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
 #include "Firestore/core/src/firebase/firestore/util/hashing.h"

Firestore/Source/Model/FSTFieldValue.h

@@ -103,27 +103,6 @@ typedef NS_ENUM(NSInteger, FSTTypeOrder) {
 
 @end
 
-/**
- * Represents a locally-applied Server Timestamp.
- *
- * Notes:
- * - FSTServerTimestampValue instances are created as the result of applying an FSTTransformMutation
- *   (see [FSTTransformMutation applyTo]). They can only exist in the local view of a document.
- *   Therefore they do not need to be parsed or serialized.
- * - When evaluated locally (e.g. via FSTDocumentSnapshot data), they by default evaluate to NSNull.
- *   This behavior can be configured by passing custom FieldValueOptions to `valueWithOptions:`.
- * - They sort after all Timestamp values. With respect to other FSTServerTimestampValues, they
- *   sort by their localWriteTime.
- */
-@interface FSTServerTimestampValue : FSTFieldValue <id>
-+ (instancetype)serverTimestampValueWithLocalWriteTime:(const firebase::Timestamp &)localWriteTime
-                                         previousValue:(nullable FSTFieldValue *)previousValue;
-
-@property(nonatomic, assign, readonly) const firebase::Timestamp &localWriteTime;
-@property(nonatomic, strong, readonly, nullable) FSTFieldValue *previousValue;
-
-@end
-
 /**
  * A reference value stored in Firestore.
  */

Firestore/Source/Model/FSTFieldValue.mm

@@ -128,81 +128,6 @@ - (double)doubleValue {
 
 @end
 
-#pragma mark - FSTServerTimestampValue
-
-@implementation FSTServerTimestampValue {
-  Timestamp _localWriteTime;
-}
-
-+ (instancetype)serverTimestampValueWithLocalWriteTime:(const Timestamp &)localWriteTime
-                                         previousValue:(nullable FSTFieldValue *)previousValue {
-  return [[FSTServerTimestampValue alloc] initWithLocalWriteTime:localWriteTime
-                                                   previousValue:previousValue];
-}
-
-- (id)initWithLocalWriteTime:(const Timestamp &)localWriteTime
-               previousValue:(nullable FSTFieldValue *)previousValue {
-  self = [super init];
-  if (self) {
-    _localWriteTime = localWriteTime;
-    _previousValue = previousValue;
-  }
-  return self;
-}
-
-- (FieldValue::Type)type {
-  return FieldValue::Type::ServerTimestamp;
-}
-
-- (FSTTypeOrder)typeOrder {
-  return FSTTypeOrderTimestamp;
-}
-
-- (id)value {
-  return [NSNull null];
-}
-
-- (id)valueWithOptions:(const FieldValueOptions &)options {
-  switch (options.server_timestamp_behavior()) {
-    case ServerTimestampBehavior::kNone:
-      return [NSNull null];
-    case ServerTimestampBehavior::kEstimate:
-      return [FieldValue::FromTimestamp(self.localWriteTime).Wrap() valueWithOptions:options];
-    case ServerTimestampBehavior::kPrevious:
-      return self.previousValue ? [self.previousValue valueWithOptions:options] : [NSNull null];
-    default:
-      HARD_FAIL("Unexpected server timestamp option: %s", options.server_timestamp_behavior());
-  }
-}
-
-- (BOOL)isEqual:(id)other {
-  return [other isKindOfClass:[FSTFieldValue class]] &&
-         ((FSTFieldValue *)other).type == FieldValue::Type::ServerTimestamp &&
-         self.localWriteTime == ((FSTServerTimestampValue *)other).localWriteTime;
-}
-
-- (NSUInteger)hash {
-  return TimestampInternal::Hash(self.localWriteTime);
-}
-
-- (NSString *)description {
-  return [NSString
-      stringWithFormat:@"<ServerTimestamp localTime=%s>", self.localWriteTime.ToString().c_str()];
-}
-
-- (NSComparisonResult)compare:(FSTFieldValue *)other {
-  if (other.type == FieldValue::Type::ServerTimestamp) {
-    return WrapCompare(self.localWriteTime, ((FSTServerTimestampValue *)other).localWriteTime);
-  } else if (other.type == FieldValue::Type::Timestamp) {
-    // Server timestamps come after all concrete timestamps.
-    return NSOrderedDescending;
-  } else {
-    return [self defaultCompare:other];
-  }
-}
-
-@end
-
 #pragma mark - FSTReferenceValue
 
 @interface FSTReferenceValue ()
@@ -617,20 +542,19 @@ - (BOOL)isEqual:(id)other {
 - (id)value {
   switch (self.internalValue.type()) {
     case FieldValue::Type::Null:
+      // NSDictionary disallows storing `nil` values. Use NSNull as an
+      // explicitly existing null value.
       return [NSNull null];
     case FieldValue::Type::Boolean:
       return self.internalValue.boolean_value() ? @YES : @NO;
     case FieldValue::Type::Integer:
       return @(self.internalValue.integer_value());
     case FieldValue::Type::Double:
       return @(self.internalValue.double_value());
-    case FieldValue::Type::Timestamp: {
-      auto timestamp = self.internalValue.timestamp_value();
-      return [[FIRTimestamp alloc] initWithSeconds:timestamp.seconds()
-                                       nanoseconds:timestamp.nanoseconds()];
-    }
+    case FieldValue::Type::Timestamp:
+      return MakeFIRTimestamp(self.internalValue.timestamp_value());
     case FieldValue::Type::ServerTimestamp:
-      HARD_FAIL("TODO(rsgowman): implement");
+      return [NSNull null];
     case FieldValue::Type::String:
       return util::WrapNSString(self.internalValue.string_value());
     case FieldValue::Type::Blob:
@@ -655,6 +579,22 @@ - (id)valueWithOptions:(const model::FieldValueOptions &)options {
         return [[self value] dateValue];
       }
 
+    case FieldValue::Type::ServerTimestamp: {
+      const auto &sts = self.internalValue.server_timestamp_value();
+      switch (options.server_timestamp_behavior()) {
+        case ServerTimestampBehavior::kNone:
+          return [NSNull null];
+        case ServerTimestampBehavior::kEstimate:
+          return
+              [FieldValue::FromTimestamp(sts.local_write_time()).Wrap() valueWithOptions:options];
+        case ServerTimestampBehavior::kPrevious:
+          return sts.previous_value() ? [sts.previous_value() valueWithOptions:options]
+                                      : [NSNull null];
+        default:
+          HARD_FAIL("Unexpected server timestamp option: %s", options.server_timestamp_behavior());
+      }
+    }
+
     default:
       return [self value];
   }
@@ -669,14 +609,8 @@ - (NSComparisonResult)compare:(FSTFieldValue *)other {
   // FSTDelegateValue handles (eg) booleans to ensure this case never occurs.
 
   if (FieldValue::Comparable(self.type, other.type)) {
-    if ([other isKindOfClass:[FSTServerTimestampValue class]]) {
-      HARD_ASSERT(self.type == FieldValue::Type::Timestamp);
-      // Server timestamps come after all concrete timestamps.
-      return NSOrderedAscending;
-    } else {
-      HARD_ASSERT([other isKindOfClass:[FSTDelegateValue class]]);
-      return WrapCompare<FieldValue>(self.internalValue, ((FSTDelegateValue *)other).internalValue);
-    }
+    HARD_ASSERT([other isKindOfClass:[FSTDelegateValue class]]);
+    return WrapCompare<FieldValue>(self.internalValue, ((FSTDelegateValue *)other).internalValue);
   } else {
     return [self defaultCompare:other];
   }

Firestore/Source/Model/FSTMutation.mm

@@ -481,7 +481,7 @@ - (FSTMaybeDocument *)applyToRemoteDocument:(nullable FSTMaybeDocument *)maybeDo
  *
  * @param baseDocument The document prior to applying this mutation batch.
  * @param localWriteTime The local time of the transform mutation (used to generate
- * FSTServerTimestampValues).
+ *     ServerTimestampValues).
  * @return The transform results array.
  */
 - (NSArray<FSTFieldValue *> *)

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

@@ -51,5 +51,6 @@ cc_library(
     absl_meta
     firebase_firestore_api_input_validation
     firebase_firestore_core
+    firebase_firestore_model
     firebase_firestore_util
 )