Add a method taking a completion argument for asynchronous error updates (google#229)

Author: mdmathias

GTMAppAuth/Sources/AuthSession.swift

@@ -285,26 +285,30 @@ public final class AuthSession: NSObject, GTMSessionFetcherAuthorizer, NSSecureC
     }
     let callbackQueue = fetcherService?.callbackQueue ?? DispatchQueue.main
 
-    if let error = args.error, let delegate = self.delegate {
-      // If there is an updated error, use that; otherwise, use whatever is already in `args.error`
-      let newError = delegate.updatedError?(forAuthSession: self, originalError: error)
-      args.error = newError ?? error
+    let callback: () -> Void = {
+      callbackQueue.async {
+        switch args.callbackStyle {
+        case .completion(let callback):
+          self.invokeCompletionCallback(with: callback, error: args.error)
+        case .delegate(let delegate, let selector):
+          self.invokeCallback(
+            withDelegate: delegate,
+            selector: selector,
+            request: request,
+            error: args.error
+          )
+        }
+      }
     }
 
-    callbackQueue.async { [weak self] in
-      guard let self = self else { return }
-
-      switch args.callbackStyle {
-      case .completion(let callback):
-        self.invokeCompletionCallback(with: callback, error: args.error)
-      case .delegate(let delegate, let selector):
-        self.invokeCallback(
-          withDelegate: delegate,
-          selector: selector,
-          request: request,
-          error: args.error
-        )
+    if let error = args.error, let delegate = self.delegate, delegate.updateError != nil {
+      // Use updated error if exists; otherwise, use whatever is already in `args.error`
+      delegate.updateError?(forAuthSession: self, originalError: error) { updatedError in
+        args.error = updatedError ?? error
+        callback()
       }
+    } else {
+      callback()
     }
   }
 

GTMAppAuth/Sources/AuthSessionDelegate.swift

@@ -30,15 +30,16 @@ public protocol AuthSessionDelegate {
 
   /// A method notifying the delegate that the authorization request failed.
   ///
-  /// Use this method to examine the error behind the failed authorization request and supply a more
-  /// custom error specifying whatever context is needed.
+  /// Use this method to examine the error behind the failed authorization request and supply a
+  /// customized error created asynchronously that specifies whatever context is needed.
   ///
   /// - Parameters:
   ///   - authSession: The `AuthSession` whose authorization request failed.
   ///   - originalError: The original `Error` associated with the failure.
-  /// - Returns: The new error for `AuthSession` to send back or nil if no error should be sent.
-  @objc optional func updatedError(
+  ///   - completion: An escaping closure to pass back the updated error.
+  @objc optional func updateError(
     forAuthSession authSession: AuthSession,
-    originalError: Error
-  ) -> Error?
+    originalError: Error,
+    completion: @escaping (Error?) -> Void
+  )
 }

GTMAppAuth/Tests/Helpers/AuthorizationTestingHelp.swift

@@ -63,10 +63,42 @@ public class AuthorizationTestDelegate: NSObject {
   }
 }
 
+/// A testing helper that does not implement the update error method in `AuthSessionDelegate`.
+@objc(GTMAuthSessionDelegateProvideMissingEMMErrorHandling)
+public class AuthSessionDelegateProvideMissingEMMErrorHandling: NSObject, AuthSessionDelegate {
+  /// The `AuthSession` to which this delegate was given.
+  @objc public var originalAuthSession: AuthSession?
+
+  /// Whether or not the delegate callback for additional refresh parameters was called.
+  ///
+  /// - Note: Defaults to `false`.
+  @objc public var additionalRefreshParametersCalled = false
+
+  /// Whether or not the delegate callback for authorization request failure was called.
+  ///
+  /// - Note: Defaults to `false`.
+  @objc public var updatedErrorCalled = false
+
+  /// The expected error from the delegate callback.
+  @objc public let expectedError: NSError?
+
+  @objc public init(originalAuthSession: AuthSession, expectedError: NSError? = nil) {
+    self.originalAuthSession = originalAuthSession
+    self.expectedError = expectedError
+  }
+
+  public func additionalTokenRefreshParameters(
+    forAuthSession authSession: AuthSession
+  ) -> [String : String]? {
+    XCTAssertEqual(authSession, originalAuthSession)
+    additionalRefreshParametersCalled = true
+    return [:]
+  }
+}
+
 /// A testing helper given to `AuthSession`'s `delegate` to verify delegate callbacks.
 @objc(GTMAuthSessionDelegateProvider)
 public class AuthSessionDelegateProvider: NSObject, AuthSessionDelegate {
-
   /// The `AuthSession` to which this delegate was given.
   @objc public var originalAuthSession: AuthSession?
 
@@ -96,15 +128,13 @@ public class AuthSessionDelegateProvider: NSObject, AuthSessionDelegate {
     return [:]
   }
 
-  public func updatedError(
+  public func updateError(
     forAuthSession authSession: AuthSession,
-    originalError: Error
-  ) -> Error? {
+    originalError: Error,
+    completion: @escaping (Error?) -> Void
+  ) {
     XCTAssertEqual(authSession, originalAuthSession)
     updatedErrorCalled = true
-    guard let expectedError = expectedError else {
-      return nil
-    }
-    return expectedError
+    completion(expectedError)
   }
 }

GTMAppAuth/Tests/Unit/AuthSessionTests.swift

@@ -268,6 +268,35 @@ class AuthSessionTests: XCTestCase {
     XCTAssertTrue(authSessionDelegate.updatedErrorCalled)
   }
 
+  func testAuthSessionAuthorizeRequestWithCompletionFailDelegateNoImplementUpdateErrorCallback() {
+    let authorizeSecureRequestExpectation = expectation(
+      description: "Authorize with completion expectation"
+    )
+    let authSession = AuthSession(
+      authState: OIDAuthState.testInstance()
+    )
+
+    // There must be an error here for the delegate to get the `updatedError` callback
+    let insecureRequest = NSMutableURLRequest(url: insecureFakeURL)
+    let expectedError = AuthSession.Error.cannotAuthorizeRequest(insecureRequest as URLRequest)
+    let authSessionDelegate = AuthSessionDelegateProvideMissingEMMErrorHandling(
+      originalAuthSession: authSession
+    )
+    authSession.delegate = authSessionDelegate
+
+    authSession.authorizeRequest(insecureRequest) { error in
+      guard let error = error else {
+        return XCTFail("There should be an `NSError` authorizing \(insecureRequest)")
+      }
+      XCTAssertEqual(error as? AuthSession.Error, expectedError)
+      authorizeSecureRequestExpectation.fulfill()
+    }
+    XCTAssertTrue(authSession.isAuthorizingRequest(insecureRequest as URLRequest))
+    waitForExpectations(timeout: expectationTimeout)
+    XCTAssertFalse(authSession.isAuthorizedRequest(insecureRequest as URLRequest))
+    XCTAssertFalse(authSessionDelegate.updatedErrorCalled)
+  }
+
   func testAuthSessionAuthorizeRequestWithCompletionDidFailButDelegateDoesntUpdateError() {
     let authorizeSecureRequestExpectation = expectation(
       description: "Authorize with completion expectation"