Add a flag to control whether DocumentSnapshots return Dates or Timestamps for timestamp fields (#831)

* add a new property `timestampsInSnapshotsEnabled` to `FirestoreSettings`, `false` by default;
* add a verbose warning message urging users to opt into the new behavior;
* set `timestampsInSnapshotsEnabled` to true in the integration tests to reduce the verbose console
spam during the test run and make sure the flag won't break anything once it's flipped.

2 files changed, 74 insertions, 43 deletions

diff --git a/Firestore/Source/Model/FSTFieldValue.h b/Firestore/Source/Model/FSTFieldValue.h
index 7d72138..6914f4d 100644
--- a/Firestore/Source/Model/FSTFieldValue.h
+++ b/Firestore/Source/Model/FSTFieldValue.h
@@ -55,6 +55,8 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) {
 
 @property(nonatomic, readonly, assign) FSTServerTimestampBehavior serverTimestampBehavior;
 
+@property(nonatomic) BOOL timestampsInSnapshotsEnabled;
+
 - (instancetype)init NS_UNAVAILABLE;
 
 /**
@@ -62,10 +64,12 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) {
  * server timestamps.
  */
 - (instancetype)initWithServerTimestampBehavior:(FSTServerTimestampBehavior)serverTimestampBehavior
+                   timestampsInSnapshotsEnabled:(BOOL)timestampsInSnapshotsEnabled
     NS_DESIGNATED_INITIALIZER;
 
-/** Creates an FSTFieldValueOption instance from FIRSnapshotOptions. */
-+ (instancetype)optionsForSnapshotOptions:(FIRSnapshotOptions *)value;
+/** Creates an FSTFieldValueOptions instance from FIRSnapshotOptions. */
++ (instancetype)optionsForSnapshotOptions:(FIRSnapshotOptions *)value
+             timestampsInSnapshotsEnabled:(BOOL)timestampsInSnapshotsEnabled;
 
 @end
 
@@ -163,9 +167,8 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) {
 /**
  * A timestamp value stored in Firestore.
  */
-@interface FSTTimestampValue : FSTFieldValue <NSDate *>
+@interface FSTTimestampValue : FSTFieldValue <FIRTimestamp *>
 + (instancetype)timestampValue:(FIRTimestamp *)value;
-- (FIRTimestamp *)internalValue;
 @end
 
 /**
@@ -194,7 +197,6 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) {
  */
 @interface FSTGeoPointValue : FSTFieldValue <FIRGeoPoint *>
 + (instancetype)geoPointValue:(FIRGeoPoint *)value;
-- (FIRGeoPoint *)valueWithOptions:(FSTFieldValueOptions *)options;
 @end
 
 /**
@@ -202,7 +204,6 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) {
  */
 @interface FSTBlobValue : FSTFieldValue <NSData *>
 + (instancetype)blobValue:(NSData *)value;
-- (NSData *)valueWithOptions:(FSTFieldValueOptions *)options;
 @end
 
 /**
@@ -211,7 +212,6 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) {
 @interface FSTReferenceValue : FSTFieldValue <FSTDocumentKey *>
 + (instancetype)referenceValue:(FSTDocumentKey *)value
                     databaseID:(const firebase::firestore::model::DatabaseId *)databaseID;
-- (FSTDocumentKey *)valueWithOptions:(FSTFieldValueOptions *)options;
 // Does not own this DatabaseId.
 @property(nonatomic, assign, readonly) const firebase::firestore::model::DatabaseId *databaseID;
 @end
@@ -239,7 +239,6 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) {
 - (instancetype)initWithImmutableDictionary:
     (FSTImmutableSortedDictionary<NSString *, FSTFieldValue *> *)value NS_DESIGNATED_INITIALIZER;
 
-- (NSDictionary<NSString *, id> *)valueWithOptions:(FSTFieldValueOptions *)options;
 - (FSTImmutableSortedDictionary<NSString *, FSTFieldValue *> *)internalValue;
 
 /** Returns the value at the given path if it exists. Returns nil otherwise. */
diff --git a/Firestore/Source/Model/FSTFieldValue.mm b/Firestore/Source/Model/FSTFieldValue.mm
index 2f013c3..80bd11f 100644
--- a/Firestore/Source/Model/FSTFieldValue.mm
+++ b/Firestore/Source/Model/FSTFieldValue.mm
@@ -48,28 +48,35 @@ NS_ASSUME_NONNULL_BEGIN
 
 @implementation FSTFieldValueOptions
 
-+ (instancetype)optionsForSnapshotOptions:(FIRSnapshotOptions *)options {
-  if (options.serverTimestampBehavior == FSTServerTimestampBehaviorNone) {
-    static FSTFieldValueOptions *defaultInstance = nil;
-    static dispatch_once_t onceToken;
-
-    dispatch_once(&onceToken, ^{
-      defaultInstance = [[FSTFieldValueOptions alloc]
-          initWithServerTimestampBehavior:FSTServerTimestampBehaviorNone];
-    });
-    return defaultInstance;
-  } else {
-    return [[FSTFieldValueOptions alloc]
-        initWithServerTimestampBehavior:options.serverTimestampBehavior];
++ (instancetype)optionsForSnapshotOptions:(FIRSnapshotOptions *)options
+             timestampsInSnapshotsEnabled:(BOOL)timestampsInSnapshotsEnabled {
+  FSTServerTimestampBehavior convertedServerTimestampBehavior = FSTServerTimestampBehaviorNone;
+  switch (options.serverTimestampBehavior) {
+    case FIRServerTimestampBehaviorNone:
+      convertedServerTimestampBehavior = FSTServerTimestampBehaviorNone;
+      break;
+    case FIRServerTimestampBehaviorEstimate:
+      convertedServerTimestampBehavior = FSTServerTimestampBehaviorEstimate;
+      break;
+    case FIRServerTimestampBehaviorPrevious:
+      convertedServerTimestampBehavior = FSTServerTimestampBehaviorPrevious;
+      break;
+    default:
+      FSTFail(@"Unexpected server timestamp option: %ld", (long)options.serverTimestampBehavior);
   }
+
+  return
+      [[FSTFieldValueOptions alloc] initWithServerTimestampBehavior:convertedServerTimestampBehavior
+                                       timestampsInSnapshotsEnabled:timestampsInSnapshotsEnabled];
 }
 
-- (instancetype)initWithServerTimestampBehavior:
-    (FSTServerTimestampBehavior)serverTimestampBehavior {
+- (instancetype)initWithServerTimestampBehavior:(FSTServerTimestampBehavior)serverTimestampBehavior
+                   timestampsInSnapshotsEnabled:(BOOL)timestampsInSnapshotsEnabled {
   self = [super init];
 
   if (self) {
     _serverTimestampBehavior = serverTimestampBehavior;
+    _timestampsInSnapshotsEnabled = timestampsInSnapshotsEnabled;
   }
   return self;
 }
@@ -89,12 +96,11 @@ NS_ASSUME_NONNULL_BEGIN
 }
 
 - (id)value {
-  return [self valueWithOptions:[FSTFieldValueOptions
-                                    optionsForSnapshotOptions:[FIRSnapshotOptions defaultOptions]]];
+  @throw FSTAbstractMethodException();  // NOLINT
 }
 
 - (id)valueWithOptions:(FSTFieldValueOptions *)options {
-  @throw FSTAbstractMethodException();  // NOLINT
+  return [self value];
 }
 
 - (BOOL)isEqual:(id)other {
@@ -143,7 +149,7 @@ NS_ASSUME_NONNULL_BEGIN
   return FSTTypeOrderNull;
 }
 
-- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+- (id)value {
   return [NSNull null];
 }
 
@@ -209,7 +215,7 @@ NS_ASSUME_NONNULL_BEGIN
   return FSTTypeOrderBoolean;
 }
 
-- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+- (id)value {
   return self.internalValue ? @YES : @NO;
 }
 
@@ -290,7 +296,7 @@ NS_ASSUME_NONNULL_BEGIN
   return self;
 }
 
-- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+- (id)value {
   return @(self.internalValue);
 }
 
@@ -342,7 +348,7 @@ NS_ASSUME_NONNULL_BEGIN
   return self;
 }
 
-- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+- (id)value {
   return @(self.internalValue);
 }
 
@@ -400,7 +406,7 @@ struct Comparator<NSString *> {
   return FSTTypeOrderString;
 }
 
-- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+- (id)value {
   return self.internalValue;
 }
 
@@ -447,9 +453,16 @@ struct Comparator<NSString *> {
   return FSTTypeOrderTimestamp;
 }
 
+- (id)value {
+  return self.internalValue;
+}
+
 - (id)valueWithOptions:(FSTFieldValueOptions *)options {
-  // For developers, we expose Timestamps as Dates.
-  return self.internalValue.approximateDateValue;
+  if (options.timestampsInSnapshotsEnabled) {
+    return self.value;
+  } else {
+    return [self.value dateValue];
+  }
 }
 
 - (BOOL)isEqual:(id)other {
@@ -473,7 +486,6 @@ struct Comparator<NSString *> {
 }
 
 @end
-
 #pragma mark - FSTServerTimestampValue
 
 @implementation FSTServerTimestampValue
@@ -498,16 +510,20 @@ struct Comparator<NSString *> {
   return FSTTypeOrderTimestamp;
 }
 
+- (id)value {
+  return [NSNull null];
+}
+
 - (id)valueWithOptions:(FSTFieldValueOptions *)options {
   switch (options.serverTimestampBehavior) {
     case FSTServerTimestampBehaviorNone:
       return [NSNull null];
     case FSTServerTimestampBehaviorEstimate:
-      return [self.localWriteTime approximateDateValue];
+      return [[FSTTimestampValue timestampValue:self.localWriteTime] valueWithOptions:options];
     case FSTServerTimestampBehaviorPrevious:
       return self.previousValue ? [self.previousValue valueWithOptions:options] : [NSNull null];
     default:
-      FSTFail(@"Unexpected server timestamp option: %d", (int)options.serverTimestampBehavior);
+      FSTFail(@"Unexpected server timestamp option: %ld", (long)options.serverTimestampBehavior);
   }
 }
 
@@ -561,7 +577,7 @@ struct Comparator<NSString *> {
   return FSTTypeOrderGeoPoint;
 }
 
-- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+- (id)value {
   return self.internalValue;
 }
 
@@ -583,7 +599,6 @@ struct Comparator<NSString *> {
 }
 
 @end
-
 #pragma mark - FSTBlobValue
 
 static NSComparisonResult CompareBytes(NSData *left, NSData *right) {
@@ -625,7 +640,7 @@ static NSComparisonResult CompareBytes(NSData *left, NSData *right) {
   return FSTTypeOrderBlob;
 }
 
-- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+- (id)value {
   return self.internalValue;
 }
 
@@ -669,7 +684,7 @@ static NSComparisonResult CompareBytes(NSData *left, NSData *right) {
   return self;
 }
 
-- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+- (id)value {
   return self.key;
 }
 
@@ -753,6 +768,15 @@ static const NSComparator StringComparator = ^NSComparisonResult(NSString *left,
   return [self initWithImmutableDictionary:dictionary];
 }
 
+- (id)value {
+  NSMutableDictionary *result = [NSMutableDictionary dictionary];
+  [self.internalValue
+      enumerateKeysAndObjectsUsingBlock:^(NSString *key, FSTFieldValue *obj, BOOL *stop) {
+        result[key] = [obj value];
+      }];
+  return result;
+}
+
 - (id)valueWithOptions:(FSTFieldValueOptions *)options {
   NSMutableDictionary *result = [NSMutableDictionary dictionary];
   [self.internalValue
@@ -832,8 +856,8 @@ static const NSComparator StringComparator = ^NSComparisonResult(NSString *left,
     // Recursive base case:
     return [self objectBySettingValue:value forField:childName];
   } else {
-    // Nested path. Recursively generate a new sub-object and then wrap a new FSTObjectValue around
-    // the result.
+    // Nested path. Recursively generate a new sub-object and then wrap a new FSTObjectValue
+    // around the result.
     FSTFieldValue *child = [_internalValue objectForKey:childName];
     FSTObjectValue *childObject;
     if ([child isKindOfClass:[FSTObjectValue class]]) {
@@ -908,7 +932,7 @@ static const NSComparator StringComparator = ^NSComparisonResult(NSString *left,
   return [self.internalValue hash];
 }
 
-- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+- (id)value {
   NSMutableArray *result = [NSMutableArray arrayWithCapacity:_internalValue.count];
   [self.internalValue enumerateObjectsUsingBlock:^(FSTFieldValue *obj, NSUInteger idx, BOOL *stop) {
     [result addObject:[obj value]];
@@ -916,6 +940,14 @@ static const NSComparator StringComparator = ^NSComparisonResult(NSString *left,
   return result;
 }
 
+- (id)valueWithOptions:(FSTFieldValueOptions *)options {
+  NSMutableArray *result = [NSMutableArray arrayWithCapacity:_internalValue.count];
+  [self.internalValue enumerateObjectsUsingBlock:^(FSTFieldValue *obj, NSUInteger idx, BOOL *stop) {
+    [result addObject:[obj valueWithOptions:options]];
+  }];
+  return result;
+}
+
 - (FSTTypeOrder)typeOrder {
   return FSTTypeOrderArray;
 }