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.

4 files changed, 46 insertions, 4 deletions

diff --git a/Firestore/Source/API/FIRDocumentSnapshot.mm b/Firestore/Source/API/FIRDocumentSnapshot.mm
index 39d6af1..0fd59f4 100644
--- a/Firestore/Source/API/FIRDocumentSnapshot.mm
+++ b/Firestore/Source/API/FIRDocumentSnapshot.mm
@@ -18,6 +18,8 @@
 
 #include <utility>
 
+#import "FIRFirestoreSettings.h"
+
 #import "Firestore/Source/API/FIRDocumentReference+Internal.h"
 #import "Firestore/Source/API/FIRFieldPath+Internal.h"
 #import "Firestore/Source/API/FIRFirestore+Internal.h"
@@ -149,7 +151,10 @@ NS_ASSUME_NONNULL_BEGIN
   return self.internalDocument == nil
              ? nil
              : [self convertedObject:[self.internalDocument data]
-                             options:[FSTFieldValueOptions optionsForSnapshotOptions:options]];
+                             options:[FSTFieldValueOptions
+                                            optionsForSnapshotOptions:options
+                                         timestampsInSnapshotsEnabled:
+                                             self.firestore.settings.timestampsInSnapshotsEnabled]];
 }
 
 - (nullable id)valueForField:(id)field {
@@ -171,7 +176,10 @@ NS_ASSUME_NONNULL_BEGIN
   return fieldValue == nil
              ? nil
              : [self convertedValue:fieldValue
-                            options:[FSTFieldValueOptions optionsForSnapshotOptions:options]];
+                            options:[FSTFieldValueOptions
+                                           optionsForSnapshotOptions:options
+                                        timestampsInSnapshotsEnabled:
+                                            self.firestore.settings.timestampsInSnapshotsEnabled]];
 }
 
 - (nullable id)objectForKeyedSubscript:(id)key {
diff --git a/Firestore/Source/API/FIRFirestore.mm b/Firestore/Source/API/FIRFirestore.mm
index f04bd8b..45d67cf 100644
--- a/Firestore/Source/API/FIRFirestore.mm
+++ b/Firestore/Source/API/FIRFirestore.mm
@@ -242,6 +242,34 @@ extern "C" NSString *const FIRFirestoreErrorDomain = @"FIRFirestoreErrorDomain";
       FSTAssert(_settings.host, @"FirestoreSettings.host cannot be nil.");
       FSTAssert(_settings.dispatchQueue, @"FirestoreSettings.dispatchQueue cannot be nil.");
 
+      if (!_settings.timestampsInSnapshotsEnabled) {
+        FSTWarn(
+            @"The behavior for system Date objects stored in Firestore is going to change "
+             "AND YOUR APP MAY BREAK.\n"
+             "To hide this warning and ensure your app does not break, you need to add "
+             "the following code to your app before calling any other Cloud Firestore methods:\n"
+             "\n"
+             "let db = Firestore.firestore()\n"
+             "let settings = db.settings\n"
+             "settings.areTimestampsInSnapshotsEnabled = true\n"
+             "db.settings = settings\n"
+             "\n"
+             "With this change, timestamps stored in Cloud Firestore will be read back as "
+             "Firebase Timestamp objects instead of as system Date objects. So you will "
+             "also need to update code expecting a Date to instead expect a Timestamp. "
+             "For example:\n"
+             "\n"
+             "// old:\n"
+             "let date: Date = documentSnapshot.get(\"created_at\") as! Date\n"
+             "// new:\n"
+             "let timestamp: Timestamp = documentSnapshot.get(\"created_at\") as! Timestamp\n"
+             "let date: Date = timestamp.dateValue()\n"
+             "\n"
+             "Please audit all existing usages of Date when you enable the new behavior. In a "
+             "future release, the behavior will be changed to the new behavior, so if you do not "
+             "follow these steps, YOUR APP MAY BREAK.");
+      }
+
       const DatabaseInfo database_info(*self.databaseID, util::MakeStringView(_persistenceKey),
                                        util::MakeStringView(_settings.host), _settings.sslEnabled);
 
diff --git a/Firestore/Source/API/FIRFirestoreSettings.mm b/Firestore/Source/API/FIRFirestoreSettings.mm
index 9677ff6..8f998ec 100644
--- a/Firestore/Source/API/FIRFirestoreSettings.mm
+++ b/Firestore/Source/API/FIRFirestoreSettings.mm
@@ -23,6 +23,8 @@ NS_ASSUME_NONNULL_BEGIN
 static NSString *const kDefaultHost = @"firestore.googleapis.com";
 static const BOOL kDefaultSSLEnabled = YES;
 static const BOOL kDefaultPersistenceEnabled = YES;
+// TODO(b/73820332): flip the default.
+static const BOOL kDefaultTimestampsInSnapshotsEnabled = NO;
 
 @implementation FIRFirestoreSettings
 
@@ -32,6 +34,7 @@ static const BOOL kDefaultPersistenceEnabled = YES;
     _sslEnabled = kDefaultSSLEnabled;
     _dispatchQueue = dispatch_get_main_queue();
     _persistenceEnabled = kDefaultPersistenceEnabled;
+    _timestampsInSnapshotsEnabled = kDefaultTimestampsInSnapshotsEnabled;
   }
   return self;
 }
@@ -47,7 +50,8 @@ static const BOOL kDefaultPersistenceEnabled = YES;
   return [self.host isEqual:otherSettings.host] &&
          self.isSSLEnabled == otherSettings.isSSLEnabled &&
          self.dispatchQueue == otherSettings.dispatchQueue &&
-         self.isPersistenceEnabled == otherSettings.isPersistenceEnabled;
+         self.isPersistenceEnabled == otherSettings.isPersistenceEnabled &&
+         self.timestampsInSnapshotsEnabled == otherSettings.timestampsInSnapshotsEnabled;
 }
 
 - (NSUInteger)hash {
@@ -55,6 +59,7 @@ static const BOOL kDefaultPersistenceEnabled = YES;
   result = 31 * result + (self.isSSLEnabled ? 1231 : 1237);
   // Ignore the dispatchQueue to avoid having to deal with sizeof(dispatch_queue_t).
   result = 31 * result + (self.isPersistenceEnabled ? 1231 : 1237);
+  result = 31 * result + (self.timestampsInSnapshotsEnabled ? 1231 : 1237);
   return result;
 }
 
@@ -64,6 +69,7 @@ static const BOOL kDefaultPersistenceEnabled = YES;
   copy.sslEnabled = _sslEnabled;
   copy.dispatchQueue = _dispatchQueue;
   copy.persistenceEnabled = _persistenceEnabled;
+  copy.timestampsInSnapshotsEnabled = _timestampsInSnapshotsEnabled;
   return copy;
 }
 
diff --git a/Firestore/Source/API/FIRTimestamp.m b/Firestore/Source/API/FIRTimestamp.m
index 489b921..e5a2cd3 100644
--- a/Firestore/Source/API/FIRTimestamp.m
+++ b/Firestore/Source/API/FIRTimestamp.m
@@ -121,7 +121,7 @@ static const int kNanosPerSecond = 1000000000;
 
 #pragma mark - Public methods
 
-- (NSDate *)approximateDateValue {
+- (NSDate *)dateValue {
   NSTimeInterval interval = (NSTimeInterval)self.seconds + ((NSTimeInterval)self.nanoseconds) / 1e9;
   return [NSDate dateWithTimeIntervalSince1970:interval];
 }