diff --git a/Sources/Core/GTMSessionFetcher.m b/Sources/Core/GTMSessionFetcher.m index f3636ec..636ede0 100644 --- a/Sources/Core/GTMSessionFetcher.m +++ b/Sources/Core/GTMSessionFetcher.m @@ -1788,8 +1788,27 @@ - (void)endBackgroundTask { - (void)authorizeRequest { GTMSessionCheckNotSynchronized(self); + BOOL stopped = NO; @synchronized(self) { - _delayState = kDelayStateAuthorizing; + if (_userStoppedFetching) { + stopped = YES; + } else { + // Go into the delayed state for getting the authorization. + _delayState = kDelayStateAuthorizing; + } + } + + if (stopped) { + // We end up here if someone calls `stopFetching` from another thread/queue while + // the fetch was being started up, so while `stopFetching` did the needed shutdown + // we have to ensure the requested callback was triggered. + if (self.stopFetchingTriggersCompletionHandler) { + NSError *error = [NSError errorWithDomain:kGTMSessionFetcherErrorDomain + code:GTMSessionFetcherErrorUserCancelled + userInfo:nil]; + [self finishWithError:error shouldRetry:NO]; + } + return; } id authorizer = self.authorizer; @@ -1827,13 +1846,26 @@ - (void)authorizer:(nullable id __unused)auth finishedWithError:(nullable NSError *)error { GTMSessionCheckNotSynchronized(self); + @synchronized(self) { + // If `stopFetching` was called, do nothing, since the fetch was in a delay state + // any needed callback already happened. + if (_userStoppedFetching) { + return; + } + if (error == nil) { + _request = authorizedRequest; + + // If `stopFetching` wasn't called, clear the `_delayState`, so a call after this + // point will trigger a callback as needed. This also ensure if this is going to + // error below a cancel callback couldn't also trigger. + _delayState = kDelayStateNotDelayed; + } + } + if (error != nil) { // We can't fetch without authorization [self failToBeginFetchWithError:(NSError *_Nonnull)error]; } else { - @synchronized(self) { - _request = authorizedRequest; - } [self beginFetchMayDelay:NO mayAuthorize:NO mayDecorate:YES]; } } @@ -2080,7 +2112,12 @@ - (void)stopFetching { // `stopFetchReleasingCallbacks:` will dequeue it if there is a sevice throttled // delay, so the canceled callback needs to be directly triggered since the serivce // won't attempt to restart it. - triggerCallback = _delayState == kDelayStateServiceDelayed && self.stopFetchingTriggersCompletionHandler; + // + // And the authorization delay assumes all stop notifications needed will be done + // from here. + triggerCallback = + (_delayState == kDelayStateServiceDelayed || _delayState == kDelayStateAuthorizing) && + self.stopFetchingTriggersCompletionHandler; } // @synchronized(self) if (triggerCallback) { diff --git a/UnitTests/GTMSessionFetcherFetchingTest.m b/UnitTests/GTMSessionFetcherFetchingTest.m index 489c254..cf1b284 100644 --- a/UnitTests/GTMSessionFetcherFetchingTest.m +++ b/UnitTests/GTMSessionFetcherFetchingTest.m @@ -1585,57 +1585,47 @@ - (void)testDelayedSyncAuthCancelFetchWithCallback_WithoutFetcherService { } - (void)testImmediateSyncAuthCancelFetchWithCallback { - XCTSkip(@"Has failed on CI, but not locally, needs investigation."); [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer syncAuthorizer]]; } - (void)testImmediateSyncAuthCancelFetchWithCallback_WithoutFetcherService { _fetcherService = nil; - XCTSkip(@"Has failed on CI, but not locally, needs investigation."); [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer syncAuthorizer]]; } - (void)testDelayedAsyncAuthCancelFetchWithCallback { - XCTSkip(@"Currently fails, needs fixing."); [self internalCancelFetchWithCallback:1 authorizer:[TestAuthorizer asyncAuthorizer]]; } - (void)testDelayedAsyncAuthCancelFetchWithCallback_WithoutFetcherService { _fetcherService = nil; - XCTSkip(@"Currently fails, needs fixing."); [self internalCancelFetchWithCallback:1 authorizer:[TestAuthorizer asyncAuthorizer]]; } - (void)testImmediateAsyncAuthCancelFetchWithCallback { - XCTSkip(@"Currently fails, needs fixing."); [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer asyncAuthorizer]]; } - (void)testImmediateAsyncAuthCancelFetchWithCallback_WithoutFetcherService { _fetcherService = nil; - XCTSkip(@"Currently fails, needs fixing."); [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer asyncAuthorizer]]; } - (void)testDelayedAsyncDelayedAuthCancelFetchWithCallback { - XCTSkip(@"Currently fails, needs fixing."); [self internalCancelFetchWithCallback:1 authorizer:[TestAuthorizer asyncAuthorizerDelayed:2]]; } - (void)testDelayedAsyncDelayedAuthCancelFetchWithCallback_WithoutFetcherService { _fetcherService = nil; - XCTSkip(@"Currently fails, needs fixing."); [self internalCancelFetchWithCallback:1 authorizer:[TestAuthorizer asyncAuthorizerDelayed:2]]; } - (void)testImmediateAsyncDelayedAuthCancelFetchWithCallback { - XCTSkip(@"Currently fails, needs fixing."); [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer asyncAuthorizerDelayed:1]]; } - (void)testImmediateAsyncDelayedAuthCancelFetchWithCallback_WithoutFetcherService { _fetcherService = nil; - XCTSkip(@"Currently fails, needs fixing."); [self internalCancelFetchWithCallback:0 authorizer:[TestAuthorizer asyncAuthorizerDelayed:1]]; } @@ -1649,7 +1639,17 @@ - (void)internalCancelFetchWithCallback:(unsigned int)sleepTime #pragma clang diagnostic pop if (!_isServerRunning) return; - CREATE_START_STOP_NOTIFICATION_EXPECTATIONS(1, 1); + // If the authorizer is async, then the fetch won't fully begin, and there won't ever be + // a start notification (and thus stop notification). + int expectedNotificationCount = ((TestAuthorizer*)authorizer).isAsync ? 0 : 1; + XCTestExpectation *fetcherStartedExpectation = nil; + XCTestExpectation *fetcherStoppedExpectation = nil; + if (expectedNotificationCount) { + fetcherStartedExpectation = + [[XCTNSNotificationExpectation alloc] initWithName:kGTMSessionFetcherStartedNotification]; + fetcherStoppedExpectation = + [[XCTNSNotificationExpectation alloc] initWithName:kGTMSessionFetcherStoppedNotification]; + } FetcherNotificationsCounter *fnctr = [[FetcherNotificationsCounter alloc] init]; @@ -1674,19 +1674,17 @@ - (void)internalCancelFetchWithCallback:(unsigned int)sleepTime } [fetcher stopFetching]; - WAIT_FOR_START_STOP_NOTIFICATION_EXPECTATIONS(); - [self waitForExpectationsWithTimeout:_timeoutInterval handler:nil]; [self assertCallbacksReleasedForFetcher:fetcher]; // Check the notifications. - XCTAssertEqual(fnctr.fetchStarted, 1, @"%@", fnctr.fetchersStartedDescriptions); - XCTAssertEqual(fnctr.fetchStopped, 1, @"%@", fnctr.fetchersStoppedDescriptions); + XCTAssertEqual(fnctr.fetchStarted, expectedNotificationCount, @"%@", fnctr.fetchersStartedDescriptions); + XCTAssertEqual(fnctr.fetchStopped, fnctr.fetchStarted, @"%@", fnctr.fetchersStoppedDescriptions); XCTAssertEqual(fnctr.fetchCompletionInvoked, 1); #if GTM_BACKGROUND_TASK_FETCHING [self waitForBackgroundTaskEndedNotifications:fnctr]; - XCTAssertEqual(fnctr.backgroundTasksStarted.count, (NSUInteger)1); + XCTAssertEqual(fnctr.backgroundTasksStarted.count, (NSUInteger)expectedNotificationCount); XCTAssertEqualObjects(fnctr.backgroundTasksStarted, fnctr.backgroundTasksEnded); #endif }