diff options
author | Michael Lehenbauer <mikelehen@gmail.com> | 2017-12-15 14:56:03 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-12-15 14:56:03 -0800 |
commit | d4de7a6e86476991e2363dd09f623b2f0edfbee4 (patch) | |
tree | 182dfb0a5143b9465f9f01d7424982da8d0a775d /Firestore/Source | |
parent | 98e08bc0b883d24cf2a0e658924ddd14dbf07d65 (diff) |
b/68276665: Raise isFromCache=true events when offline (#567)
* Plumbs FSTOnlineState changes through to views.
* View sets this.current to false on FSTOnlineStateFailed, triggering
isFromCache=true events. It will automatically be returned to true
once the listen is reestablished and we get a new CURRENT message.
* Updated tests (and added one new one) to verify behavior.
* Unifies setOnlineStateToUnknown, setOnlineStateToHealthy, and
updateAndBroadcastOnlineState into a single updateOnlineState
method.
* Split disableNetwork into (public) disableNetwork and
(private) disableNetworkWithTargetOnlineState methods..
* Some miscellaneous comment cleanup.
* Add missing comment per CR feedback.
Diffstat (limited to 'Firestore/Source')
-rw-r--r-- | Firestore/Source/Core/FSTFirestoreClient.h | 2 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTFirestoreClient.m | 7 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTSyncEngine.h | 3 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTSyncEngine.m | 15 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTTypes.h | 9 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTView.h | 7 | ||||
-rw-r--r-- | Firestore/Source/Core/FSTView.m | 18 | ||||
-rw-r--r-- | Firestore/Source/Remote/FSTRemoteStore.m | 77 |
8 files changed, 97 insertions, 41 deletions
diff --git a/Firestore/Source/Core/FSTFirestoreClient.h b/Firestore/Source/Core/FSTFirestoreClient.h index 6a1e11b..0ecf2f6 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.h +++ b/Firestore/Source/Core/FSTFirestoreClient.h @@ -38,7 +38,7 @@ NS_ASSUME_NONNULL_BEGIN * SDK architecture. It is responsible for creating the worker queue that is shared by all of the * other components in the system. */ -@interface FSTFirestoreClient : NSObject +@interface FSTFirestoreClient : NSObject <FSTOnlineStateDelegate> /** * Creates and returns a FSTFirestoreClient with the given parameters. diff --git a/Firestore/Source/Core/FSTFirestoreClient.m b/Firestore/Source/Core/FSTFirestoreClient.m index 2e0e407..d8bdde7 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.m +++ b/Firestore/Source/Core/FSTFirestoreClient.m @@ -172,7 +172,7 @@ NS_ASSUME_NONNULL_BEGIN // Setup wiring for remote store. _remoteStore.syncEngine = _syncEngine; - _remoteStore.onlineStateDelegate = _eventManager; + _remoteStore.onlineStateDelegate = self; // NOTE: RemoteStore depends on LocalStore (for persisting stream tokens, refilling mutation // queue, etc.) so must be started after LocalStore. @@ -187,6 +187,11 @@ NS_ASSUME_NONNULL_BEGIN [self.syncEngine userDidChange:user]; } +- (void)watchStreamDidChangeOnlineState:(FSTOnlineState)onlineState { + [self.syncEngine applyOnlineStateChange:onlineState]; + [self.eventManager watchStreamDidChangeOnlineState:onlineState]; +} + - (void)disableNetworkWithCompletion:(nullable FSTVoidErrorBlock)completion { [self.workerDispatchQueue dispatchAsync:^{ [self.remoteStore disableNetwork]; diff --git a/Firestore/Source/Core/FSTSyncEngine.h b/Firestore/Source/Core/FSTSyncEngine.h index bb45196..316ac05 100644 --- a/Firestore/Source/Core/FSTSyncEngine.h +++ b/Firestore/Source/Core/FSTSyncEngine.h @@ -100,6 +100,9 @@ NS_ASSUME_NONNULL_BEGIN - (void)userDidChange:(FSTUser *)user; +/** Applies an FSTOnlineState change to the sync engine and notifies any views of the change. */ +- (void)applyOnlineStateChange:(FSTOnlineState)onlineState; + @end NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Core/FSTSyncEngine.m b/Firestore/Source/Core/FSTSyncEngine.m index 98658e4..aa1ccae 100644 --- a/Firestore/Source/Core/FSTSyncEngine.m +++ b/Firestore/Source/Core/FSTSyncEngine.m @@ -318,6 +318,21 @@ NS_ASSUME_NONNULL_BEGIN [self emitNewSnapshotsWithChanges:changes remoteEvent:remoteEvent]; } +- (void)applyOnlineStateChange:(FSTOnlineState)onlineState { + NSMutableArray<FSTViewSnapshot *> *newViewSnapshots = [NSMutableArray array]; + [self.queryViewsByQuery + enumerateKeysAndObjectsUsingBlock:^(FSTQuery *query, FSTQueryView *queryView, BOOL *stop) { + FSTViewChange *viewChange = [queryView.view applyOnlineStateChange:onlineState]; + FSTAssert(viewChange.limboChanges.count == 0, + @"OnlineState should not affect limbo documents."); + if (viewChange.snapshot) { + [newViewSnapshots addObject:viewChange.snapshot]; + } + }]; + + [self.delegate handleViewSnapshots:newViewSnapshots]; +} + - (void)rejectListenWithTargetID:(FSTBoxedTargetID *)targetID error:(NSError *)error { [self assertDelegateExistsForSelector:_cmd]; diff --git a/Firestore/Source/Core/FSTTypes.h b/Firestore/Source/Core/FSTTypes.h index c10f1bf..b47bd0b 100644 --- a/Firestore/Source/Core/FSTTypes.h +++ b/Firestore/Source/Core/FSTTypes.h @@ -67,8 +67,8 @@ typedef void (^FSTTransactionBlock)(FSTTransaction *transaction, typedef NS_ENUM(NSUInteger, FSTOnlineState) { /** * The Firestore client is in an unknown online state. This means the client is either not - * actively trying to establish a connection or it was previously in an unknown state and is - * trying to establish a connection. + * actively trying to establish a connection or it is currently trying to establish a connection, + * but it has not succeeded or failed yet. */ FSTOnlineStateUnknown, @@ -80,9 +80,8 @@ typedef NS_ENUM(NSUInteger, FSTOnlineState) { FSTOnlineStateHealthy, /** - * The client has tried to establish a connection but has failed. - * This state is reached after either a connection attempt failed or a healthy stream was closed - * for unexpected reasons. + * The client considers itself offline. It is either trying to establish a connection but + * failing, or it has been explicitly marked offline via a call to `disableNetwork`. */ FSTOnlineStateFailed }; diff --git a/Firestore/Source/Core/FSTView.h b/Firestore/Source/Core/FSTView.h index ed230a3..beadf46 100644 --- a/Firestore/Source/Core/FSTView.h +++ b/Firestore/Source/Core/FSTView.h @@ -16,6 +16,7 @@ #import <Foundation/Foundation.h> +#import "Firestore/Source/Core/FSTTypes.h" #import "Firestore/Source/Model/FSTDocumentDictionary.h" #import "Firestore/Source/Model/FSTDocumentKeySet.h" @@ -138,6 +139,12 @@ typedef NS_ENUM(NSInteger, FSTLimboDocumentChangeType) { - (FSTViewChange *)applyChangesToDocuments:(FSTViewDocumentChanges *)docChanges targetChange:(nullable FSTTargetChange *)targetChange; +/** + * Applies an FSTOnlineState change to the view, potentially generating an FSTViewChange if the + * view's syncState changes as a result. + */ +- (FSTViewChange *)applyOnlineStateChange:(FSTOnlineState)onlineState; + @end NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Core/FSTView.m b/Firestore/Source/Core/FSTView.m index 9b44bf4..d539bb6 100644 --- a/Firestore/Source/Core/FSTView.m +++ b/Firestore/Source/Core/FSTView.m @@ -330,6 +330,24 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang } } +- (FSTViewChange *)applyOnlineStateChange:(FSTOnlineState)onlineState { + if (self.isCurrent && onlineState == FSTOnlineStateFailed) { + // If we're offline, set `current` to NO and then call applyChanges to refresh our syncState + // and generate an FSTViewChange as appropriate. We are guaranteed to get a new FSTTargetChange + // that sets `current` back to YES once the client is back online. + self.current = NO; + return + [self applyChangesToDocuments:[[FSTViewDocumentChanges alloc] + initWithDocumentSet:self.documentSet + changeSet:[FSTDocumentViewChangeSet changeSet] + needsRefill:NO + mutatedKeys:self.mutatedKeys]]; + } else { + // No effect, just return a no-op FSTViewChange. + return [[FSTViewChange alloc] initWithSnapshot:nil limboChanges:@[]]; + } +} + #pragma mark - Private methods /** Returns whether the doc for the given key should be in limbo. */ diff --git a/Firestore/Source/Remote/FSTRemoteStore.m b/Firestore/Source/Remote/FSTRemoteStore.m index 063e487..12cffd4 100644 --- a/Firestore/Source/Remote/FSTRemoteStore.m +++ b/Firestore/Source/Remote/FSTRemoteStore.m @@ -160,27 +160,39 @@ static const int kOnlineAttemptsBeforeFailure = 2; [self enableNetwork]; } -- (void)setOnlineStateToHealthy { - self.shouldWarnOffline = NO; - [self updateAndNotifyAboutOnlineState:FSTOnlineStateHealthy]; -} +/** + * Updates our OnlineState to the new state, updating local state and notifying the + * onlineStateHandler as appropriate. + */ +- (void)updateOnlineState:(FSTOnlineState)newState { + if (newState == FSTOnlineStateHealthy) { + // We've connected to watch at least once. Don't warn the developer about being offline going + // forward. + self.shouldWarnOffline = NO; + } else if (newState == FSTOnlineStateUnknown) { + // The state is set to unknown when a healthy stream is closed (e.g. due to a token timeout) or + // when we have no active listens and therefore there's no need to start the stream. Assuming + // there is (possibly in the future) an active listen, then we will eventually move to state + // Online or Failed, but we always want to make at least kOnlineAttemptsBeforeFailure attempts + // before failing, so we reset the count here. + self.watchStreamFailures = 0; + } -- (void)setOnlineStateToUnknown { - // The state is set to unknown when a healthy stream is closed (e.g. due to a token timeout) or - // when we have no active listens and therefore there's no need to start the stream. Assuming - // there is (possibly in the future) an active listen, then we will eventually move to state - // Online or Failed, but we always want to make at least kOnlineAttemptsBeforeFailure attempts - // before failing, so we reset the count here. - self.watchStreamFailures = 0; - [self updateAndNotifyAboutOnlineState:FSTOnlineStateUnknown]; + // Update and broadcast the new state. + if (newState != self.watchStreamOnlineState) { + self.watchStreamOnlineState = newState; + [self.onlineStateDelegate watchStreamDidChangeOnlineState:newState]; + } } +/** + * Updates our FSTOnlineState as appropriate after the watch stream reports a failure. The first + * failure moves us to the 'Unknown' state. We then may allow multiple failures (based on + * kOnlineAttemptsBeforeFailure) before we actually transition to FSTOnlineStateFailed. + */ - (void)updateOnlineStateAfterFailure { - // The first failure after we are successfully connected moves us to the 'Unknown' state. We - // then may make multiple attempts (based on kOnlineAttemptsBeforeFailure) before we actually - // report failure. if (self.watchStreamOnlineState == FSTOnlineStateHealthy) { - [self setOnlineStateToUnknown]; + [self updateOnlineState:FSTOnlineStateUnknown]; } else { self.watchStreamFailures++; if (self.watchStreamFailures >= kOnlineAttemptsBeforeFailure) { @@ -188,19 +200,11 @@ static const int kOnlineAttemptsBeforeFailure = 2; FSTWarn(@"Could not reach Firestore backend."); self.shouldWarnOffline = NO; } - [self updateAndNotifyAboutOnlineState:FSTOnlineStateFailed]; + [self updateOnlineState:FSTOnlineStateFailed]; } } } -- (void)updateAndNotifyAboutOnlineState:(FSTOnlineState)watchStreamOnlineState { - BOOL didChange = (watchStreamOnlineState != self.watchStreamOnlineState); - self.watchStreamOnlineState = watchStreamOnlineState; - if (didChange) { - [self.onlineStateDelegate watchStreamDidChangeOnlineState:watchStreamOnlineState]; - } -} - #pragma mark Online/Offline state - (BOOL)isNetworkEnabled { @@ -227,12 +231,16 @@ static const int kOnlineAttemptsBeforeFailure = 2; [self fillWritePipeline]; // This may start the writeStream. // We move back to the unknown state because we might not want to re-open the stream - [self setOnlineStateToUnknown]; + [self updateOnlineState:FSTOnlineStateUnknown]; } - (void)disableNetwork { - [self updateAndNotifyAboutOnlineState:FSTOnlineStateFailed]; + // Set the FSTOnlineState to failed so get()'s return from cache, etc. + [self disableNetworkWithTargetOnlineState:FSTOnlineStateFailed]; +} +/** Disables the network, setting the FSTOnlineState to the specified targetOnlineState. */ +- (void)disableNetworkWithTargetOnlineState:(FSTOnlineState)targetOnlineState { // NOTE: We're guaranteed not to get any further events from these streams (not even a close // event). [self.watchStream stop]; @@ -243,6 +251,8 @@ static const int kOnlineAttemptsBeforeFailure = 2; self.writeStream = nil; self.watchStream = nil; + + [self updateOnlineState:targetOnlineState]; } #pragma mark Shutdown @@ -250,13 +260,12 @@ static const int kOnlineAttemptsBeforeFailure = 2; - (void)shutdown { FSTLog(@"FSTRemoteStore %p shutting down", (__bridge void *)self); - // Don't fire initial listener callbacks on shutdown. - self.onlineStateDelegate = nil; - // For now, all shutdown logic is handled by disableNetwork(). We might expand on this in the // future. if ([self isNetworkEnabled]) { - [self disableNetwork]; + // Set the FSTOnlineState to Unknown (rather than Failed) to avoid potentially triggering + // spurious listener events with cached data, etc. + [self disableNetworkWithTargetOnlineState:FSTOnlineStateUnknown]; } } @@ -266,7 +275,7 @@ static const int kOnlineAttemptsBeforeFailure = 2; // Tear down and re-create our network streams. This will ensure we get a fresh auth token // for the new user and re-fill the write pipeline with new mutations from the LocalStore // (since mutations are per-user). - [self disableNetwork]; + [self disableNetworkWithTargetOnlineState:FSTOnlineStateUnknown]; [self enableNetwork]; } @@ -348,7 +357,7 @@ static const int kOnlineAttemptsBeforeFailure = 2; - (void)watchStreamDidChange:(FSTWatchChange *)change snapshotVersion:(FSTSnapshotVersion *)snapshotVersion { // Mark the connection as healthy because we got a message from the server. - [self setOnlineStateToHealthy]; + [self updateOnlineState:FSTOnlineStateHealthy]; FSTWatchTargetChange *watchTargetChange = [change isKindOfClass:[FSTWatchTargetChange class]] ? (FSTWatchTargetChange *)change : nil; @@ -391,7 +400,7 @@ static const int kOnlineAttemptsBeforeFailure = 2; } else { // We don't need to restart the watch stream because there are no active targets. The online // state is set to unknown because there is no active attempt at establishing a connection. - [self setOnlineStateToUnknown]; + [self updateOnlineState:FSTOnlineStateUnknown]; } } |