diff options
| -rw-r--r-- | Firestore/CHANGELOG.md | 3 | ||||
| -rw-r--r-- | Firestore/Example/Tests/SpecTests/json/offline_spec_test.json | 193 | ||||
| -rw-r--r-- | Firestore/Source/Remote/FSTOnlineStateTracker.h | 5 | ||||
| -rw-r--r-- | Firestore/Source/Remote/FSTOnlineStateTracker.mm | 22 |
4 files changed, 213 insertions, 10 deletions
diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index 50958db..bcd92d0 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,4 +1,7 @@ # Unreleased +- [fixed] Fixed a regression in the Firebase iOS SDK release 4.11.0 that could + cause getDocument() requests made while offline to be delayed by up to 10 + seconds (rather than returning from cache immediately). # v0.10.4 - [changed] If the SDK's attempt to connect to the Cloud Firestore backend diff --git a/Firestore/Example/Tests/SpecTests/json/offline_spec_test.json b/Firestore/Example/Tests/SpecTests/json/offline_spec_test.json index db8324b..1af4c16 100644 --- a/Firestore/Example/Tests/SpecTests/json/offline_spec_test.json +++ b/Firestore/Example/Tests/SpecTests/json/offline_spec_test.json @@ -888,5 +888,198 @@ ] } ] + }, + "New queries return immediately with fromCache=true when offline due to stream failures.": { + "describeName": "Offline:", + "itName": "New queries return immediately with fromCache=true when offline due to stream failures.", + "tags": [], + "config": { + "useGarbageCollection": true + }, + "steps": [ + { + "userListen": [ + 2, + { + "path": "collection", + "filters": [], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "2": { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "resumeToken": "" + } + } + } + }, + { + "watchStreamClose": { + "error": { + "code": 14, + "message": "Simulated Backend Error" + }, + "runBackoffTimer": true + } + }, + { + "watchStreamClose": { + "error": { + "code": 14, + "message": "Simulated Backend Error" + }, + "runBackoffTimer": true + }, + "expect": [ + { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "errorCode": 0, + "fromCache": true, + "hasPendingWrites": false + } + ] + }, + { + "userListen": [ + 4, + { + "path": "collection2", + "filters": [], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "2": { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "resumeToken": "" + }, + "4": { + "query": { + "path": "collection2", + "filters": [], + "orderBys": [] + }, + "resumeToken": "" + } + } + }, + "expect": [ + { + "query": { + "path": "collection2", + "filters": [], + "orderBys": [] + }, + "errorCode": 0, + "fromCache": true, + "hasPendingWrites": false + } + ] + } + ] + }, + "New queries return immediately with fromCache=true when offline due to OnlineState timeout.": { + "describeName": "Offline:", + "itName": "New queries return immediately with fromCache=true when offline due to OnlineState timeout.", + "tags": [], + "config": { + "useGarbageCollection": true + }, + "steps": [ + { + "userListen": [ + 2, + { + "path": "collection", + "filters": [], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "2": { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "resumeToken": "" + } + } + } + }, + { + "runTimer": "online_state_timeout", + "expect": [ + { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "errorCode": 0, + "fromCache": true, + "hasPendingWrites": false + } + ] + }, + { + "userListen": [ + 4, + { + "path": "collection2", + "filters": [], + "orderBys": [] + } + ], + "stateExpect": { + "activeTargets": { + "2": { + "query": { + "path": "collection", + "filters": [], + "orderBys": [] + }, + "resumeToken": "" + }, + "4": { + "query": { + "path": "collection2", + "filters": [], + "orderBys": [] + }, + "resumeToken": "" + } + } + }, + "expect": [ + { + "query": { + "path": "collection2", + "filters": [], + "orderBys": [] + }, + "errorCode": 0, + "fromCache": true, + "hasPendingWrites": false + } + ] + } + ] } } diff --git a/Firestore/Source/Remote/FSTOnlineStateTracker.h b/Firestore/Source/Remote/FSTOnlineStateTracker.h index a414a18..2521c84 100644 --- a/Firestore/Source/Remote/FSTOnlineStateTracker.h +++ b/Firestore/Source/Remote/FSTOnlineStateTracker.h @@ -43,9 +43,10 @@ NS_ASSUME_NONNULL_BEGIN @property(nonatomic, weak) id<FSTOnlineStateDelegate> onlineStateDelegate; /** - * Called by FSTRemoteStore when a watch stream is started. + * Called by FSTRemoteStore when a watch stream is started (including on each backoff attempt). * - * It sets the FSTOnlineState to Unknown and starts the onlineStateTimer if necessary. + * If this is the first attempt, it sets the FSTOnlineState to Unknown and starts the + * onlineStateTimer. */ - (void)handleWatchStreamStart; diff --git a/Firestore/Source/Remote/FSTOnlineStateTracker.mm b/Firestore/Source/Remote/FSTOnlineStateTracker.mm index 41650cd..e782397 100644 --- a/Firestore/Source/Remote/FSTOnlineStateTracker.mm +++ b/Firestore/Source/Remote/FSTOnlineStateTracker.mm @@ -47,7 +47,7 @@ static const NSTimeInterval kOnlineStateTimeout = 10; * Unknown to Offline without waiting for the stream to actually fail (kMaxWatchStreamFailures * times). */ -@property(nonatomic, strong, nullable) FSTDelayedCallback *watchStreamTimer; +@property(nonatomic, strong, nullable) FSTDelayedCallback *onlineStateTimer; /** * Whether the client should log a warning message if it fails to connect to the backend @@ -71,14 +71,15 @@ static const NSTimeInterval kOnlineStateTimeout = 10; } - (void)handleWatchStreamStart { - [self setAndBroadcastState:FSTOnlineStateUnknown]; + if (self.watchStreamFailures == 0) { + [self setAndBroadcastState:FSTOnlineStateUnknown]; - if (self.watchStreamTimer == nil) { - self.watchStreamTimer = [self.queue + FSTAssert(!self.onlineStateTimer, @"onlineStateTimer shouldn't be started yet"); + self.onlineStateTimer = [self.queue dispatchAfterDelay:kOnlineStateTimeout timerID:FSTTimerIDOnlineStateTimeout block:^{ - self.watchStreamTimer = nil; + self.onlineStateTimer = nil; FSTAssert( self.state == FSTOnlineStateUnknown, @"Timer should be canceled if we transitioned to a different state."); @@ -100,6 +101,11 @@ static const NSTimeInterval kOnlineStateTimeout = 10; - (void)handleWatchStreamFailure { if (self.state == FSTOnlineStateOnline) { [self setAndBroadcastState:FSTOnlineStateUnknown]; + + // To get to FSTOnlineStateOnline, updateState: must have been called which would have reset + // our heuristics. + FSTAssert(self.watchStreamFailures == 0, @"watchStreamFailures must be 0"); + FSTAssert(!self.onlineStateTimer, @"onlineStateTimer must be nil"); } else { self.watchStreamFailures++; if (self.watchStreamFailures >= kMaxWatchStreamFailures) { @@ -138,9 +144,9 @@ static const NSTimeInterval kOnlineStateTimeout = 10; } - (void)clearOnlineStateTimer { - if (self.watchStreamTimer) { - [self.watchStreamTimer cancel]; - self.watchStreamTimer = nil; + if (self.onlineStateTimer) { + [self.onlineStateTimer cancel]; + self.onlineStateTimer = nil; } } |