From 33b12f6c70729d56c6e6350d435559cec1c44c61 Mon Sep 17 00:00:00 2001 From: Gil Date: Sat, 14 Apr 2018 17:19:06 -0700 Subject: Replace `DocumentListenOptions` with a simple boolean. (#1099) * Replace `DocumentListenOptions` with a simple boolean. Instead of calling `addSnapshotListener(options: DocumentListenOptions.includeMetadataChanges(true))` call `addSnapshotListener(includeMetadataChanges:true)` * Style --- Firestore/CHANGELOG.md | 4 + Firestore/Example/SwiftBuildTest/main.swift | 14 +- .../Tests/Integration/API/FIRDatabaseTests.mm | 162 ++++++++++----------- .../Tests/Integration/API/FIRWriteBatchTests.mm | 4 +- .../Example/Tests/Util/FSTIntegrationTestCase.mm | 17 ++- Firestore/Source/API/FIRDocumentReference.mm | 66 ++------- Firestore/Source/Public/FIRDocumentReference.h | 34 +---- Firestore/Source/Public/FIRSnapshotMetadata.h | 10 +- 8 files changed, 131 insertions(+), 180 deletions(-) diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index 9987471..11df638 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,4 +1,8 @@ # Unreleased +- [changed] Replaced the `DocumentListenOptions` object with a simple boolean. + Instead of calling + `addSnapshotListener(options: DocumentListenOptions.includeMetadataChanges(true))` + call `addSnapshotListener(includeMetadataChanges:true)`. # v0.11.0 - [fixed] Fixed a regression in the Firebase iOS SDK release 4.11.0 that could diff --git a/Firestore/Example/SwiftBuildTest/main.swift b/Firestore/Example/SwiftBuildTest/main.swift index 691adf6..555a75b 100644 --- a/Firestore/Example/SwiftBuildTest/main.swift +++ b/Firestore/Example/SwiftBuildTest/main.swift @@ -255,6 +255,19 @@ func listenToDocument(at docRef: DocumentReference) { listener.remove() } +func listenToDocumentWithMetadataChanges(at docRef: DocumentReference) { + let listener = docRef.addSnapshotListener(includeMetadataChanges: true) { document, error in + if let document = document { + if document.metadata.hasPendingWrites { + print("Has pending writes") + } + } + } + + // Unsubscribe. + listener.remove() +} + func listenToDocuments(matching query: Query) { let listener = query.addSnapshotListener { snap, error in if let error = error { @@ -330,7 +343,6 @@ func transactions() { func types() { let _: CollectionReference let _: DocumentChange - let _: DocumentListenOptions let _: DocumentReference let _: DocumentSnapshot let _: FieldPath diff --git a/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm b/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm index 312c3ff..503d50b 100644 --- a/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRDatabaseTests.mm @@ -370,32 +370,30 @@ __block XCTestExpectation *dataCompletion; __block int callbacks = 0; - FIRDocumentListenOptions *options = - [[FIRDocumentListenOptions options] includeMetadataChanges:YES]; + id listenerRegistration = [docRef + addSnapshotListenerWithIncludeMetadataChanges:YES + listener:^(FIRDocumentSnapshot *_Nullable doc, + NSError *error) { + callbacks++; - id listenerRegistration = - [docRef addSnapshotListenerWithOptions:options - listener:^(FIRDocumentSnapshot *_Nullable doc, NSError *error) { - callbacks++; - - if (callbacks == 1) { - XCTAssertNotNil(doc); - XCTAssertFalse(doc.exists); - [emptyCompletion fulfill]; + if (callbacks == 1) { + XCTAssertNotNil(doc); + XCTAssertFalse(doc.exists); + [emptyCompletion fulfill]; - } else if (callbacks == 2) { - XCTAssertEqualObjects(doc.data, (@{ @"a" : @1 })); - XCTAssertEqual(doc.metadata.hasPendingWrites, YES); + } else if (callbacks == 2) { + XCTAssertEqualObjects(doc.data, (@{ @"a" : @1 })); + XCTAssertEqual(doc.metadata.hasPendingWrites, YES); - } else if (callbacks == 3) { - XCTAssertEqualObjects(doc.data, (@{ @"a" : @1 })); - XCTAssertEqual(doc.metadata.hasPendingWrites, NO); - [dataCompletion fulfill]; + } else if (callbacks == 3) { + XCTAssertEqualObjects(doc.data, (@{ @"a" : @1 })); + XCTAssertEqual(doc.metadata.hasPendingWrites, NO); + [dataCompletion fulfill]; - } else if (callbacks == 4) { - XCTFail("Should not have received this callback"); - } - }]; + } else if (callbacks == 4) { + XCTFail("Should not have received this callback"); + } + }]; [self awaitExpectations]; dataCompletion = [self expectationWithDescription:@"data snapshot"]; @@ -458,40 +456,38 @@ __block XCTestExpectation *changeCompletion; __block int callbacks = 0; - FIRDocumentListenOptions *options = - [[FIRDocumentListenOptions options] includeMetadataChanges:YES]; + id listenerRegistration = [docRef + addSnapshotListenerWithIncludeMetadataChanges:YES + listener:^(FIRDocumentSnapshot *_Nullable doc, + NSError *error) { + callbacks++; - id listenerRegistration = - [docRef addSnapshotListenerWithOptions:options - listener:^(FIRDocumentSnapshot *_Nullable doc, NSError *error) { - callbacks++; - - if (callbacks == 1) { - XCTAssertEqualObjects(doc.data, initialData); - XCTAssertEqual(doc.metadata.hasPendingWrites, NO); - XCTAssertEqual(doc.metadata.isFromCache, YES); - - } else if (callbacks == 2) { - XCTAssertEqualObjects(doc.data, initialData); - XCTAssertEqual(doc.metadata.hasPendingWrites, NO); - XCTAssertEqual(doc.metadata.isFromCache, NO); - [initialCompletion fulfill]; - - } else if (callbacks == 3) { - XCTAssertEqualObjects(doc.data, changedData); - XCTAssertEqual(doc.metadata.hasPendingWrites, YES); - XCTAssertEqual(doc.metadata.isFromCache, NO); - - } else if (callbacks == 4) { - XCTAssertEqualObjects(doc.data, changedData); - XCTAssertEqual(doc.metadata.hasPendingWrites, NO); - XCTAssertEqual(doc.metadata.isFromCache, NO); - [changeCompletion fulfill]; - - } else if (callbacks == 5) { - XCTFail("Should not have received this callback"); - } - }]; + if (callbacks == 1) { + XCTAssertEqualObjects(doc.data, initialData); + XCTAssertEqual(doc.metadata.hasPendingWrites, NO); + XCTAssertEqual(doc.metadata.isFromCache, YES); + + } else if (callbacks == 2) { + XCTAssertEqualObjects(doc.data, initialData); + XCTAssertEqual(doc.metadata.hasPendingWrites, NO); + XCTAssertEqual(doc.metadata.isFromCache, NO); + [initialCompletion fulfill]; + + } else if (callbacks == 3) { + XCTAssertEqualObjects(doc.data, changedData); + XCTAssertEqual(doc.metadata.hasPendingWrites, YES); + XCTAssertEqual(doc.metadata.isFromCache, NO); + + } else if (callbacks == 4) { + XCTAssertEqualObjects(doc.data, changedData); + XCTAssertEqual(doc.metadata.hasPendingWrites, NO); + XCTAssertEqual(doc.metadata.isFromCache, NO); + [changeCompletion fulfill]; + + } else if (callbacks == 5) { + XCTFail("Should not have received this callback"); + } + }]; [self awaitExpectations]; changeCompletion = [self expectationWithDescription:@"listen for changed data"]; @@ -548,39 +544,37 @@ [self writeDocumentRef:docRef data:initialData]; - FIRDocumentListenOptions *options = - [[FIRDocumentListenOptions options] includeMetadataChanges:YES]; - XCTestExpectation *initialCompletion = [self expectationWithDescription:@"initial data"]; __block XCTestExpectation *changeCompletion; __block int callbacks = 0; - id listenerRegistration = - [docRef addSnapshotListenerWithOptions:options - listener:^(FIRDocumentSnapshot *_Nullable doc, NSError *error) { - callbacks++; - - if (callbacks == 1) { - XCTAssertEqualObjects(doc.data, initialData); - XCTAssertEqual(doc.metadata.hasPendingWrites, NO); - XCTAssertEqual(doc.metadata.isFromCache, YES); - - } else if (callbacks == 2) { - XCTAssertEqualObjects(doc.data, initialData); - XCTAssertEqual(doc.metadata.hasPendingWrites, NO); - XCTAssertEqual(doc.metadata.isFromCache, NO); - [initialCompletion fulfill]; - - } else if (callbacks == 3) { - XCTAssertFalse(doc.exists); - XCTAssertEqual(doc.metadata.hasPendingWrites, NO); - XCTAssertEqual(doc.metadata.isFromCache, NO); - [changeCompletion fulfill]; - - } else if (callbacks == 4) { - XCTFail("Should not have received this callback"); - } - }]; + id listenerRegistration = [docRef + addSnapshotListenerWithIncludeMetadataChanges:YES + listener:^(FIRDocumentSnapshot *_Nullable doc, + NSError *error) { + callbacks++; + + if (callbacks == 1) { + XCTAssertEqualObjects(doc.data, initialData); + XCTAssertEqual(doc.metadata.hasPendingWrites, NO); + XCTAssertEqual(doc.metadata.isFromCache, YES); + + } else if (callbacks == 2) { + XCTAssertEqualObjects(doc.data, initialData); + XCTAssertEqual(doc.metadata.hasPendingWrites, NO); + XCTAssertEqual(doc.metadata.isFromCache, NO); + [initialCompletion fulfill]; + + } else if (callbacks == 3) { + XCTAssertFalse(doc.exists); + XCTAssertEqual(doc.metadata.hasPendingWrites, NO); + XCTAssertEqual(doc.metadata.isFromCache, NO); + [changeCompletion fulfill]; + + } else if (callbacks == 4) { + XCTFail("Should not have received this callback"); + } + }]; [self awaitExpectations]; changeCompletion = [self expectationWithDescription:@"listen for changed data"]; diff --git a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm index 9a2fef1..b1d6738 100644 --- a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm +++ b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm @@ -248,9 +248,7 @@ - (void)testCanWriteTheSameDocumentMultipleTimes { FIRDocumentReference *doc = [self documentRef]; FSTEventAccumulator *accumulator = [FSTEventAccumulator accumulatorForTest:self]; - [doc - addSnapshotListenerWithOptions:[[FIRDocumentListenOptions options] includeMetadataChanges:YES] - listener:accumulator.valueEventHandler]; + [doc addSnapshotListenerWithIncludeMetadataChanges:YES listener:accumulator.valueEventHandler]; FIRDocumentSnapshot *initialSnap = [accumulator awaitEventWithName:@"initial event"]; XCTAssertFalse(initialSnap.exists); diff --git a/Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm b/Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm index 79163da..2b1f9d0 100644 --- a/Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm +++ b/Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm @@ -245,14 +245,15 @@ NS_ASSUME_NONNULL_BEGIN XCTestExpectation *expectation = [self expectationWithDescription:@"listener"]; id listener = [ref - addSnapshotListenerWithOptions:[[FIRDocumentListenOptions options] includeMetadataChanges:YES] - listener:^(FIRDocumentSnapshot *snapshot, NSError *error) { - XCTAssertNil(error); - if (!requireOnline || !snapshot.metadata.fromCache) { - result = snapshot; - [expectation fulfill]; - } - }]; + addSnapshotListenerWithIncludeMetadataChanges:YES + listener:^(FIRDocumentSnapshot *snapshot, + NSError *error) { + XCTAssertNil(error); + if (!requireOnline || !snapshot.metadata.fromCache) { + result = snapshot; + [expectation fulfill]; + } + }]; [self awaitExpectations]; [listener remove]; diff --git a/Firestore/Source/API/FIRDocumentReference.mm b/Firestore/Source/API/FIRDocumentReference.mm index 9fb4541..d3a9295 100644 --- a/Firestore/Source/API/FIRDocumentReference.mm +++ b/Firestore/Source/API/FIRDocumentReference.mm @@ -52,40 +52,6 @@ using firebase::firestore::model::ResourcePath; NS_ASSUME_NONNULL_BEGIN -#pragma mark - FIRDocumentListenOptions - -@interface FIRDocumentListenOptions () - -- (instancetype)initWithIncludeMetadataChanges:(BOOL)includeMetadataChanges - NS_DESIGNATED_INITIALIZER; - -@property(nonatomic, assign, readonly) BOOL includeMetadataChanges; - -@end - -@implementation FIRDocumentListenOptions - -+ (instancetype)options { - return [[FIRDocumentListenOptions alloc] init]; -} - -- (instancetype)initWithIncludeMetadataChanges:(BOOL)includeMetadataChanges { - if (self = [super init]) { - _includeMetadataChanges = includeMetadataChanges; - } - return self; -} - -- (instancetype)init { - return [self initWithIncludeMetadataChanges:NO]; -} - -- (instancetype)includeMetadataChanges:(BOOL)includeMetadataChanges { - return [[FIRDocumentListenOptions alloc] initWithIncludeMetadataChanges:includeMetadataChanges]; -} - -@end - #pragma mark - FIRDocumentReference @interface FIRDocumentReference () @@ -198,10 +164,9 @@ NS_ASSUME_NONNULL_BEGIN - (void)getDocumentWithCompletion:(void (^)(FIRDocumentSnapshot *_Nullable document, NSError *_Nullable error))completion { - FSTListenOptions *listenOptions = - [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:YES - includeDocumentMetadataChanges:YES - waitForSyncWhenOnline:YES]; + FSTListenOptions *options = [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:YES + includeDocumentMetadataChanges:YES + waitForSyncWhenOnline:YES]; dispatch_semaphore_t registered = dispatch_semaphore_create(0); __block id listenerRegistration; @@ -238,20 +203,20 @@ NS_ASSUME_NONNULL_BEGIN } }; - listenerRegistration = - [self addSnapshotListenerInternalWithOptions:listenOptions listener:listener]; + listenerRegistration = [self addSnapshotListenerInternalWithOptions:options listener:listener]; dispatch_semaphore_signal(registered); } - (id)addSnapshotListener:(FIRDocumentSnapshotBlock)listener { - return [self addSnapshotListenerWithOptions:nil listener:listener]; + return [self addSnapshotListenerWithIncludeMetadataChanges:NO listener:listener]; } -- (id)addSnapshotListenerWithOptions: - (nullable FIRDocumentListenOptions *)options - listener:(FIRDocumentSnapshotBlock)listener { - return [self addSnapshotListenerInternalWithOptions:[self internalOptions:options] - listener:listener]; +- (id) +addSnapshotListenerWithIncludeMetadataChanges:(BOOL)includeMetadataChanges + listener:(FIRDocumentSnapshotBlock)listener { + FSTListenOptions *options = + [self internalOptionsForIncludeMetadataChanges:includeMetadataChanges]; + return [self addSnapshotListenerInternalWithOptions:options listener:listener]; } - (id) @@ -291,11 +256,10 @@ addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions } /** Converts the public API options object to the internal options object. */ -- (FSTListenOptions *)internalOptions:(nullable FIRDocumentListenOptions *)options { - return - [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:options.includeMetadataChanges - includeDocumentMetadataChanges:options.includeMetadataChanges - waitForSyncWhenOnline:NO]; +- (FSTListenOptions *)internalOptionsForIncludeMetadataChanges:(BOOL)includeMetadataChanges { + return [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:includeMetadataChanges + includeDocumentMetadataChanges:includeMetadataChanges + waitForSyncWhenOnline:NO]; } @end diff --git a/Firestore/Source/Public/FIRDocumentReference.h b/Firestore/Source/Public/FIRDocumentReference.h index 7fcc7a8..dd2d106 100644 --- a/Firestore/Source/Public/FIRDocumentReference.h +++ b/Firestore/Source/Public/FIRDocumentReference.h @@ -25,29 +25,6 @@ NS_ASSUME_NONNULL_BEGIN -/** - * Options for use with `[FIRDocumentReference addSnapshotListener]` to control the behavior of the - * snapshot listener. - */ -NS_SWIFT_NAME(DocumentListenOptions) -@interface FIRDocumentListenOptions : NSObject - -+ (instancetype)options NS_SWIFT_UNAVAILABLE("Use initializer"); - -- (instancetype)init; - -/** - * Sets the includeMetadataChanges option which controls whether metadata-only changes (i.e. only - * `FIRDocumentSnapshot.metadata` changed) should trigger snapshot events. Default is NO. - * - * @param includeMetadataChanges Whether to raise events for metadata-only changes. - * @return The receiver is returned for optional method chaining. - */ -- (instancetype)includeMetadataChanges:(BOOL)includeMetadataChanges - NS_SWIFT_NAME(includeMetadataChanges(_:)); - -@end - typedef void (^FIRDocumentSnapshotBlock)(FIRDocumentSnapshot *_Nullable snapshot, NSError *_Nullable error); @@ -208,16 +185,17 @@ NS_SWIFT_NAME(DocumentReference) /** * Attaches a listener for DocumentSnapshot events. * - * @param options Options controlling the listener behavior. + * @param includeMetadataChanges Whether metadata-only changes (i.e. only + * `FIRDocumentSnapshot.metadata` changed) should trigger snapshot events. * @param listener The listener to attach. * * @return A FIRListenerRegistration that can be used to remove this listener. */ // clang-format off -- (id)addSnapshotListenerWithOptions: - (nullable FIRDocumentListenOptions *)options - listener:(FIRDocumentSnapshotBlock)listener - NS_SWIFT_NAME(addSnapshotListener(options:listener:)); +- (id) +addSnapshotListenerWithIncludeMetadataChanges:(BOOL)includeMetadataChanges + listener:(FIRDocumentSnapshotBlock)listener + NS_SWIFT_NAME(addSnapshotListener(includeMetadataChanges:listener:)); // clang-format on @end diff --git a/Firestore/Source/Public/FIRSnapshotMetadata.h b/Firestore/Source/Public/FIRSnapshotMetadata.h index 4c7ff98..f47f383 100644 --- a/Firestore/Source/Public/FIRSnapshotMetadata.h +++ b/Firestore/Source/Public/FIRSnapshotMetadata.h @@ -27,16 +27,16 @@ NS_SWIFT_NAME(SnapshotMetadata) /** * Returns YES if the snapshot contains the result of local writes (e.g. set() or update() calls) * that have not yet been committed to the backend. If your listener has opted into metadata updates - * (via `FIRDocumentListenOptions` or `FIRQueryListenOptions`) you will receive another snapshot - * with `hasPendingWrites` equal to NO once the writes have been committed to the backend. + * (via `includeMetadataChanges:YES`) you will receive another snapshot with `hasPendingWrites` + * equal to NO once the writes have been committed to the backend. */ @property(nonatomic, assign, readonly, getter=hasPendingWrites) BOOL pendingWrites; /** * Returns YES if the snapshot was created from cached data rather than guaranteed up-to-date server - * data. If your listener has opted into metadata updates (via `FIRDocumentListenOptions` or - * `FIRQueryListenOptions`) you will receive another snapshot with `isFromCache` equal to NO once - * the client has received up-to-date data from the backend. + * data. If your listener has opted into metadata updates (via `includeMetadataChanges:YES`) you + * will receive another snapshot with `isFromCache` equal to NO once the client has received + * up-to-date data from the backend. */ @property(nonatomic, assign, readonly, getter=isFromCache) BOOL fromCache; -- cgit v1.2.3