Skip to content

Commit 9fc23fb

Browse files
wilhuffwilhuff
authored
Migrate FSTOnlineState to C++ OnlineState (#1705)
1 parent b5afc6b commit 9fc23fb

19 files changed

+139
-111
lines changed

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

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,22 @@
2626

2727
#import "Firestore/Example/Tests/Util/FSTHelpers.h"
2828

29+
#include "Firestore/core/src/firebase/firestore/model/types.h"
30+
31+
using firebase::firestore::model::OnlineState;
32+
2933
NS_ASSUME_NONNULL_BEGIN
3034

35+
/**
36+
* Converts an OnlineState to an NSNumber, usually for the purpose of adding
37+
* it to an NSArray or similar container. There's no direct conversion from a
38+
* strongly-typed enum to an integral type that could be passed to an NSNumber
39+
* initializer.
40+
*/
41+
static NSNumber *ToNSNumber(OnlineState state) {
42+
return @(static_cast<std::underlying_type<OnlineState>::type>(state));
43+
}
44+
3145
// FSTEventManager implements this delegate privately
3246
@interface FSTEventManager () <FSTSyncEngineDelegate>
3347
@end
@@ -139,23 +153,24 @@ - (void)testWillForwardOnlineStateChanges {
139153
FSTQueryListener *fakeListener = OCMClassMock([FSTQueryListener class]);
140154
NSMutableArray *events = [NSMutableArray array];
141155
OCMStub([fakeListener query]).andReturn(query);
142-
OCMStub([fakeListener applyChangedOnlineState:FSTOnlineStateUnknown])
156+
OCMStub([fakeListener applyChangedOnlineState:OnlineState::Unknown])
143157
.andDo(^(NSInvocation *invocation) {
144-
[events addObject:@(FSTOnlineStateUnknown)];
158+
[events addObject:ToNSNumber(OnlineState::Unknown)];
145159
});
146-
OCMStub([fakeListener applyChangedOnlineState:FSTOnlineStateOnline])
160+
OCMStub([fakeListener applyChangedOnlineState:OnlineState::Online])
147161
.andDo(^(NSInvocation *invocation) {
148-
[events addObject:@(FSTOnlineStateOnline)];
162+
[events addObject:ToNSNumber(OnlineState::Online)];
149163
});
150164

151165
FSTSyncEngine *syncEngineMock = OCMClassMock([FSTSyncEngine class]);
152166
OCMExpect([syncEngineMock setDelegate:[OCMArg any]]);
153167
FSTEventManager *eventManager = [FSTEventManager eventManagerWithSyncEngine:syncEngineMock];
154168

155169
[eventManager addListener:fakeListener];
156-
XCTAssertEqualObjects(events, @[ @(FSTOnlineStateUnknown) ]);
157-
[eventManager applyChangedOnlineState:FSTOnlineStateOnline];
158-
XCTAssertEqualObjects(events, (@[ @(FSTOnlineStateUnknown), @(FSTOnlineStateOnline) ]));
170+
XCTAssertEqualObjects(events, @[ ToNSNumber(OnlineState::Unknown) ]);
171+
[eventManager applyChangedOnlineState:OnlineState::Online];
172+
XCTAssertEqualObjects(events,
173+
(@[ ToNSNumber(OnlineState::Unknown), ToNSNumber(OnlineState::Online) ]));
159174
}
160175

161176
@end

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

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@
2727

2828
#import "Firestore/Example/Tests/Util/FSTHelpers.h"
2929

30+
#include "Firestore/core/src/firebase/firestore/model/types.h"
3031
#include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h"
3132
#include "absl/memory/memory.h"
3233

33-
using firebase::firestore::util::internal::ExecutorLibdispatch;
3434
using firebase::firestore::model::DocumentKeySet;
35+
using firebase::firestore::model::OnlineState;
36+
using firebase::firestore::util::internal::ExecutorLibdispatch;
3537

3638
NS_ASSUME_NONNULL_BEGIN
3739

@@ -338,10 +340,10 @@ - (void)testWillWaitForSyncIfOnline {
338340
FSTViewSnapshot *snap3 =
339341
FSTTestApplyChanges(view, @[], FSTTestTargetChangeAckDocuments({doc1.key, doc2.key}));
340342

341-
[listener applyChangedOnlineState:FSTOnlineStateOnline]; // no event
343+
[listener applyChangedOnlineState:OnlineState::Online]; // no event
342344
[listener queryDidChangeViewSnapshot:snap1];
343-
[listener applyChangedOnlineState:FSTOnlineStateUnknown];
344-
[listener applyChangedOnlineState:FSTOnlineStateOnline];
345+
[listener applyChangedOnlineState:OnlineState::Unknown];
346+
[listener applyChangedOnlineState:OnlineState::Online];
345347
[listener queryDidChangeViewSnapshot:snap2];
346348
[listener queryDidChangeViewSnapshot:snap3];
347349

@@ -377,12 +379,12 @@ - (void)testWillRaiseInitialEventWhenGoingOffline {
377379
FSTViewSnapshot *snap1 = FSTTestApplyChanges(view, @[ doc1 ], nil);
378380
FSTViewSnapshot *snap2 = FSTTestApplyChanges(view, @[ doc2 ], nil);
379381

380-
[listener applyChangedOnlineState:FSTOnlineStateOnline]; // no event
381-
[listener queryDidChangeViewSnapshot:snap1]; // no event
382-
[listener applyChangedOnlineState:FSTOnlineStateOffline]; // event
383-
[listener applyChangedOnlineState:FSTOnlineStateUnknown]; // no event
384-
[listener applyChangedOnlineState:FSTOnlineStateOffline]; // no event
385-
[listener queryDidChangeViewSnapshot:snap2]; // another event
382+
[listener applyChangedOnlineState:OnlineState::Online]; // no event
383+
[listener queryDidChangeViewSnapshot:snap1]; // no event
384+
[listener applyChangedOnlineState:OnlineState::Offline]; // event
385+
[listener applyChangedOnlineState:OnlineState::Unknown]; // no event
386+
[listener applyChangedOnlineState:OnlineState::Offline]; // no event
387+
[listener queryDidChangeViewSnapshot:snap2]; // another event
386388

387389
FSTDocumentViewChange *change1 =
388390
[FSTDocumentViewChange changeWithDocument:doc1 type:FSTDocumentViewChangeTypeAdded];
@@ -417,9 +419,9 @@ - (void)testWillRaiseInitialEventWhenGoingOfflineAndThereAreNoDocs {
417419
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
418420
FSTViewSnapshot *snap1 = FSTTestApplyChanges(view, @[], nil);
419421

420-
[listener applyChangedOnlineState:FSTOnlineStateOnline]; // no event
421-
[listener queryDidChangeViewSnapshot:snap1]; // no event
422-
[listener applyChangedOnlineState:FSTOnlineStateOffline]; // event
422+
[listener applyChangedOnlineState:OnlineState::Online]; // no event
423+
[listener queryDidChangeViewSnapshot:snap1]; // no event
424+
[listener applyChangedOnlineState:OnlineState::Offline]; // event
423425

424426
FSTViewSnapshot *expectedSnap = [[FSTViewSnapshot alloc]
425427
initWithQuery:query
@@ -443,8 +445,8 @@ - (void)testWillRaiseInitialEventWhenStartingOfflineAndThereAreNoDocs {
443445
FSTView *view = [[FSTView alloc] initWithQuery:query remoteDocuments:DocumentKeySet{}];
444446
FSTViewSnapshot *snap1 = FSTTestApplyChanges(view, @[], nil);
445447

446-
[listener applyChangedOnlineState:FSTOnlineStateOffline]; // no event
447-
[listener queryDidChangeViewSnapshot:snap1]; // event
448+
[listener applyChangedOnlineState:OnlineState::Offline]; // no event
449+
[listener queryDidChangeViewSnapshot:snap1]; // event
448450

449451
FSTViewSnapshot *expectedSnap = [[FSTViewSnapshot alloc]
450452
initWithQuery:query

‎Firestore/Example/Tests/SpecTests/FSTMockDatastore.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#import "Firestore/Source/Remote/FSTDatastore.h"
2020

2121
#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h"
22+
#include "Firestore/core/src/firebase/firestore/model/types.h"
2223

2324
NS_ASSUME_NONNULL_BEGIN
2425

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
using firebase::firestore::core::DatabaseInfo;
4949
using firebase::firestore::model::DatabaseId;
5050
using firebase::firestore::model::DocumentKey;
51+
using firebase::firestore::model::OnlineState;
5152
using firebase::firestore::model::SnapshotVersion;
5253
using firebase::firestore::model::TargetId;
5354

@@ -180,7 +181,7 @@ - (void)drainQueue {
180181
return _currentUser;
181182
}
182183

183-
- (void)applyChangedOnlineState:(FSTOnlineState)onlineState {
184+
- (void)applyChangedOnlineState:(OnlineState)onlineState {
184185
[self.syncEngine applyChangedOnlineState:onlineState];
185186
[self.eventManager applyChangedOnlineState:onlineState];
186187
}

‎Firestore/Source/Core/FSTEventManager.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ NS_ASSUME_NONNULL_BEGIN
6363

6464
- (void)queryDidChangeViewSnapshot:(FSTViewSnapshot *)snapshot;
6565
- (void)queryDidError:(NSError *)error;
66-
- (void)applyChangedOnlineState:(FSTOnlineState)onlineState;
66+
- (void)applyChangedOnlineState:(firebase::firestore::model::OnlineState)onlineState;
6767

6868
@property(nonatomic, strong, readonly) FSTQuery *query;
6969

‎Firestore/Source/Core/FSTEventManager.mm

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
2424

25+
using firebase::firestore::model::OnlineState;
2526
using firebase::firestore::model::TargetId;
2627

2728
NS_ASSUME_NONNULL_BEGIN
@@ -97,7 +98,7 @@ @interface FSTQueryListener ()
9798
@property(nonatomic, assign, readwrite) BOOL raisedInitialEvent;
9899

99100
/** The last online state this query listener got. */
100-
@property(nonatomic, assign, readwrite) FSTOnlineState onlineState;
101+
@property(nonatomic, assign, readwrite) OnlineState onlineState;
101102

102103
/** The FSTViewSnapshotHandler associated with this query listener. */
103104
@property(nonatomic, copy, nullable) FSTViewSnapshotHandler viewSnapshotHandler;
@@ -154,7 +155,7 @@ - (void)queryDidError:(NSError *)error {
154155
self.viewSnapshotHandler(nil, error);
155156
}
156157

157-
- (void)applyChangedOnlineState:(FSTOnlineState)onlineState {
158+
- (void)applyChangedOnlineState:(OnlineState)onlineState {
158159
self.onlineState = onlineState;
159160
if (self.snapshot && !self.raisedInitialEvent &&
160161
[self shouldRaiseInitialEventForSnapshot:self.snapshot onlineState:onlineState]) {
@@ -163,7 +164,7 @@ - (void)applyChangedOnlineState:(FSTOnlineState)onlineState {
163164
}
164165

165166
- (BOOL)shouldRaiseInitialEventForSnapshot:(FSTViewSnapshot *)snapshot
166-
onlineState:(FSTOnlineState)onlineState {
167+
onlineState:(OnlineState)onlineState {
167168
HARD_ASSERT(!self.raisedInitialEvent,
168169
"Determining whether to raise initial event, but already had first event.");
169170

@@ -174,7 +175,7 @@ - (BOOL)shouldRaiseInitialEventForSnapshot:(FSTViewSnapshot *)snapshot
174175

175176
// NOTE: We consider OnlineState.Unknown as online (it should become Offline or Online if we
176177
// wait long enough).
177-
BOOL maybeOnline = onlineState != FSTOnlineStateOffline;
178+
BOOL maybeOnline = onlineState != OnlineState::Offline;
178179
// Don't raise the event if we're online, aren't synced yet (checked
179180
// above) and are waiting for a sync.
180181
if (self.options.waitForSyncWhenOnline && maybeOnline) {
@@ -183,7 +184,7 @@ - (BOOL)shouldRaiseInitialEventForSnapshot:(FSTViewSnapshot *)snapshot
183184
}
184185

185186
// Raise data from cache if we have any documents or we are offline
186-
return !snapshot.documents.isEmpty || onlineState == FSTOnlineStateOffline;
187+
return !snapshot.documents.isEmpty || onlineState == OnlineState::Offline;
187188
}
188189

189190
- (BOOL)shouldRaiseEventForSnapshot:(FSTViewSnapshot *)snapshot {
@@ -239,7 +240,7 @@ - (instancetype)initWithSyncEngine:(FSTSyncEngine *)syncEngine NS_DESIGNATED_INI
239240
@property(nonatomic, strong, readonly) FSTSyncEngine *syncEngine;
240241
@property(nonatomic, strong, readonly)
241242
NSMutableDictionary<FSTQuery *, FSTQueryListenersInfo *> *queries;
242-
@property(nonatomic, assign) FSTOnlineState onlineState;
243+
@property(nonatomic, assign) OnlineState onlineState;
243244

244245
@end
245246

@@ -324,7 +325,7 @@ - (void)handleError:(NSError *)error forQuery:(FSTQuery *)query {
324325
[self.queries removeObjectForKey:query];
325326
}
326327

327-
- (void)applyChangedOnlineState:(FSTOnlineState)onlineState {
328+
- (void)applyChangedOnlineState:(OnlineState)onlineState {
328329
self.onlineState = onlineState;
329330
for (FSTQueryListenersInfo *info in self.queries.objectEnumerator) {
330331
for (FSTQueryListener *listener in info.listeners) {

‎Firestore/Source/Core/FSTFirestoreClient.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
*/
1616

1717
#import <Foundation/Foundation.h>
18+
1819
#include <memory>
1920

21+
#import "Firestore/Source/Core/FSTTypes.h"
2022
#import "Firestore/Source/Core/FSTViewSnapshot.h"
2123
#import "Firestore/Source/Remote/FSTRemoteStore.h"
2224

‎Firestore/Source/Core/FSTFirestoreClient.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
using firebase::firestore::core::DatabaseInfo;
5757
using firebase::firestore::model::DatabaseId;
5858
using firebase::firestore::model::DocumentKeySet;
59-
59+
using firebase::firestore::model::OnlineState;
6060
using firebase::firestore::util::internal::Executor;
6161

6262
NS_ASSUME_NONNULL_BEGIN
@@ -221,7 +221,7 @@ - (void)credentialDidChangeWithUser:(const User &)user {
221221
[self.syncEngine credentialDidChangeWithUser:user];
222222
}
223223

224-
- (void)applyChangedOnlineState:(FSTOnlineState)onlineState {
224+
- (void)applyChangedOnlineState:(OnlineState)onlineState {
225225
[self.syncEngine applyChangedOnlineState:onlineState];
226226
[self.eventManager applyChangedOnlineState:onlineState];
227227
}

‎Firestore/Source/Core/FSTSyncEngine.h

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

1717
#import <Foundation/Foundation.h>
1818

19+
#import "Firestore/Source/Core/FSTTypes.h"
1920
#import "Firestore/Source/Remote/FSTRemoteStore.h"
2021

2122
#include "Firestore/core/src/firebase/firestore/auth/user.h"
@@ -102,8 +103,8 @@ NS_ASSUME_NONNULL_BEGIN
102103

103104
- (void)credentialDidChangeWithUser:(const firebase::firestore::auth::User &)user;
104105

105-
/** Applies an FSTOnlineState change to the sync engine and notifies any views of the change. */
106-
- (void)applyChangedOnlineState:(FSTOnlineState)onlineState;
106+
/** Applies an OnlineState change to the sync engine and notifies any views of the change. */
107+
- (void)applyChangedOnlineState:(firebase::firestore::model::OnlineState)onlineState;
107108

108109
@end
109110

‎Firestore/Source/Core/FSTSyncEngine.mm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
using firebase::firestore::model::DocumentKey;
5454
using firebase::firestore::model::DocumentKeySet;
5555
using firebase::firestore::model::ListenSequenceNumber;
56+
using firebase::firestore::model::OnlineState;
5657
using firebase::firestore::model::SnapshotVersion;
5758
using firebase::firestore::model::TargetId;
5859

@@ -346,7 +347,7 @@ - (void)applyRemoteEvent:(FSTRemoteEvent *)remoteEvent {
346347
[self emitNewSnapshotsWithChanges:changes remoteEvent:remoteEvent];
347348
}
348349

349-
- (void)applyChangedOnlineState:(FSTOnlineState)onlineState {
350+
- (void)applyChangedOnlineState:(OnlineState)onlineState {
350351
NSMutableArray<FSTViewSnapshot *> *newViewSnapshots = [NSMutableArray array];
351352
[self.queryViewsByQuery
352353
enumerateKeysAndObjectsUsingBlock:^(FSTQuery *query, FSTQueryView *queryView, BOOL *stop) {

‎Firestore/Source/Core/FSTTypes.h

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616

1717
#import <Foundation/Foundation.h>
1818

19-
#include "Firestore/core/src/firebase/firestore/model/types.h"
20-
2119
NS_ASSUME_NONNULL_BEGIN
2220

2321
@class FSTMaybeDocument;
@@ -58,34 +56,4 @@ typedef void (^FSTVoidMaybeDocumentArrayErrorBlock)(
5856
typedef void (^FSTTransactionBlock)(FSTTransaction *transaction,
5957
void (^completion)(id _Nullable, NSError *_Nullable));
6058

61-
/**
62-
* Describes the online state of the Firestore client. Note that this does not indicate whether
63-
* or not the remote store is trying to connect or not. This is primarily used by the View /
64-
* EventManager code to change their behavior while offline (e.g. get() calls shouldn't wait for
65-
* data from the server and snapshot events should set metadata.isFromCache=true).
66-
*/
67-
typedef NS_ENUM(NSUInteger, FSTOnlineState) {
68-
/**
69-
* The Firestore client is in an unknown online state. This means the client is either not
70-
* actively trying to establish a connection or it is currently trying to establish a connection,
71-
* but it has not succeeded or failed yet. Higher-level components should not operate in
72-
* offline mode.
73-
*/
74-
FSTOnlineStateUnknown,
75-
76-
/**
77-
* The client is connected and the connections are healthy. This state is reached after a
78-
* successful connection and there has been at least one successful message received from the
79-
* backends.
80-
*/
81-
FSTOnlineStateOnline,
82-
83-
/**
84-
* The client is either trying to establish a connection but failing, or it has been explicitly
85-
* marked offline via a call to `disableNetwork`. Higher-level components should operate in
86-
* offline mode.
87-
*/
88-
FSTOnlineStateOffline
89-
};
90-
9159
NS_ASSUME_NONNULL_END

‎Firestore/Source/Core/FSTView.h

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

1717
#import <Foundation/Foundation.h>
1818

19-
#import "Firestore/Source/Core/FSTTypes.h"
2019
#import "Firestore/Source/Model/FSTDocumentDictionary.h"
2120

2221
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
2322
#include "Firestore/core/src/firebase/firestore/model/document_key_set.h"
23+
#include "Firestore/core/src/firebase/firestore/model/types.h"
2424

2525
@class FSTDocumentSet;
2626
@class FSTDocumentViewChangeSet;
@@ -144,10 +144,10 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) {
144144
targetChange:(nullable FSTTargetChange *)targetChange;
145145

146146
/**
147-
* Applies an FSTOnlineState change to the view, potentially generating an FSTViewChange if the
147+
* Applies an OnlineState change to the view, potentially generating an FSTViewChange if the
148148
* view's syncState changes as a result.
149149
*/
150-
- (FSTViewChange *)applyChangedOnlineState:(FSTOnlineState)onlineState;
150+
- (FSTViewChange *)applyChangedOnlineState:(firebase::firestore::model::OnlineState)onlineState;
151151

152152
/**
153153
* The set of remote documents that the server has told us belongs to the target associated with

‎Firestore/Source/Core/FSTView.mm

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
using firebase::firestore::model::DocumentKey;
3232
using firebase::firestore::model::DocumentKeySet;
33+
using firebase::firestore::model::OnlineState;
3334

3435
NS_ASSUME_NONNULL_BEGIN
3536

@@ -355,8 +356,8 @@ - (FSTViewChange *)applyChangesToDocuments:(FSTViewDocumentChanges *)docChanges
355356
}
356357
}
357358

358-
- (FSTViewChange *)applyChangedOnlineState:(FSTOnlineState)onlineState {
359-
if (self.isCurrent && onlineState == FSTOnlineStateOffline) {
359+
- (FSTViewChange *)applyChangedOnlineState:(OnlineState)onlineState {
360+
if (self.isCurrent && onlineState == OnlineState::Offline) {
360361
// If we're offline, set `current` to NO and then call applyChanges to refresh our syncState
361362
// and generate an FSTViewChange as appropriate. We are guaranteed to get a new FSTTargetChange
362363
// that sets `current` back to YES once the client is back online.

0 commit comments

Comments
 (0)