Skip to content

Commit ce07f8d

Browse files
wilhuffwilhuff
authored
Port FSTListenOptions to C++ (#2617)
1 parent 080c64d commit ce07f8d

File tree

14 files changed

+168
-137
lines changed

14 files changed

+168
-137
lines changed

‎Firestore/Example/Tests/Core/FSTEventManagerTests.mm‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ @implementation FSTEventManagerTests
6262

6363
- (FSTQueryListener *)noopListenerForQuery:(FSTQuery *)query {
6464
return [[FSTQueryListener alloc] initWithQuery:query
65-
options:[FSTListenOptions defaultOptions]
65+
options:ListenOptions::DefaultOptions()
6666
viewSnapshotHandler:[](const StatusOr<ViewSnapshot> &) {}];
6767
}
6868

@@ -102,7 +102,7 @@ - (void)testHandlesUnlistenOnUnknownListenerGracefully {
102102
- (FSTQueryListener *)queryListenerForQuery:(FSTQuery *)query
103103
withHandler:(ViewSnapshotHandler &&)handler {
104104
return [[FSTQueryListener alloc] initWithQuery:query
105-
options:[FSTListenOptions defaultOptions]
105+
options:ListenOptions::DefaultOptions()
106106
viewSnapshotHandler:std::move(handler)];
107107
}
108108

‎Firestore/Example/Tests/Core/FSTQueryListenerTests.mm‎

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,12 @@ @interface FSTQueryListenerTests : XCTestCase
7878

7979
@implementation FSTQueryListenerTests {
8080
DelayedConstructor<ExecutorLibdispatch> _executor;
81-
FSTListenOptions *_includeMetadataChanges;
81+
ListenOptions _includeMetadataChanges;
8282
}
8383

8484
- (void)setUp {
8585
_executor.Init(dispatch_queue_create("FSTQueryListenerTests Queue", DISPATCH_QUEUE_SERIAL));
86-
_includeMetadataChanges = [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:YES
87-
includeDocumentMetadataChanges:YES
88-
waitForSyncWhenOnline:NO];
86+
_includeMetadataChanges = ListenOptions::FromIncludeMetadataChanges(true);
8987
}
9088

9189
- (void)testRaisesCollectionEvents {
@@ -248,9 +246,10 @@ - (void)testRaisesDocumentMetadataEventsOnlyWhenSpecified {
248246
FSTTestDoc("rooms/Eros", 1, @{@"name" : @"Eros"}, FSTDocumentStateSynced);
249247
FSTDocument *doc3 = FSTTestDoc("rooms/Other", 3, @{@"name" : @"Other"}, FSTDocumentStateSynced);
250248

251-
FSTListenOptions *options = [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:NO
252-
includeDocumentMetadataChanges:YES
253-
waitForSyncWhenOnline:NO];
249+
ListenOptions options(
250+
/*include_query_metadata_changes=*/false,
251+
/*include_document_metadata_changes=*/true,
252+
/*wait_for_sync_when_online=*/false);
254253

255254
FSTQueryListener *filteredListener = [self listenToQuery:query
256255
accumulatingSnapshots:&filteredAccum];
@@ -300,9 +299,10 @@ - (void)testRaisesQueryMetadataEventsOnlyWhenHasPendingWritesOnTheQueryChanges {
300299
FSTTestDoc("rooms/Hades", 2, @{@"name" : @"Hades"}, FSTDocumentStateSynced);
301300
FSTDocument *doc3 = FSTTestDoc("rooms/Other", 3, @{@"name" : @"Other"}, FSTDocumentStateSynced);
302301

303-
FSTListenOptions *options = [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:YES
304-
includeDocumentMetadataChanges:NO
305-
waitForSyncWhenOnline:NO];
302+
ListenOptions options(
303+
/*include_query_metadata_changes=*/true,
304+
/*include_document_metadata_changes=*/false,
305+
/*wait_for_sync_when_online=*/false);
306306
FSTQueryListener *fullListener = [self listenToQuery:query
307307
options:options
308308
accumulatingSnapshots:&fullAccum];
@@ -373,12 +373,14 @@ - (void)testWillWaitForSyncIfOnline {
373373
FSTQuery *query = FSTTestQuery("rooms");
374374
FSTDocument *doc1 = FSTTestDoc("rooms/Eros", 1, @{@"name" : @"Eros"}, FSTDocumentStateSynced);
375375
FSTDocument *doc2 = FSTTestDoc("rooms/Hades", 2, @{@"name" : @"Hades"}, FSTDocumentStateSynced);
376-
FSTQueryListener *listener =
377-
[self listenToQuery:query
378-
options:[[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:NO
379-
includeDocumentMetadataChanges:NO
380-
waitForSyncWhenOnline:YES]
381-
accumulatingSnapshots:&events];
376+
377+
ListenOptions options(
378+
/*include_query_metadata_changes=*/false,
379+
/*include_document_metadata_changes=*/false,
380+
/*wait_for_sync_when_online=*/true);
381+
FSTQueryListener *listener = [self listenToQuery:query
382+
options:options
383+
accumulatingSnapshots:&events];
382384

383385
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
384386
ViewSnapshot snap1 = FSTTestApplyChanges(view, @[ doc1 ], absl::nullopt).value();
@@ -412,12 +414,15 @@ - (void)testWillRaiseInitialEventWhenGoingOffline {
412414
FSTQuery *query = FSTTestQuery("rooms");
413415
FSTDocument *doc1 = FSTTestDoc("rooms/Eros", 1, @{@"name" : @"Eros"}, FSTDocumentStateSynced);
414416
FSTDocument *doc2 = FSTTestDoc("rooms/Hades", 2, @{@"name" : @"Hades"}, FSTDocumentStateSynced);
415-
FSTQueryListener *listener =
416-
[self listenToQuery:query
417-
options:[[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:NO
418-
includeDocumentMetadataChanges:NO
419-
waitForSyncWhenOnline:YES]
420-
accumulatingSnapshots:&events];
417+
418+
ListenOptions options(
419+
/*include_query_metadata_changes=*/false,
420+
/*include_document_metadata_changes=*/false,
421+
/*wait_for_sync_when_online=*/true);
422+
423+
FSTQueryListener *listener = [self listenToQuery:query
424+
options:options
425+
accumulatingSnapshots:&events];
421426

422427
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
423428
ViewSnapshot snap1 = FSTTestApplyChanges(view, @[ doc1 ], absl::nullopt).value();
@@ -457,7 +462,7 @@ - (void)testWillRaiseInitialEventWhenGoingOfflineAndThereAreNoDocs {
457462

458463
FSTQuery *query = FSTTestQuery("rooms");
459464
FSTQueryListener *listener = [self listenToQuery:query
460-
options:[FSTListenOptions defaultOptions]
465+
options:ListenOptions::DefaultOptions()
461466
accumulatingSnapshots:&events];
462467

463468
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
@@ -483,7 +488,7 @@ - (void)testWillRaiseInitialEventWhenStartingOfflineAndThereAreNoDocs {
483488

484489
FSTQuery *query = FSTTestQuery("rooms");
485490
FSTQueryListener *listener = [self listenToQuery:query
486-
options:[FSTListenOptions defaultOptions]
491+
options:ListenOptions::DefaultOptions()
487492
accumulatingSnapshots:&events];
488493

489494
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
@@ -505,12 +510,12 @@ - (void)testWillRaiseInitialEventWhenStartingOfflineAndThereAreNoDocs {
505510

506511
- (FSTQueryListener *)listenToQuery:(FSTQuery *)query handler:(ViewSnapshotHandler &&)handler {
507512
return [[FSTQueryListener alloc] initWithQuery:query
508-
options:[FSTListenOptions defaultOptions]
513+
options:ListenOptions::DefaultOptions()
509514
viewSnapshotHandler:std::move(handler)];
510515
}
511516

512517
- (FSTQueryListener *)listenToQuery:(FSTQuery *)query
513-
options:(FSTListenOptions *)options
518+
options:(ListenOptions)options
514519
accumulatingSnapshots:(std::vector<ViewSnapshot> *)values {
515520
return [[FSTQueryListener alloc] initWithQuery:query
516521
options:options
@@ -522,7 +527,7 @@ - (FSTQueryListener *)listenToQuery:(FSTQuery *)query
522527
- (FSTQueryListener *)listenToQuery:(FSTQuery *)query
523528
accumulatingSnapshots:(std::vector<ViewSnapshot> *)values {
524529
return [self listenToQuery:query
525-
options:[FSTListenOptions defaultOptions]
530+
options:ListenOptions::DefaultOptions()
526531
accumulatingSnapshots:values];
527532
}
528533

‎Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,7 @@ - (FSTOutstandingWrite *)receiveWriteError:(int)errorCode
349349
- (TargetId)addUserListenerWithQuery:(FSTQuery *)query {
350350
// TODO(dimond): Allow customizing listen options in spec tests
351351
// TODO(dimond): Change spec tests to verify isFromCache on snapshots
352-
FSTListenOptions *options = [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:YES
353-
includeDocumentMetadataChanges:YES
354-
waitForSyncWhenOnline:NO];
352+
ListenOptions options = ListenOptions::FromIncludeMetadataChanges(true);
355353
FSTQueryListener *listener = [[FSTQueryListener alloc]
356354
initWithQuery:query
357355
options:options

‎Firestore/Source/API/FIRDocumentReference.mm‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -204,23 +204,15 @@ - (void)getDocumentWithSource:(FIRFirestoreSource)source
204204
- (id<FIRListenerRegistration>)
205205
addSnapshotListenerWithIncludeMetadataChanges:(BOOL)includeMetadataChanges
206206
listener:(FIRDocumentSnapshotBlock)listener {
207-
FSTListenOptions *options =
208-
[self internalOptionsForIncludeMetadataChanges:includeMetadataChanges];
207+
ListenOptions options = ListenOptions::FromIncludeMetadataChanges(includeMetadataChanges);
209208
return [self addSnapshotListenerInternalWithOptions:options listener:listener];
210209
}
211210

212-
- (id<FIRListenerRegistration>)
213-
addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions
214-
listener:(FIRDocumentSnapshotBlock)listener {
211+
- (id<FIRListenerRegistration>)addSnapshotListenerInternalWithOptions:(ListenOptions)internalOptions
212+
listener:(FIRDocumentSnapshotBlock)
213+
listener {
215214
return _documentReference.AddSnapshotListener([self wrapDocumentSnapshotBlock:listener],
216-
internalOptions);
217-
}
218-
219-
/** Converts the public API options object to the internal options object. */
220-
- (FSTListenOptions *)internalOptionsForIncludeMetadataChanges:(BOOL)includeMetadataChanges {
221-
return [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:includeMetadataChanges
222-
includeDocumentMetadataChanges:includeMetadataChanges
223-
waitForSyncWhenOnline:NO];
215+
std::move(internalOptions));
224216
}
225217

226218
- (StatusOrCallback<DocumentSnapshot>)wrapDocumentSnapshotBlock:(FIRDocumentSnapshotBlock)block {

‎Firestore/Source/API/FIRQuery.mm‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,10 @@ - (void)getDocumentsWithSource:(FIRFirestoreSource)source
118118
return;
119119
}
120120

121-
FSTListenOptions *listenOptions =
122-
[[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:YES
123-
includeDocumentMetadataChanges:YES
124-
waitForSyncWhenOnline:YES];
121+
ListenOptions listenOptions(
122+
/*include_query_metadata_changes=*/true,
123+
/*include_document_metadata_changes=*/true,
124+
/*wait_for_sync_when_online=*/true);
125125

126126
dispatch_semaphore_t registered = dispatch_semaphore_create(0);
127127
__block id<FIRListenerRegistration> listenerRegistration;
@@ -164,13 +164,13 @@ - (void)getDocumentsWithSource:(FIRFirestoreSource)source
164164
- (id<FIRListenerRegistration>)
165165
addSnapshotListenerWithIncludeMetadataChanges:(BOOL)includeMetadataChanges
166166
listener:(FIRQuerySnapshotBlock)listener {
167-
auto options = [self internalOptionsForIncludeMetadataChanges:includeMetadataChanges];
167+
auto options = ListenOptions::FromIncludeMetadataChanges(includeMetadataChanges);
168168
return [self addSnapshotListenerInternalWithOptions:options listener:listener];
169169
}
170170

171-
- (id<FIRListenerRegistration>)
172-
addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions
173-
listener:(FIRQuerySnapshotBlock)listener {
171+
- (id<FIRListenerRegistration>)addSnapshotListenerInternalWithOptions:(ListenOptions)internalOptions
172+
listener:
173+
(FIRQuerySnapshotBlock)listener {
174174
Firestore *firestore = self.firestore.wrapped;
175175
FSTQuery *query = self.query;
176176

@@ -665,13 +665,6 @@ - (FSTBound *)boundFromFieldValues:(NSArray<id> *)fieldValues isBefore:(BOOL)isB
665665
return [FSTBound boundWithPosition:components isBefore:isBefore];
666666
}
667667

668-
/** Converts the public API options object to the internal options object. */
669-
- (FSTListenOptions *)internalOptionsForIncludeMetadataChanges:(BOOL)includeMetadataChanges {
670-
return [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:includeMetadataChanges
671-
includeDocumentMetadataChanges:includeMetadataChanges
672-
waitForSyncWhenOnline:NO];
673-
}
674-
675668
@end
676669

677670
NS_ASSUME_NONNULL_END

‎Firestore/Source/Core/FSTEventManager.h‎

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#import <Foundation/Foundation.h>
1818

19+
#include "Firestore/core/src/firebase/firestore/core/listen_options.h"
1920
#include "Firestore/core/src/firebase/firestore/core/view_snapshot.h"
2021
#include "Firestore/core/src/firebase/firestore/model/types.h"
2122
#include "Firestore/core/src/firebase/firestore/util/status.h"
@@ -25,26 +26,7 @@
2526

2627
NS_ASSUME_NONNULL_BEGIN
2728

28-
#pragma mark - FSTListenOptions
29-
30-
@interface FSTListenOptions : NSObject
31-
32-
+ (instancetype)defaultOptions;
33-
34-
- (instancetype)initWithIncludeQueryMetadataChanges:(BOOL)includeQueryMetadataChanges
35-
includeDocumentMetadataChanges:(BOOL)includeDocumentMetadataChanges
36-
waitForSyncWhenOnline:(BOOL)waitForSyncWhenOnline
37-
NS_DESIGNATED_INITIALIZER;
38-
39-
- (instancetype)init NS_UNAVAILABLE;
40-
41-
@property(nonatomic, assign, readonly) BOOL includeQueryMetadataChanges;
42-
43-
@property(nonatomic, assign, readonly) BOOL includeDocumentMetadataChanges;
44-
45-
@property(nonatomic, assign, readonly) BOOL waitForSyncWhenOnline;
46-
47-
@end
29+
using firebase::firestore::core::ListenOptions;
4830

4931
#pragma mark - FSTQueryListener
5032

@@ -55,7 +37,7 @@ NS_ASSUME_NONNULL_BEGIN
5537
@interface FSTQueryListener : NSObject
5638

5739
- (instancetype)initWithQuery:(FSTQuery *)query
58-
options:(FSTListenOptions *)options
40+
options:(ListenOptions)options
5941
viewSnapshotHandler:(firebase::firestore::core::ViewSnapshotHandler &&)viewSnapshotHandler
6042
NS_DESIGNATED_INITIALIZER;
6143

‎Firestore/Source/Core/FSTEventManager.mm‎

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -28,48 +28,15 @@
2828
#include "Firestore/core/src/firebase/firestore/util/status.h"
2929
#include "absl/types/optional.h"
3030

31+
NS_ASSUME_NONNULL_BEGIN
32+
3133
using firebase::firestore::core::DocumentViewChange;
3234
using firebase::firestore::core::ViewSnapshot;
3335
using firebase::firestore::core::ViewSnapshotHandler;
3436
using firebase::firestore::model::OnlineState;
3537
using firebase::firestore::model::TargetId;
36-
using firebase::firestore::util::Status;
3738
using firebase::firestore::util::MakeStatus;
38-
39-
NS_ASSUME_NONNULL_BEGIN
40-
41-
#pragma mark - FSTListenOptions
42-
43-
@implementation FSTListenOptions
44-
45-
+ (instancetype)defaultOptions {
46-
static FSTListenOptions *defaultOptions;
47-
static dispatch_once_t onceToken;
48-
dispatch_once(&onceToken, ^{
49-
defaultOptions = [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:NO
50-
includeDocumentMetadataChanges:NO
51-
waitForSyncWhenOnline:NO];
52-
});
53-
return defaultOptions;
54-
}
55-
56-
- (instancetype)initWithIncludeQueryMetadataChanges:(BOOL)includeQueryMetadataChanges
57-
includeDocumentMetadataChanges:(BOOL)includeDocumentMetadataChanges
58-
waitForSyncWhenOnline:(BOOL)waitForSyncWhenOnline {
59-
if (self = [super init]) {
60-
_includeQueryMetadataChanges = includeQueryMetadataChanges;
61-
_includeDocumentMetadataChanges = includeDocumentMetadataChanges;
62-
_waitForSyncWhenOnline = waitForSyncWhenOnline;
63-
}
64-
return self;
65-
}
66-
67-
- (instancetype)init {
68-
HARD_FAIL("FSTListenOptions init not supported");
69-
return nil;
70-
}
71-
72-
@end
39+
using firebase::firestore::util::Status;
7340

7441
#pragma mark - FSTQueryListenersInfo
7542

@@ -113,8 +80,6 @@ @interface FSTQueryListener ()
11380
/** The last received view snapshot. */
11481
- (const absl::optional<ViewSnapshot> &)snapshot;
11582

116-
@property(nonatomic, strong, readonly) FSTListenOptions *options;
117-
11883
/**
11984
* Initial snapshots (e.g. from cache) may not be propagated to the ViewSnapshotHandler.
12085
* This flag is set to YES once we've actually raised an event.
@@ -127,18 +92,20 @@ @interface FSTQueryListener ()
12792
@end
12893

12994
@implementation FSTQueryListener {
95+
ListenOptions _options;
96+
13097
absl::optional<ViewSnapshot> _snapshot;
13198

13299
/** The ViewSnapshotHandler associated with this query listener. */
133100
ViewSnapshotHandler _viewSnapshotHandler;
134101
}
135102

136103
- (instancetype)initWithQuery:(FSTQuery *)query
137-
options:(FSTListenOptions *)options
104+
options:(ListenOptions)options
138105
viewSnapshotHandler:(ViewSnapshotHandler &&)viewSnapshotHandler {
139106
if (self = [super init]) {
140107
_query = query;
141-
_options = options;
108+
_options = std::move(options);
142109
_viewSnapshotHandler = std::move(viewSnapshotHandler);
143110
_raisedInitialEvent = NO;
144111
}
@@ -153,7 +120,7 @@ - (void)queryDidChangeViewSnapshot:(ViewSnapshot)snapshot {
153120
HARD_ASSERT(!snapshot.document_changes().empty() || snapshot.sync_state_changed(),
154121
"We got a new snapshot with no changes?");
155122

156-
if (!self.options.includeDocumentMetadataChanges) {
123+
if (!_options.include_document_metadata_changes()) {
157124
// Remove the metadata-only changes.
158125
std::vector<DocumentViewChange> changes;
159126
for (const DocumentViewChange &change : snapshot.document_changes()) {
@@ -210,7 +177,7 @@ - (BOOL)shouldRaiseInitialEventForSnapshot:(const ViewSnapshot &)snapshot
210177
BOOL maybeOnline = onlineState != OnlineState::Offline;
211178
// Don't raise the event if we're online, aren't synced yet (checked
212179
// above) and are waiting for a sync.
213-
if (self.options.waitForSyncWhenOnline && maybeOnline) {
180+
if (_options.wait_for_sync_when_online() && maybeOnline) {
214181
HARD_ASSERT(snapshot.from_cache(), "Waiting for sync, but snapshot is not from cache.");
215182
return NO;
216183
}
@@ -230,7 +197,7 @@ - (BOOL)shouldRaiseEventForSnapshot:(const ViewSnapshot &)snapshot {
230197
BOOL hasPendingWritesChanged = _snapshot.has_value() && _snapshot.value().has_pending_writes() !=
231198
snapshot.has_pending_writes();
232199
if (snapshot.sync_state_changed() || hasPendingWritesChanged) {
233-
return self.options.includeQueryMetadataChanges;
200+
return _options.include_query_metadata_changes();
234201
}
235202

236203
// Generally we should have hit one of the cases above, but it's possible to get here if there

0 commit comments

Comments
 (0)