diff options
author | Konstantin Varlamov <var-const@users.noreply.github.com> | 2018-03-30 14:50:15 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-03-30 14:50:15 -0400 |
commit | a17740e9146e4e2431d62964d044287cccc3ee85 (patch) | |
tree | 479f280a07f4200924e0ef7cce183aa35e14c5bb | |
parent | 653aea7b50247bb0f6a7e8e1b4ab782553849f74 (diff) |
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.
18 files changed, 264 insertions, 91 deletions
diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index c0fa621..a0493d7 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -2,6 +2,16 @@ - [fixed] Fixed a regression in the Firebase iOS SDK release 4.11.0 that could cause `getDocument()` requests made while offline to be delayed by up to 10 seconds (rather than returning from cache immediately). +- [feature] Added a new `Timestamp` class to represent timestamp fields, + currently supporting up to microsecond precision. It can be passed to API + methods anywhere a system Date is currently accepted. To make + `DocumentSnapshot`s read timestamp fields back as `Timestamp`s instead of + Dates, you can set the newly added property `areTimestampsInSnapshotsEnabled` + in `FirestoreSettings` to `true`. Note that the current behavior + (`DocumentSnapshot`s returning system Dates) will be removed in a future + release. Using `Timestamp`s avoids rounding errors (system Date is stored as + a floating-point value, so the value read back from a `DocumentSnapshot` + might be slightly different from the value written). # v0.10.4 - [changed] If the SDK's attempt to connect to the Cloud Firestore backend diff --git a/Firestore/Example/SwiftBuildTest/main.swift b/Firestore/Example/SwiftBuildTest/main.swift index cd2462b..3087085 100644 --- a/Firestore/Example/SwiftBuildTest/main.swift +++ b/Firestore/Example/SwiftBuildTest/main.swift @@ -52,6 +52,7 @@ func initializeDb() -> Firestore { let settings = FirestoreSettings() settings.host = "localhost" settings.isPersistenceEnabled = true + settings.areTimestampsInSnapshotsEnabled = true firestore.settings = settings return firestore @@ -333,6 +334,7 @@ func types() { let _: Firestore let _: FirestoreSettings let _: GeoPoint + let _: Timestamp let _: ListenerRegistration let _: QueryListenOptions let _: Query diff --git a/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm b/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm index 751e7ff..312c3ff 100644 --- a/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm @@ -181,7 +181,7 @@ FIRDocumentSnapshot *document = [self readDocumentForRef:doc]; XCTAssertEqual(document[@"updated"], @NO); - XCTAssertTrue([document[@"time"] isKindOfClass:[NSDate class]]); + XCTAssertTrue([document[@"time"] isKindOfClass:[FIRTimestamp class]]); } - (void)testCanDeleteFieldUsingMerge { diff --git a/Firestore/Example/Tests/Integration/API/FIRFieldsTests.mm b/Firestore/Example/Tests/Integration/API/FIRFieldsTests.mm index 0e75b8e..30db5e3 100644 --- a/Firestore/Example/Tests/Integration/API/FIRFieldsTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRFieldsTests.mm @@ -26,6 +26,10 @@ @interface FIRFieldsTests : FSTIntegrationTestCase @end +NSDictionary<NSString *, id> *testDataWithTimestamps(FIRTimestamp *timestamp) { + return @{ @"timestamp" : timestamp, @"nested" : @{@"timestamp2" : timestamp} }; +} + @implementation FIRFieldsTests - (NSDictionary<NSString *, id> *)testNestedDataNumbered:(int)number { @@ -221,28 +225,83 @@ [self awaitExpectations]; } -- (NSDictionary<NSString *, id> *)testDataWithTimestamp:(FIRTimestamp *)timestamp { - return @{ - @"timestamp" : [timestamp approximateDateValue], - @"metadata" : @{@"nestedTimestamp" : [timestamp approximateDateValue]} - }; +- (FIRDocumentSnapshot *)snapshotWithTimestamps:(FIRTimestamp *)timestamp { + FIRDocumentReference *doc = [self documentRef]; + NSDictionary<NSString *, id> *data = + @{ @"timestamp" : timestamp, + @"nested" : @{@"timestamp2" : timestamp} }; + [self writeDocumentRef:doc data:data]; + return [self readDocumentForRef:doc]; } -// This test should break once the default for how timestamps are returned changes. -- (void)testThatDataContainsNativeDateType { - NSDate *date = [NSDate date]; - FIRTimestamp *timestamp = [FIRTimestamp timestampWithDate:date]; +// Note: timestampsInSnapshotsEnabled is set to "true" in FSTIntegrationTestCase, so this test is +// not affected by the current default in FIRFirestoreSettings. +- (void)testTimestampsInSnapshots { + FIRTimestamp *originalTimestamp = [FIRTimestamp timestampWithSeconds:100 nanoseconds:123456789]; FIRDocumentReference *doc = [self documentRef]; - [self writeDocumentRef:doc data:[self testDataWithTimestamp:timestamp]]; + [self writeDocumentRef:doc data:testDataWithTimestamps(originalTimestamp)]; + + FIRDocumentSnapshot *snapshot = [self readDocumentForRef:doc]; + NSDictionary<NSString *, id> *data = [snapshot data]; + // Timestamp are currently truncated to microseconds after being written to the database. + FIRTimestamp *truncatedTimestamp = + [FIRTimestamp timestampWithSeconds:originalTimestamp.seconds + nanoseconds:originalTimestamp.nanoseconds / 1000 * 1000]; + + FIRTimestamp *timestampFromSnapshot = snapshot[@"timestamp"]; + FIRTimestamp *timestampFromData = data[@"timestamp"]; + XCTAssertEqualObjects(truncatedTimestamp, timestampFromData); + XCTAssertEqualObjects(timestampFromSnapshot, timestampFromData); + + timestampFromSnapshot = snapshot[@"nested.timestamp2"]; + timestampFromData = data[@"nested"][@"timestamp2"]; + XCTAssertEqualObjects(truncatedTimestamp, timestampFromData); + XCTAssertEqualObjects(timestampFromSnapshot, timestampFromData); +} +@end - FIRDocumentSnapshot *result = [self readDocumentForRef:doc]; - NSDate *resultDate = result.data[@"timestamp"]; - XCTAssertEqualWithAccuracy([resultDate timeIntervalSince1970], [date timeIntervalSince1970], - 0.000001); - XCTAssertEqualObjects(result.data[@"timestamp"], resultDate); - NSDate *resultNestedDate = result[@"metadata.nestedTimestamp"]; - XCTAssertEqualWithAccuracy([resultNestedDate timeIntervalSince1970], [date timeIntervalSince1970], - 0.000001); +@interface FIRTimestampsInSnapshotsLegacyBehaviorTests : FSTIntegrationTestCase +@end + +@implementation FIRTimestampsInSnapshotsLegacyBehaviorTests + +- (void)setUp { + [super setUp]; + // Settings can only be redefined before client is initialized, so this has to happen in setUp. + FIRFirestoreSettings *settings = self.db.settings; + settings.timestampsInSnapshotsEnabled = NO; + self.db.settings = settings; +} + +- (void)testLegacyBehaviorForTimestampFields { + NSDate *originalDate = [NSDate date]; + FIRDocumentReference *doc = [self documentRef]; + [self writeDocumentRef:doc + data:testDataWithTimestamps([FIRTimestamp timestampWithDate:originalDate])]; + FIRDocumentSnapshot *snapshot = [self readDocumentForRef:doc]; + NSDictionary<NSString *, id> *data = [snapshot data]; + double microsecond = 0.000001; + + NSDate *timestampFromSnapshot = snapshot[@"timestamp"]; + NSDate *timestampFromData = data[@"timestamp"]; + XCTAssertEqualObjects(timestampFromSnapshot, timestampFromData); + XCTAssertEqualWithAccuracy([timestampFromSnapshot timeIntervalSince1970], + [originalDate timeIntervalSince1970], microsecond); + + timestampFromSnapshot = snapshot[@"nested.timestamp2"]; + timestampFromData = data[@"nested"][@"timestamp2"]; + XCTAssertEqualObjects(timestampFromSnapshot, timestampFromData); + XCTAssertEqualWithAccuracy([timestampFromSnapshot timeIntervalSince1970], + [originalDate timeIntervalSince1970], microsecond); +} + +- (void)testLegacyBehaviorForServerTimestampFields { + FIRDocumentReference *doc = [self documentRef]; + [self writeDocumentRef:doc data:@{@"when" : [FIRFieldValue fieldValueForServerTimestamp]}]; + + FIRDocumentSnapshot *snapshot = [self readDocumentForRef:doc]; + XCTAssertTrue([snapshot[@"when"] isKindOfClass:[NSDate class]]); + XCTAssertTrue([snapshot.data[@"when"] isKindOfClass:[NSDate class]]); } @end diff --git a/Firestore/Example/Tests/Integration/API/FIRServerTimestampTests.mm b/Firestore/Example/Tests/Integration/API/FIRServerTimestampTests.mm index 916ce7e..4d51434 100644 --- a/Firestore/Example/Tests/Integration/API/FIRServerTimestampTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRServerTimestampTests.mm @@ -125,7 +125,7 @@ /** Verifies a snapshot containing _setData but with a local estimate for the timestamps. */ - (void)verifyTimestampsAreEstimatedInSnapshot:(FIRDocumentSnapshot *)snapshot { id timestamp = [snapshot valueForField:@"when" options:_returnEstimatedValue]; - XCTAssertTrue([timestamp isKindOfClass:[NSDate class]]); + XCTAssertTrue([timestamp isKindOfClass:[FIRTimestamp class]]); XCTAssertEqualObjects([snapshot dataWithOptions:_returnEstimatedValue], [self expectedDataWithTimestamp:timestamp]); } @@ -148,10 +148,10 @@ /** Verifies a snapshot containing _setData but with resolved server timestamps. */ - (void)verifySnapshotWithResolvedTimestamps:(FIRDocumentSnapshot *)snapshot { XCTAssertTrue(snapshot.exists); - NSDate *when = snapshot[@"when"]; - XCTAssertTrue([when isKindOfClass:[NSDate class]]); + FIRTimestamp *when = snapshot[@"when"]; + XCTAssertTrue([when isKindOfClass:[FIRTimestamp class]]); // Tolerate up to 10 seconds of clock skew between client and server. - XCTAssertEqualWithAccuracy(when.timeIntervalSinceNow, 0, 10); + XCTAssertEqualWithAccuracy(when.seconds, [FIRTimestamp timestamp].seconds, 10); // Validate the rest of the document. XCTAssertEqualObjects(snapshot.data, [self expectedDataWithTimestamp:when]); @@ -213,14 +213,14 @@ XCTAssertEqualObjects([localSnapshot valueForField:@"a"], [NSNull null]); XCTAssertEqualObjects([localSnapshot valueForField:@"a" options:_returnPreviousValue], @42); XCTAssertTrue([[localSnapshot valueForField:@"a" options:_returnEstimatedValue] - isKindOfClass:[NSDate class]]); + isKindOfClass:[FIRTimestamp class]]); FIRDocumentSnapshot *remoteSnapshot = [self waitForRemoteEvent]; - XCTAssertTrue([[remoteSnapshot valueForField:@"a"] isKindOfClass:[NSDate class]]); + XCTAssertTrue([[remoteSnapshot valueForField:@"a"] isKindOfClass:[FIRTimestamp class]]); XCTAssertTrue([[remoteSnapshot valueForField:@"a" options:_returnPreviousValue] - isKindOfClass:[NSDate class]]); + isKindOfClass:[FIRTimestamp class]]); XCTAssertTrue([[remoteSnapshot valueForField:@"a" options:_returnEstimatedValue] - isKindOfClass:[NSDate class]]); + isKindOfClass:[FIRTimestamp class]]); } - (void)testServerTimestampsWithConsecutiveUpdates { @@ -241,7 +241,7 @@ [self enableNetwork]; FIRDocumentSnapshot *remoteSnapshot = [self waitForRemoteEvent]; - XCTAssertTrue([[remoteSnapshot valueForField:@"a"] isKindOfClass:[NSDate class]]); + XCTAssertTrue([[remoteSnapshot valueForField:@"a"] isKindOfClass:[FIRTimestamp class]]); } - (void)testServerTimestampsPreviousValueFromLocalMutation { @@ -266,7 +266,7 @@ [self enableNetwork]; FIRDocumentSnapshot *remoteSnapshot = [self waitForRemoteEvent]; - XCTAssertTrue([[remoteSnapshot valueForField:@"a"] isKindOfClass:[NSDate class]]); + XCTAssertTrue([[remoteSnapshot valueForField:@"a"] isKindOfClass:[FIRTimestamp class]]); } - (void)testServerTimestampsWorkViaTransactionSet { diff --git a/Firestore/Example/Tests/Integration/API/FIRTypeTests.mm b/Firestore/Example/Tests/Integration/API/FIRTypeTests.mm index 5140b90..740cde0 100644 --- a/Firestore/Example/Tests/Integration/API/FIRTypeTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRTypeTests.mm @@ -56,9 +56,22 @@ }]; } -- (void)testCanReadAndWriteTimestampFields { +- (void)testCanReadAndWriteDateFields { // Choose a value that can be converted losslessly between fixed point and double - NSDate *timestamp = [NSDate dateWithTimeIntervalSince1970:1491847082.125]; + NSDate *date = [NSDate dateWithTimeIntervalSince1970:1491847082.125]; + + // NSDates are read back as FIRTimestamps, so assertSuccessfulRoundtrip cannot be used here. + FIRDocumentReference *doc = [self.db documentWithPath:@"rooms/eros"]; + [self writeDocumentRef:doc data:@{@"date" : date}]; + FIRDocumentSnapshot *document = [self readDocumentForRef:doc]; + XCTAssertTrue(document.exists); + XCTAssertEqualObjects(document.data, @{@"date" : [FIRTimestamp timestampWithDate:date]}); +} + +- (void)testCanReadAndWriteTimestampFields { + // Timestamps are currently truncated to microseconds on the backend, so only be precise to + // microseconds to ensure the value read back is exactly the same. + FIRTimestamp *timestamp = [FIRTimestamp timestampWithSeconds:123456 nanoseconds:123456000]; [self assertSuccessfulRoundtrip:@{@"timestamp" : timestamp}]; } diff --git a/Firestore/Example/Tests/Model/FSTFieldValueTests.mm b/Firestore/Example/Tests/Model/FSTFieldValueTests.mm index 1a207f4..98504b5 100644 --- a/Firestore/Example/Tests/Model/FSTFieldValueTests.mm +++ b/Firestore/Example/Tests/Model/FSTFieldValueTests.mm @@ -227,10 +227,8 @@ union DoubleBits { for (id value in values) { FSTFieldValue *wrapped = FSTTestFieldValue(value); XCTAssertEqualObjects([wrapped class], [FSTTimestampValue class]); - XCTAssertEqualObjects([wrapped value], value); - - XCTAssertEqualObjects(((FSTTimestampValue *)wrapped).internalValue, - [FIRTimestamp timestampWithDate:value]); + XCTAssertEqualObjects([[wrapped value] class], [FIRTimestamp class]); + XCTAssertEqualObjects([wrapped value], [FIRTimestamp timestampWithDate:value]); } } @@ -572,14 +570,14 @@ union DoubleBits { FSTObjectValue *value = FSTTestObjectValue(input); id output = [value value]; { - XCTAssertTrue([output[@"array"][1] isKindOfClass:[NSDate class]]); - NSDate *actual = output[@"array"][1]; - XCTAssertEqualWithAccuracy(date.timeIntervalSince1970, actual.timeIntervalSince1970, 0.000001); + XCTAssertTrue([output[@"array"][1] isKindOfClass:[FIRTimestamp class]]); + FIRTimestamp *actual = output[@"array"][1]; + XCTAssertEqualObjects([FIRTimestamp timestampWithDate:date], actual); } { - XCTAssertTrue([output[@"obj"][@"date"] isKindOfClass:[NSDate class]]); - NSDate *actual = output[@"obj"][@"date"]; - XCTAssertEqualWithAccuracy(date.timeIntervalSince1970, actual.timeIntervalSince1970, 0.000001); + XCTAssertTrue([output[@"obj"][@"date"] isKindOfClass:[FIRTimestamp class]]); + FIRTimestamp *actual = output[@"array"][1]; + XCTAssertEqualObjects([FIRTimestamp timestampWithDate:date], actual); } } diff --git a/Firestore/Example/Tests/Model/FSTMutationTests.mm b/Firestore/Example/Tests/Model/FSTMutationTests.mm index 40ded40..1f9193e 100644 --- a/Firestore/Example/Tests/Model/FSTMutationTests.mm +++ b/Firestore/Example/Tests/Model/FSTMutationTests.mm @@ -143,7 +143,7 @@ using firebase::firestore::model::DocumentKey; mutationResult:mutationResult]; NSDictionary *expectedData = - @{ @"foo" : @{@"bar" : _timestamp.approximateDateValue}, + @{ @"foo" : @{@"bar" : _timestamp.dateValue}, @"baz" : @"baz-value" }; XCTAssertEqualObjects(transformedDoc, FSTTestDoc("collection/key", 0, expectedData, NO)); } diff --git a/Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm b/Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm index 059f257..611bcc8 100644 --- a/Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm +++ b/Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm @@ -129,6 +129,7 @@ NS_ASSUME_NONNULL_BEGIN } settings.host = host; settings.persistenceEnabled = YES; + settings.timestampsInSnapshotsEnabled = YES; NSLog(@"Configured integration test for %@ with SSL: %@", settings.host, settings.sslEnabled ? @"YES" : @"NO"); return settings; 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]; } 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; } diff --git a/Firestore/Source/Public/FIRFirestoreSettings.h b/Firestore/Source/Public/FIRFirestoreSettings.h index 7a1f2a3..cd3f91c 100644 --- a/Firestore/Source/Public/FIRFirestoreSettings.h +++ b/Firestore/Source/Public/FIRFirestoreSettings.h @@ -44,6 +44,23 @@ NS_SWIFT_NAME(FirestoreSettings) /** Set to false to disable local persistent storage. */ @property(nonatomic, getter=isPersistenceEnabled) BOOL persistenceEnabled; +/** + * Enables the use of FIRTimestamps for timestamp fields in FIRDocumentSnapshots. + * + * Currently, Firestore returns timestamp fields as an NSDate but NSDate is implemented as a double + * which loses precision and causes unexpected behavior when using a timestamp from a snapshot as + * a part of a subsequent query. + * + * Setting timestampsInSnapshotsEnabled to true will cause Firestore to return FIRTimestamp values + * instead of NSDate, avoiding this kind of problem. To make this work you must also change any code + * that uses NSDate to use FIRTimestamp instead. + * + * NOTE: in the future timestampsInSnapshotsEnabled = true will become the default and this option + * will be removed so you should change your code to use FIRTimestamp now and opt-in to this new + * behavior as soon as you can. + */ +@property(nonatomic, getter=areTimestampsInSnapshotsEnabled) BOOL timestampsInSnapshotsEnabled; + @end NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Public/FIRTimestamp.h b/Firestore/Source/Public/FIRTimestamp.h index d0a77f3..bf4aff4 100644 --- a/Firestore/Source/Public/FIRTimestamp.h +++ b/Firestore/Source/Public/FIRTimestamp.h @@ -60,7 +60,7 @@ NS_SWIFT_NAME(Timestamp) + (instancetype)timestamp; /** Returns a new NSDate corresponding to this timestamp. This may lose precision. */ -- (NSDate *)approximateDateValue; +- (NSDate *)dateValue; - (NSComparisonResult)compare:(FIRTimestamp *)other; diff --git a/Firestore/Source/Remote/FSTSerializerBeta.mm b/Firestore/Source/Remote/FSTSerializerBeta.mm index 9531ddc..3a22a3f 100644 --- a/Firestore/Source/Remote/FSTSerializerBeta.mm +++ b/Firestore/Source/Remote/FSTSerializerBeta.mm @@ -191,7 +191,7 @@ NS_ASSUME_NONNULL_BEGIN return [self encodedString:[fieldValue value]]; } else if (fieldClass == [FSTTimestampValue class]) { - return [self encodedTimestampValue:((FSTTimestampValue *)fieldValue).internalValue]; + return [self encodedTimestampValue:[fieldValue value]]; } else if (fieldClass == [FSTGeoPointValue class]) { return [self encodedGeoPointValue:[fieldValue value]]; |