Make disconnect callback nullable.

GoogleSignIn/Sources/GIDSignIn.m

@@ -209,7 +209,7 @@ static const NSTimeInterval kMinimumRestoredAccessTokenTimeToExpire = 600.0;
   [self removeAllKeychainEntries];
 }
 
-- (void)disconnectWithCallback:(GIDDisconnectCallback)callback {
+- (void)disconnectWithCallback:(nullable GIDDisconnectCallback)callback {
   GIDGoogleUser *user = _currentUser;
   OIDAuthState *authState = user.authentication.authState;
   if (!authState) {
@@ -226,7 +226,9 @@ static const NSTimeInterval kMinimumRestoredAccessTokenTimeToExpire = 600.0;
   if (!token) {
     [self signOut];
     // Nothing to do here, consider the operation successful.
-    callback(nil);
+    if (callback) {
+      callback(nil);
+    }
     return;
   }
   NSString *revokeURLString = [NSString stringWithFormat:kRevokeTokenURLTemplate,
@@ -241,13 +243,12 @@ static const NSTimeInterval kMinimumRestoredAccessTokenTimeToExpire = 600.0;
               fromAuthState:authState
                 withComment:@"GIDSignIn: revoke tokens"
       withCompletionHandler:^(NSData *data, NSError *error) {
-    // Revoking an already revoked token seems always successful, which saves the trouble here for
-    // us.
-    if (error) {
-      callback(error);
-    } else {
+    // Revoking an already revoked token seems always successful, which helps us here.
+    if (!error) {
       [self signOut];
-      callback(nil);
+    }
+    if (callback) {
+      callback(error);
     }
   }];
 }

GoogleSignIn/Sources/Public/GoogleSignIn/GIDSignIn.h

@@ -171,8 +171,8 @@ typedef void (^GIDDisconnectCallback)(NSError *_Nullable error);
 /// Disconnects the current user from the app and revokes previous authentication. If the operation
 /// succeeds, the OAuth 2.0 token is also removed from keychain.
 ///
-/// @param callback The `GIDSignInCallback` block that is called on completion.
-- (void)disconnectWithCallback:(GIDDisconnectCallback)callback;
+/// @param callback The optional `GIDSignInCallback` block that is called on completion.
+- (void)disconnectWithCallback:(nullable GIDDisconnectCallback)callback;
 
 @end
 

GoogleSignIn/Tests/Unit/GIDSignInTest.m

@@ -605,7 +605,7 @@ static void *kTestObserverContext = &kTestObserverContext;
 
 #pragma mark - Tests - disconnectWithCallback:
 
-// Verifies disconnect calls delegate disconnect method with no errors if access token is present.
+// Verifies disconnect calls callback with no errors if access token is present.
 - (void)testDisconnect_accessToken {
   [[[_authorization expect] andReturn:_authState] authState];
   [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse];
@@ -618,13 +618,26 @@ static void *kTestObserverContext = &kTestObserverContext;
       [expectation fulfill];
     }
   }];
-  [self verifyAndRevokeToken:kAccessToken];
+  [self verifyAndRevokeToken:kAccessToken hasCallback:YES];
   [_authorization verify];
   [_authState verify];
   [_tokenResponse verify];
 }
 
-// Verifies disconnect calls delegate disconnect method with no errors if refresh token is present.
+// Verifies disconnect if access token is present.
+- (void)testDisconnectNoCallback_accessToken {
+  [[[_authorization expect] andReturn:_authState] authState];
+  [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse];
+  [[[_tokenResponse expect] andReturn:kAccessToken] accessToken];
+  [[[_authorization expect] andReturn:_fetcherService] fetcherService];
+  [_signIn disconnectWithCallback:nil];
+  [self verifyAndRevokeToken:kAccessToken hasCallback:NO];
+  [_authorization verify];
+  [_authState verify];
+  [_tokenResponse verify];
+}
+
+// Verifies disconnect calls callback with no errors if refresh token is present.
 - (void)testDisconnect_refreshToken {
   [[[_authorization expect] andReturn:_authState] authState];
   [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse];
@@ -639,13 +652,13 @@ static void *kTestObserverContext = &kTestObserverContext;
       [expectation fulfill];
     }
   }];
-  [self verifyAndRevokeToken:kRefreshToken];
+  [self verifyAndRevokeToken:kRefreshToken hasCallback:YES];
   [_authorization verify];
   [_authState verify];
   [_tokenResponse verify];
 }
 
-// Verifies disconnect errors are passed along to the delegate.
+// Verifies disconnect errors are passed along to the callback.
 - (void)testDisconnect_errors {
   [[[_authorization expect] andReturn:_authState] authState];
   [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse];
@@ -668,8 +681,24 @@ static void *kTestObserverContext = &kTestObserverContext;
   [_tokenResponse verify];
 }
 
-// Verifies disconnect calls delegate disconnect method and clear keychain with no errors if no
-// tokens are present.
+// Verifies disconnect with errors
+- (void)testDisconnectNoCallback_errors {
+  [[[_authorization expect] andReturn:_authState] authState];
+  [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse];
+  [[[_tokenResponse expect] andReturn:kAccessToken] accessToken];
+  [[[_authorization expect] andReturn:_fetcherService] fetcherService];
+  [_signIn disconnectWithCallback:nil];
+  XCTAssertTrue([self isFetcherStarted], @"should start fetching");
+  // Emulate result back from server.
+  NSError *error = [self error];
+  [self didFetch:nil error:error];
+  [_authorization verify];
+  [_authState verify];
+  [_tokenResponse verify];
+}
+
+
+// Verifies disconnect calls callback with no errors and clears keychain if no tokens are present.
 - (void)testDisconnect_noTokens {
   [[[_authorization expect] andReturn:_authState] authState];
   [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse];
@@ -691,6 +720,21 @@ static void *kTestObserverContext = &kTestObserverContext;
   [_tokenResponse verify];
 }
 
+// Verifies disconnect clears keychain if no tokens are present.
+- (void)testDisconnectNoCallback_noTokens {
+  [[[_authorization expect] andReturn:_authState] authState];
+  [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse];
+  [[[_tokenResponse expect] andReturn:nil] accessToken];
+  [[[_authState expect] andReturn:_tokenResponse] lastTokenResponse];
+  [[[_tokenResponse expect] andReturn:nil] refreshToken];
+  [_signIn disconnectWithCallback:nil];
+  XCTAssertFalse([self isFetcherStarted], @"should not fetch");
+  XCTAssertTrue(_keychainRemoved, @"keychain should be removed");
+  [_authorization verify];
+  [_authState verify];
+  [_tokenResponse verify];
+}
+
 - (void)testPresentingViewControllerException {
   _signIn.presentingViewController = nil;
   XCTAssertThrows([_signIn signIn]);
@@ -927,7 +971,7 @@ static void *kTestObserverContext = &kTestObserverContext;
 }
 
 // Verifies a fetcher has started for revoking token and emulates a server response.
-- (void)verifyAndRevokeToken:(NSString *)token {
+- (void)verifyAndRevokeToken:(NSString *)token hasCallback:(BOOL)hasCallback {
   XCTAssertTrue([self isFetcherStarted], @"should start fetching");
   NSURL *url = [self fetchedURL];
   XCTAssertEqualObjects([url scheme], @"https", @"scheme must match");
@@ -939,7 +983,9 @@ static void *kTestObserverContext = &kTestObserverContext;
                         @"token parameter should match");
   // Emulate result back from server.
   [self didFetch:nil error:nil];
-  [self waitForExpectationsWithTimeout:1 handler:nil];
+  if (hasCallback) {
+    [self waitForExpectationsWithTimeout:1 handler:nil];
+  }
   XCTAssertTrue(_keychainRemoved, @"should clear saved keychain name");
 }