From a17740e9146e4e2431d62964d044287cccc3ee85 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Fri, 30 Mar 2018 14:50:15 -0400 Subject: 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. --- Firestore/CHANGELOG.md | 10 ++ Firestore/Example/SwiftBuildTest/main.swift | 2 + .../Tests/Integration/API/FIRDatabaseTests.mm | 2 +- .../Tests/Integration/API/FIRFieldsTests.mm | 95 +++++++++++++++---- .../Integration/API/FIRServerTimestampTests.mm | 20 ++-- .../Example/Tests/Integration/API/FIRTypeTests.mm | 17 +++- .../Example/Tests/Model/FSTFieldValueTests.mm | 18 ++-- Firestore/Example/Tests/Model/FSTMutationTests.mm | 2 +- .../Example/Tests/Util/FSTIntegrationTestCase.mm | 1 + Firestore/Source/API/FIRDocumentSnapshot.mm | 12 ++- Firestore/Source/API/FIRFirestore.mm | 28 ++++++ Firestore/Source/API/FIRFirestoreSettings.mm | 8 +- Firestore/Source/API/FIRTimestamp.m | 2 +- Firestore/Source/Model/FSTFieldValue.h | 15 ++- Firestore/Source/Model/FSTFieldValue.mm | 102 ++++++++++++++------- Firestore/Source/Public/FIRFirestoreSettings.h | 17 ++++ Firestore/Source/Public/FIRTimestamp.h | 2 +- Firestore/Source/Remote/FSTSerializerBeta.mm | 2 +- 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 *testDataWithTimestamps(FIRTimestamp *timestamp) { + return @{ @"timestamp" : timestamp, @"nested" : @{@"timestamp2" : timestamp} }; +} + @implementation FIRFieldsTests - (NSDictionary *)testNestedDataNumbered:(int)number { @@ -221,28 +225,83 @@ [self awaitExpectations]; } -- (NSDictionary *)testDataWithTimestamp:(FIRTimestamp *)timestamp { - return @{ - @"timestamp" : [timestamp approximateDateValue], - @"metadata" : @{@"nestedTimestamp" : [timestamp approximateDateValue]} - }; +- (FIRDocumentSnapshot *)snapshotWithTimestamps:(FIRTimestamp *)timestamp { + FIRDocumentReference *doc = [self documentRef]; + NSDictionary *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 *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 *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 +#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 +@interface FSTTimestampValue : FSTFieldValue + (instancetype)timestampValue:(FIRTimestamp *)value; -- (FIRTimestamp *)internalValue; @end /** @@ -194,7 +197,6 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) { */ @interface FSTGeoPointValue : FSTFieldValue + (instancetype)geoPointValue:(FIRGeoPoint *)value; -- (FIRGeoPoint *)valueWithOptions:(FSTFieldValueOptions *)options; @end /** @@ -202,7 +204,6 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) { */ @interface FSTBlobValue : FSTFieldValue + (instancetype)blobValue:(NSData *)value; -- (NSData *)valueWithOptions:(FSTFieldValueOptions *)options; @end /** @@ -211,7 +212,6 @@ typedef NS_ENUM(NSInteger, FSTServerTimestampBehavior) { @interface FSTReferenceValue : FSTFieldValue + (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 *)value NS_DESIGNATED_INITIALIZER; -- (NSDictionary *)valueWithOptions:(FSTFieldValueOptions *)options; - (FSTImmutableSortedDictionary *)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 { return FSTTypeOrderString; } -- (id)valueWithOptions:(FSTFieldValueOptions *)options { +- (id)value { return self.internalValue; } @@ -447,9 +453,16 @@ struct Comparator { 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 { } @end - #pragma mark - FSTServerTimestampValue @implementation FSTServerTimestampValue @@ -498,16 +510,20 @@ struct Comparator { 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 { return FSTTypeOrderGeoPoint; } -- (id)valueWithOptions:(FSTFieldValueOptions *)options { +- (id)value { return self.internalValue; } @@ -583,7 +599,6 @@ struct Comparator { } @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]]; -- cgit v1.2.3