From 34ebf10b0acc65f1924d723e82085d4104bc281d Mon Sep 17 00:00:00 2001 From: Michael Lehenbauer Date: Mon, 5 Mar 2018 09:49:41 -0800 Subject: Add 10 second timeout waiting for connection before client behaves as-if offline. (#872) [Port of https://github.com/firebase/firebase-js-sdk/commit/0fa319e5e019dd0d40ab441d2ff9f8f6d4724e43] * Refactored FSTOnlineState tracking out of FSTRemoteStore and into new FSTOnlineStateTracker component. * Added a 10 second timeout to transition from OnlineState.Unknown to OnlineState.Offline rather than waiting indefinitely for the stream to succeed or fail. * Removed hack to run SpecTests using an FSTDispatchQueue that wrapped dispatch_get_main_queue(). This was incompatible with [FSTDispatchQueue runDelayedCallbacksUntil:] since it queues work and blocks waiting for it to complete. Now spec tests create / use a proper FSTDispatchQueue. * Added a SpecTest to verify OnlineState timeout behavior. * Misc cleanup: * Renamed FSTOnlineState states: Failed => Offline, Healthy => Online * Renamed FSTTimerIds (ListenStreamConnection => ListenStreamConnectionBackoff) * Added ability to run timers from spec tests. --- Firestore/Source/Core/FSTEventManager.mm | 8 +- Firestore/Source/Core/FSTFirestoreClient.mm | 4 +- Firestore/Source/Core/FSTTypes.h | 19 ++- Firestore/Source/Core/FSTView.mm | 2 +- Firestore/Source/Remote/FSTOnlineStateTracker.h | 71 +++++++++++ Firestore/Source/Remote/FSTOnlineStateTracker.mm | 149 +++++++++++++++++++++++ Firestore/Source/Remote/FSTRemoteStore.h | 8 +- Firestore/Source/Remote/FSTRemoteStore.mm | 117 ++++-------------- Firestore/Source/Remote/FSTStream.mm | 4 +- Firestore/Source/Util/FSTDispatchQueue.h | 26 +++- Firestore/Source/Util/FSTDispatchQueue.mm | 4 + 11 files changed, 301 insertions(+), 111 deletions(-) create mode 100644 Firestore/Source/Remote/FSTOnlineStateTracker.h create mode 100644 Firestore/Source/Remote/FSTOnlineStateTracker.mm (limited to 'Firestore/Source') diff --git a/Firestore/Source/Core/FSTEventManager.mm b/Firestore/Source/Core/FSTEventManager.mm index bc204a0..b02fc5f 100644 --- a/Firestore/Source/Core/FSTEventManager.mm +++ b/Firestore/Source/Core/FSTEventManager.mm @@ -169,9 +169,9 @@ NS_ASSUME_NONNULL_BEGIN return YES; } - // NOTE: We consider OnlineState.Unknown as online (it should become Failed - // or Online if we wait long enough). - BOOL maybeOnline = onlineState != FSTOnlineStateFailed; + // NOTE: We consider OnlineState.Unknown as online (it should become Offline or Online if we + // wait long enough). + BOOL maybeOnline = onlineState != FSTOnlineStateOffline; // Don't raise the event if we're online, aren't synced yet (checked // above) and are waiting for a sync. if (self.options.waitForSyncWhenOnline && maybeOnline) { @@ -180,7 +180,7 @@ NS_ASSUME_NONNULL_BEGIN } // Raise data from cache if we have any documents or we are offline - return !snapshot.documents.isEmpty || onlineState == FSTOnlineStateFailed; + return !snapshot.documents.isEmpty || onlineState == FSTOnlineStateOffline; } - (BOOL)shouldRaiseEventForSnapshot:(FSTViewSnapshot *)snapshot { diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm index fb86e0b..288cbe2 100644 --- a/Firestore/Source/Core/FSTFirestoreClient.mm +++ b/Firestore/Source/Core/FSTFirestoreClient.mm @@ -181,7 +181,9 @@ NS_ASSUME_NONNULL_BEGIN workerDispatchQueue:self.workerDispatchQueue credentials:_credentialsProvider]; - _remoteStore = [FSTRemoteStore remoteStoreWithLocalStore:_localStore datastore:datastore]; + _remoteStore = [[FSTRemoteStore alloc] initWithLocalStore:_localStore + datastore:datastore + workerDispatchQueue:self.workerDispatchQueue]; _syncEngine = [[FSTSyncEngine alloc] initWithLocalStore:_localStore remoteStore:_remoteStore diff --git a/Firestore/Source/Core/FSTTypes.h b/Firestore/Source/Core/FSTTypes.h index 877ec94..688b2bf 100644 --- a/Firestore/Source/Core/FSTTypes.h +++ b/Firestore/Source/Core/FSTTypes.h @@ -65,12 +65,18 @@ typedef void (^FSTVoidMaybeDocumentArrayErrorBlock)( typedef void (^FSTTransactionBlock)(FSTTransaction *transaction, void (^completion)(id _Nullable, NSError *_Nullable)); -/** Describes the online state of the Firestore client */ +/** + * Describes the online state of the Firestore client. Note that this does not indicate whether + * or not the remote store is trying to connect or not. This is primarily used by the View / + * EventManager code to change their behavior while offline (e.g. get() calls shouldn't wait for + * data from the server and snapshot events should set metadata.isFromCache=true). + */ 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 is currently trying to establish a connection, - * but it has not succeeded or failed yet. + * but it has not succeeded or failed yet. Higher-level components should not operate in + * offline mode. */ FSTOnlineStateUnknown, @@ -79,13 +85,14 @@ typedef NS_ENUM(NSUInteger, FSTOnlineState) { * successful connection and there has been at least one successful message received from the * backends. */ - FSTOnlineStateHealthy, + FSTOnlineStateOnline, /** - * 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`. + * The client is either trying to establish a connection but failing, or it has been explicitly + * marked offline via a call to `disableNetwork`. Higher-level components should operate in + * offline mode. */ - FSTOnlineStateFailed + FSTOnlineStateOffline }; NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Core/FSTView.mm b/Firestore/Source/Core/FSTView.mm index d6b4558..b2a39cb 100644 --- a/Firestore/Source/Core/FSTView.mm +++ b/Firestore/Source/Core/FSTView.mm @@ -337,7 +337,7 @@ static NSComparisonResult FSTCompareDocumentViewChangeTypes(FSTDocumentViewChang } - (FSTViewChange *)applyChangedOnlineState:(FSTOnlineState)onlineState { - if (self.isCurrent && onlineState == FSTOnlineStateFailed) { + if (self.isCurrent && onlineState == FSTOnlineStateOffline) { // 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. diff --git a/Firestore/Source/Remote/FSTOnlineStateTracker.h b/Firestore/Source/Remote/FSTOnlineStateTracker.h new file mode 100644 index 0000000..a414a18 --- /dev/null +++ b/Firestore/Source/Remote/FSTOnlineStateTracker.h @@ -0,0 +1,71 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import + +#import "Firestore/Source/Core/FSTTypes.h" + +@class FSTDispatchQueue; +@protocol FSTOnlineStateDelegate; + +NS_ASSUME_NONNULL_BEGIN + +/** + * A component used by the FSTRemoteStore to track the FSTOnlineState (that is, whether or not the + * client as a whole should be considered to be online or offline), implementing the appropriate + * heuristics. + * + * In particular, when the client is trying to connect to the backend, we allow up to + * kMaxWatchStreamFailures within kOnlineStateTimeout for a connection to succeed. If we have too + * many failures or the timeout elapses, then we set the FSTOnlineState to Offline, and + * the client will behave as if it is offline (getDocument() calls will return cached data, etc.). + */ +@interface FSTOnlineStateTracker : NSObject + +- (instancetype)initWithWorkerDispatchQueue:(FSTDispatchQueue *)queue; + +- (instancetype)init NS_UNAVAILABLE; + +/** A delegate to be notified on FSTOnlineState changes. */ +@property(nonatomic, weak) id onlineStateDelegate; + +/** + * Called by FSTRemoteStore when a watch stream is started. + * + * It sets the FSTOnlineState to Unknown and starts the onlineStateTimer if necessary. + */ +- (void)handleWatchStreamStart; + +/** + * Called by FSTRemoteStore when a watch stream fails. + * + * Updates our FSTOnlineState as appropriate. The first failure moves us to FSTOnlineStateUnknown. + * We then may allow multiple failures (based on kMaxWatchStreamFailures) before we actually + * transition to FSTOnlineStateOffline. + */ +- (void)handleWatchStreamFailure; + +/** + * Explicitly sets the FSTOnlineState to the specified state. + * + * Note that this resets the timers / failure counters, etc. used by our Offline heuristics, so + * it must not be used in place of handleWatchStreamStart and handleWatchStreamFailure. + */ +- (void)updateState:(FSTOnlineState)newState; + +@end + +NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Remote/FSTOnlineStateTracker.mm b/Firestore/Source/Remote/FSTOnlineStateTracker.mm new file mode 100644 index 0000000..41650cd --- /dev/null +++ b/Firestore/Source/Remote/FSTOnlineStateTracker.mm @@ -0,0 +1,149 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#import "Firestore/Source/Remote/FSTOnlineStateTracker.h" +#import "Firestore/Source/Remote/FSTRemoteStore.h" +#import "Firestore/Source/Util/FSTAssert.h" +#import "Firestore/Source/Util/FSTDispatchQueue.h" +#import "Firestore/Source/Util/FSTLogger.h" + +NS_ASSUME_NONNULL_BEGIN + +// To deal with transient failures, we allow multiple stream attempts before giving up and +// transitioning from FSTOnlineState Unknown to Offline. +static const int kMaxWatchStreamFailures = 2; + +// To deal with stream attempts that don't succeed or fail in a timely manner, we have a +// timeout for FSTOnlineState to reach Online or Offline. If the timeout is reached, we transition +// to Offline rather than waiting indefinitely. +static const NSTimeInterval kOnlineStateTimeout = 10; + +@interface FSTOnlineStateTracker () + +/** The current FSTOnlineState. */ +@property(nonatomic, assign) FSTOnlineState state; + +/** + * A count of consecutive failures to open the stream. If it reaches the maximum defined by + * kMaxWatchStreamFailures, we'll revert to FSTOnlineStateOffline. + */ +@property(nonatomic, assign) int watchStreamFailures; + +/** + * A timer that elapses after kOnlineStateTimeout, at which point we transition from FSTOnlineState + * Unknown to Offline without waiting for the stream to actually fail (kMaxWatchStreamFailures + * times). + */ +@property(nonatomic, strong, nullable) FSTDelayedCallback *watchStreamTimer; + +/** + * Whether the client should log a warning message if it fails to connect to the backend + * (initially YES, cleared after a successful stream, or if we've logged the message already). + */ +@property(nonatomic, assign) BOOL shouldWarnClientIsOffline; + +/** The FSTDispatchQueue to use for running timers (and to call onlineStateDelegate). */ +@property(nonatomic, strong, readonly) FSTDispatchQueue *queue; + +@end + +@implementation FSTOnlineStateTracker +- (instancetype)initWithWorkerDispatchQueue:(FSTDispatchQueue *)queue { + if (self = [super init]) { + _queue = queue; + _state = FSTOnlineStateUnknown; + _shouldWarnClientIsOffline = YES; + } + return self; +} + +- (void)handleWatchStreamStart { + [self setAndBroadcastState:FSTOnlineStateUnknown]; + + if (self.watchStreamTimer == nil) { + self.watchStreamTimer = [self.queue + dispatchAfterDelay:kOnlineStateTimeout + timerID:FSTTimerIDOnlineStateTimeout + block:^{ + self.watchStreamTimer = nil; + FSTAssert( + self.state == FSTOnlineStateUnknown, + @"Timer should be canceled if we transitioned to a different state."); + FSTLog( + @"Watch stream didn't reach Online or Offline within %f seconds. " + @"Considering " + "client offline.", + kOnlineStateTimeout); + [self logClientOfflineWarningIfNecessary]; + [self setAndBroadcastState:FSTOnlineStateOffline]; + + // NOTE: handleWatchStreamFailure will continue to increment + // watchStreamFailures even though we are already marked Offline but this is + // non-harmful. + }]; + } +} + +- (void)handleWatchStreamFailure { + if (self.state == FSTOnlineStateOnline) { + [self setAndBroadcastState:FSTOnlineStateUnknown]; + } else { + self.watchStreamFailures++; + if (self.watchStreamFailures >= kMaxWatchStreamFailures) { + [self clearOnlineStateTimer]; + [self logClientOfflineWarningIfNecessary]; + [self setAndBroadcastState:FSTOnlineStateOffline]; + } + } +} + +- (void)updateState:(FSTOnlineState)newState { + [self clearOnlineStateTimer]; + self.watchStreamFailures = 0; + + if (newState == FSTOnlineStateOnline) { + // We've connected to watch at least once. Don't warn the developer about being offline going + // forward. + self.shouldWarnClientIsOffline = NO; + } + + [self setAndBroadcastState:newState]; +} + +- (void)setAndBroadcastState:(FSTOnlineState)newState { + if (newState != self.state) { + self.state = newState; + [self.onlineStateDelegate applyChangedOnlineState:newState]; + } +} + +- (void)logClientOfflineWarningIfNecessary { + if (self.shouldWarnClientIsOffline) { + FSTWarn(@"Could not reach Firestore backend."); + self.shouldWarnClientIsOffline = NO; + } +} + +- (void)clearOnlineStateTimer { + if (self.watchStreamTimer) { + [self.watchStreamTimer cancel]; + self.watchStreamTimer = nil; + } +} + +@end + +NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Remote/FSTRemoteStore.h b/Firestore/Source/Remote/FSTRemoteStore.h index 4ea9379..6f4d565 100644 --- a/Firestore/Source/Remote/FSTRemoteStore.h +++ b/Firestore/Source/Remote/FSTRemoteStore.h @@ -30,6 +30,7 @@ @class FSTQueryData; @class FSTRemoteEvent; @class FSTTransaction; +@class FSTDispatchQueue; NS_ASSUME_NONNULL_BEGIN @@ -95,10 +96,11 @@ NS_ASSUME_NONNULL_BEGIN */ @interface FSTRemoteStore : NSObject -+ (instancetype)remoteStoreWithLocalStore:(FSTLocalStore *)localStore - datastore:(FSTDatastore *)datastore; +- (instancetype)initWithLocalStore:(FSTLocalStore *)localStore + datastore:(FSTDatastore *)datastore + workerDispatchQueue:(FSTDispatchQueue *)queue; -- (instancetype)init __attribute__((unavailable("Use static constructor method."))); +- (instancetype)init NS_UNAVAILABLE; @property(nonatomic, weak) id syncEngine; diff --git a/Firestore/Source/Remote/FSTRemoteStore.mm b/Firestore/Source/Remote/FSTRemoteStore.mm index b762722..081b90e 100644 --- a/Firestore/Source/Remote/FSTRemoteStore.mm +++ b/Firestore/Source/Remote/FSTRemoteStore.mm @@ -30,6 +30,7 @@ #import "Firestore/Source/Model/FSTPath.h" #import "Firestore/Source/Remote/FSTDatastore.h" #import "Firestore/Source/Remote/FSTExistenceFilter.h" +#import "Firestore/Source/Remote/FSTOnlineStateTracker.h" #import "Firestore/Source/Remote/FSTRemoteEvent.h" #import "Firestore/Source/Remote/FSTStream.h" #import "Firestore/Source/Remote/FSTWatchChange.h" @@ -50,21 +51,10 @@ NS_ASSUME_NONNULL_BEGIN */ static const int kMaxPendingWrites = 10; -/** - * The FSTRemoteStore notifies an onlineStateDelegate with FSTOnlineStateFailed if we fail to - * connect to the backend. This subsequently triggers get() requests to fail or use cached data, - * etc. Unfortunately, our connections have historically been subject to various transient failures. - * So we wait for multiple failures before notifying the onlineStateDelegate. - */ -static const int kOnlineAttemptsBeforeFailure = 2; - #pragma mark - FSTRemoteStore @interface FSTRemoteStore () -- (instancetype)initWithLocalStore:(FSTLocalStore *)localStore - datastore:(FSTDatastore *)datastore NS_DESIGNATED_INITIALIZER; - /** * The local store, used to fill the write pipeline with outbound mutations and resolve existence * filter mismatches. Immutable after initialization. @@ -110,29 +100,13 @@ static const int kOnlineAttemptsBeforeFailure = 2; @property(nonatomic, strong) NSMutableArray *accumulatedChanges; @property(nonatomic, assign) FSTBatchID lastBatchSeen; -/** - * The online state of the watch stream. The state is set to healthy if and only if there are - * messages received by the backend. - */ -@property(nonatomic, assign) FSTOnlineState watchStreamOnlineState; - -/** A count of consecutive failures to open the stream. */ -@property(nonatomic, assign) int watchStreamFailures; - -/** Whether the client should fire offline warning. */ -@property(nonatomic, assign) BOOL shouldWarnOffline; +@property(nonatomic, strong, readonly) FSTOnlineStateTracker *onlineStateTracker; #pragma mark Write Stream // The writeStream is null when the network is disabled. The non-null check is performed by // isNetworkEnabled. @property(nonatomic, strong, nullable) FSTWriteStream *writeStream; -/** - * The approximate time the StreamingWrite stream was opened. Used to estimate if stream was - * closed due to an auth expiration (a recoverable error) or some other more permanent error. - */ -@property(nonatomic, strong, nullable) NSDate *writeStreamOpenTime; - /** * A FIFO queue of in-flight writes. This is in-flight from the point of view of the caller of * writeMutations, not from the point of view from the Datastore itself. In particular, these @@ -143,12 +117,9 @@ static const int kOnlineAttemptsBeforeFailure = 2; @implementation FSTRemoteStore -+ (instancetype)remoteStoreWithLocalStore:(FSTLocalStore *)localStore - datastore:(FSTDatastore *)datastore { - return [[FSTRemoteStore alloc] initWithLocalStore:localStore datastore:datastore]; -} - -- (instancetype)initWithLocalStore:(FSTLocalStore *)localStore datastore:(FSTDatastore *)datastore { +- (instancetype)initWithLocalStore:(FSTLocalStore *)localStore + datastore:(FSTDatastore *)datastore + workerDispatchQueue:(FSTDispatchQueue *)queue { if (self = [super init]) { _localStore = localStore; _datastore = datastore; @@ -157,9 +128,8 @@ static const int kOnlineAttemptsBeforeFailure = 2; _accumulatedChanges = [NSMutableArray array]; _lastBatchSeen = kFSTBatchIDUnknown; - _watchStreamOnlineState = FSTOnlineStateUnknown; - _shouldWarnOffline = YES; _pendingWrites = [NSMutableArray array]; + _onlineStateTracker = [[FSTOnlineStateTracker alloc] initWithWorkerDispatchQueue:queue]; } return self; } @@ -169,48 +139,14 @@ static const int kOnlineAttemptsBeforeFailure = 2; [self enableNetwork]; } -/** - * Updates our OnlineState to the new state, updating local state and notifying the - * onlineStateHandler as appropriate. - */ -- (void)updateOnlineState:(FSTOnlineState)newState { - // Update and broadcast the new state. - if (newState != self.watchStreamOnlineState) { - 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; - } - self.watchStreamOnlineState = newState; - [self.onlineStateDelegate applyChangedOnlineState:newState]; - } +@dynamic onlineStateDelegate; + +- (nullable id)onlineStateDelegate { + return self.onlineStateTracker.onlineStateDelegate; } -/** - * 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 { - if (self.watchStreamOnlineState == FSTOnlineStateHealthy) { - [self updateOnlineState:FSTOnlineStateUnknown]; - } else { - self.watchStreamFailures++; - if (self.watchStreamFailures >= kOnlineAttemptsBeforeFailure) { - if (self.shouldWarnOffline) { - FSTWarn(@"Could not reach Firestore backend."); - self.shouldWarnOffline = NO; - } - [self updateOnlineState:FSTOnlineStateFailed]; - } - } +- (void)setOnlineStateDelegate:(nullable id)delegate { + self.onlineStateTracker.onlineStateDelegate = delegate; } #pragma mark Online/Offline state @@ -235,18 +171,17 @@ static const int kOnlineAttemptsBeforeFailure = 2; if ([self shouldStartWatchStream]) { [self startWatchStream]; + } else { + [self.onlineStateTracker updateState:FSTOnlineStateUnknown]; } [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 updateOnlineState:FSTOnlineStateUnknown]; } - (void)disableNetwork { [self disableNetworkInternal]; - // Set the FSTOnlineState to failed so get()'s return from cache, etc. - [self updateOnlineState:FSTOnlineStateFailed]; + // Set the FSTOnlineState to Offline so get()s return from cache, etc. + [self.onlineStateTracker updateState:FSTOnlineStateOffline]; } /** Disables the network, setting the FSTOnlineState to the specified targetOnlineState. */ @@ -270,9 +205,9 @@ static const int kOnlineAttemptsBeforeFailure = 2; - (void)shutdown { FSTLog(@"FSTRemoteStore %p shutting down", (__bridge void *)self); [self disableNetworkInternal]; - // Set the FSTOnlineState to Unknown (rather than Failed) to avoid potentially triggering + // Set the FSTOnlineState to Unknown (rather than Offline) to avoid potentially triggering // spurious listener events with cached data, etc. - [self updateOnlineState:FSTOnlineStateUnknown]; + [self.onlineStateTracker updateState:FSTOnlineStateUnknown]; } - (void)userDidChange:(const User &)user { @@ -283,7 +218,7 @@ static const int kOnlineAttemptsBeforeFailure = 2; // for the new user and re-fill the write pipeline with new mutations from the LocalStore // (since mutations are per-user). [self disableNetworkInternal]; - [self updateOnlineState:FSTOnlineStateUnknown]; + [self.onlineStateTracker updateState:FSTOnlineStateUnknown]; [self enableNetwork]; } } @@ -294,6 +229,7 @@ static const int kOnlineAttemptsBeforeFailure = 2; FSTAssert([self shouldStartWatchStream], @"startWatchStream: called when shouldStartWatchStream: is false."); [self.watchStream startWithDelegate:self]; + [self.onlineStateTracker handleWatchStreamStart]; } - (void)listenToTargetWithQueryData:(FSTQueryData *)queryData { @@ -365,8 +301,8 @@ 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 updateOnlineState:FSTOnlineStateHealthy]; + // Mark the connection as Online because we got a message from the server. + [self.onlineStateTracker updateState:FSTOnlineStateOnline]; FSTWatchTargetChange *watchTargetChange = [change isKindOfClass:[FSTWatchTargetChange class]] ? (FSTWatchTargetChange *)change : nil; @@ -397,19 +333,20 @@ static const int kOnlineAttemptsBeforeFailure = 2; - (void)watchStreamWasInterruptedWithError:(nullable NSError *)error { FSTAssert([self isNetworkEnabled], - @"watchStreamDidClose should only be called when the network is enabled"); + @"watchStreamWasInterruptedWithError: should only be called when the network is " + "enabled"); [self cleanUpWatchStreamState]; + [self.onlineStateTracker handleWatchStreamFailure]; // If the watch stream closed due to an error, retry the connection if there are any active // watch targets. if ([self shouldStartWatchStream]) { - [self updateOnlineStateAfterFailure]; [self startWatchStream]; } 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 updateOnlineState:FSTOnlineStateUnknown]; + [self.onlineStateTracker updateState:FSTOnlineStateUnknown]; } } @@ -604,8 +541,6 @@ static const int kOnlineAttemptsBeforeFailure = 2; } - (void)writeStreamDidOpen { - self.writeStreamOpenTime = [NSDate date]; - [self.writeStream writeHandshake]; } diff --git a/Firestore/Source/Remote/FSTStream.mm b/Firestore/Source/Remote/FSTStream.mm index 6bec3ad..6735df1 100644 --- a/Firestore/Source/Remote/FSTStream.mm +++ b/Firestore/Source/Remote/FSTStream.mm @@ -620,7 +620,7 @@ static const NSTimeInterval kIdleTimeout = 60.0; serializer:(FSTSerializerBeta *)serializer { self = [super initWithDatabase:database workerDispatchQueue:workerDispatchQueue - connectionTimerID:FSTTimerIDListenStreamConnection + connectionTimerID:FSTTimerIDListenStreamConnectionBackoff idleTimerID:FSTTimerIDListenStreamIdle credentials:credentials responseMessageClass:[GCFSListenResponse class]]; @@ -705,7 +705,7 @@ static const NSTimeInterval kIdleTimeout = 60.0; serializer:(FSTSerializerBeta *)serializer { self = [super initWithDatabase:database workerDispatchQueue:workerDispatchQueue - connectionTimerID:FSTTimerIDWriteStreamConnection + connectionTimerID:FSTTimerIDWriteStreamConnectionBackoff idleTimerID:FSTTimerIDWriteStreamIdle credentials:credentials responseMessageClass:[GCFSWriteResponse class]]; diff --git a/Firestore/Source/Util/FSTDispatchQueue.h b/Firestore/Source/Util/FSTDispatchQueue.h index 9b28c9c..7922600 100644 --- a/Firestore/Source/Util/FSTDispatchQueue.h +++ b/Firestore/Source/Util/FSTDispatchQueue.h @@ -23,11 +23,24 @@ NS_ASSUME_NONNULL_BEGIN * can then be used from tests to check for the presence of callbacks or to run them early. */ typedef NS_ENUM(NSInteger, FSTTimerID) { - FSTTimerIDAll, // Sentinel value to be used with runDelayedCallbacksUntil: to run all blocks. + /** All can be used with runDelayedCallbacksUntil: to run all timers. */ + FSTTimerIDAll, + + /** + * The following 4 timers are used in FSTStream for the listen and write streams. The "Idle" timer + * is used to close the stream due to inactivity. The "ConnectionBackoff" timer is used to + * restart a stream once the appropriate backoff delay has elapsed. + */ FSTTimerIDListenStreamIdle, - FSTTimerIDListenStreamConnection, + FSTTimerIDListenStreamConnectionBackoff, FSTTimerIDWriteStreamIdle, - FSTTimerIDWriteStreamConnection + FSTTimerIDWriteStreamConnectionBackoff, + + /** + * A timer used in FSTOnlineStateTracker to transition from FSTOnlineState Unknown to Offline + * after a set timeout, rather than waiting indefinitely for success or failure. + */ + FSTTimerIDOnlineStateTimeout }; /** @@ -80,6 +93,13 @@ typedef NS_ENUM(NSInteger, FSTTimerID) { */ - (void)dispatchAsyncAllowingSameQueue:(void (^)(void))block; +/** + * Wrapper for dispatch_sync(). Mostly meant for use in tests. + * + * @param block The block to run. + */ +- (void)dispatchSync:(void (^)(void))block; + /** * Schedules a callback after the specified delay. * diff --git a/Firestore/Source/Util/FSTDispatchQueue.mm b/Firestore/Source/Util/FSTDispatchQueue.mm index 5bd7f27..3184d29 100644 --- a/Firestore/Source/Util/FSTDispatchQueue.mm +++ b/Firestore/Source/Util/FSTDispatchQueue.mm @@ -187,6 +187,10 @@ NS_ASSUME_NONNULL_BEGIN dispatch_async(self.queue, block); } +- (void)dispatchSync:(void (^)(void))block { + dispatch_sync(self.queue, block); +} + - (FSTDelayedCallback *)dispatchAfterDelay:(NSTimeInterval)delay timerID:(FSTTimerID)timerID block:(void (^)(void))block { -- cgit v1.2.3