Allow user to specify dispatch queue for the `performActionWithFreshT…

…okens` method To date, all AppAuth blocks execute on the main queue, including the user's own actions. This change allows the user to specify the queue that the action will be scheduled on in `performActionWithFreshTokens`. Unlike the rest of AppAuth's callbacks, this one is called frequently – for every API call – thus it is important for the user to determine on which dispatch queue it executes. OIDAuthState itself is not yet thread-safe, so all calls, including `performActionWithFreshTokens` should be performed on the main thread. This change also removes an unneeded duplicate dispatch on the main thread after token refresh (the callback is already dispatched on the main thread).
manotnemanja · Jul 19, 2018 · b4ca39a · b4ca39a
1 parent 65ef95c
commit b4ca39a
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 26 deletions.
diff --git a/Source/OIDAuthState.h b/Source/OIDAuthState.h
@@ -225,6 +225,19 @@ typedef void (^OIDAuthStateAuthorizationCallback)(OIDAuthState *_Nullable authSt
          additionalRefreshParameters:
     (nullable NSDictionary<NSString *, NSString *> *)additionalParameters;
 
+/*! @brief Calls the block with a valid access token (refreshing it first, if needed), or if a
+        refresh was needed and failed, with the error that caused it to fail.
+    @param action The block to execute with a fresh token. This block will be executed on the main
+        thread.
+    @param additionalParameters Additional parameters for the token request if token is
+        refreshed.
+    @param dispatchQueue The dispatchQueue on which to dispatch the action block.
+ */
+- (void)performActionWithFreshTokens:(OIDAuthStateAction)action
+         additionalRefreshParameters:
+    (nullable NSDictionary<NSString *, NSString *> *)additionalParameters
+                       dispatchQueue:(dispatch_queue_t)dispatchQueue;
+
 /*! @brief Forces a token refresh the next time @c OIDAuthState.performActionWithFreshTokens: is
         called, even if the current tokens are considered valid.
  */

diff --git a/Source/OIDAuthState.m b/Source/OIDAuthState.m
@@ -65,6 +65,23 @@
  */
 static const NSUInteger kExpiryTimeTolerance = 60;
 
+/*! @brief Object to hold OIDAuthState pending actions.
+ */
+@interface OIDAuthStatePendingAction : NSObject
+@property(nonatomic, readonly, nullable) OIDAuthStateAction action;
+@property(nonatomic, readonly, nullable) dispatch_queue_t dispatchQueue;
+@end
+@implementation OIDAuthStatePendingAction
+- (id)initWithAction:(OIDAuthStateAction)action andDispatchQueue:(dispatch_queue_t)dispatchQueue {
+  self = [super init];
+  if (self) {
+    _action = action;
+    _dispatchQueue = dispatchQueue;
+  }
+  return self;
+}
+@end
+
 @interface OIDAuthState ()
 
 /*! @brief The access token generated by the authorization server.
@@ -440,9 +457,19 @@ - (void)performActionWithFreshTokens:(OIDAuthStateAction)action {
 - (void)performActionWithFreshTokens:(OIDAuthStateAction)action
          additionalRefreshParameters:
     (nullable NSDictionary<NSString *, NSString *> *)additionalParameters {
+  [self performActionWithFreshTokens:action
+         additionalRefreshParameters:additionalParameters
+                       dispatchQueue:dispatch_get_main_queue()];
+}
+
+- (void)performActionWithFreshTokens:(OIDAuthStateAction)action
+         additionalRefreshParameters:
+    (nullable NSDictionary<NSString *, NSString *> *)additionalParameters
+                       dispatchQueue:(dispatch_queue_t)dispatchQueue {
+
   if ([self isTokenFresh]) {
     // access token is valid within tolerance levels, perform action
-    dispatch_async(dispatch_get_main_queue(), ^() {
+    dispatch_async(dispatchQueue, ^{
       action(self.accessToken, self.idToken, nil);
     });
     return;
@@ -454,23 +481,25 @@ - (void)performActionWithFreshTokens:(OIDAuthStateAction)action
       OIDErrorUtilities errorWithCode:OIDErrorCodeTokenRefreshError
                       underlyingError:nil
                           description:@"Unable to refresh expired token without a refresh token."];
-    dispatch_async(dispatch_get_main_queue(), ^() {
+    dispatch_async(dispatchQueue, ^{
         action(nil, nil, tokenRefreshError);
     });
     return;
   }
 
   // access token is expired, first refresh the token, then perform action
   NSAssert(_pendingActionsSyncObject, @"_pendingActionsSyncObject cannot be nil", @"");
+  OIDAuthStatePendingAction* pendingAction =
+      [[OIDAuthStatePendingAction alloc] initWithAction:action andDispatchQueue:dispatchQueue];
   @synchronized(_pendingActionsSyncObject) {
     // if a token is already in the process of being refreshed, adds to pending actions
     if (_pendingActions) {
-      [_pendingActions addObject:action];
+      [_pendingActions addObject:pendingAction];
       return;
     }
 
     // creates a list of pending actions, starting with this one
-    _pendingActions = [NSMutableArray arrayWithObject:action];
+    _pendingActions = [NSMutableArray arrayWithObject:pendingAction];
   }
 
   // refresh the tokens
@@ -480,33 +509,33 @@ - (void)performActionWithFreshTokens:(OIDAuthStateAction)action
                  originalAuthorizationResponse:_lastAuthorizationResponse
                                       callback:^(OIDTokenResponse *_Nullable response,
                                                  NSError *_Nullable error) {
-    dispatch_async(dispatch_get_main_queue(), ^() {
-      // update OIDAuthState based on response
-      if (response) {
+    // update OIDAuthState based on response
+    if (response) {
+      self->_needsTokenRefresh = NO;
+      [self updateWithTokenResponse:response error:nil];
+    } else {
+      if (error.domain == OIDOAuthTokenErrorDomain) {
         self->_needsTokenRefresh = NO;
-        [self updateWithTokenResponse:response error:nil];
+        [self updateWithAuthorizationError:error];
       } else {
-        if (error.domain == OIDOAuthTokenErrorDomain) {
-          self->_needsTokenRefresh = NO;
-          [self updateWithAuthorizationError:error];
-        } else {
-          if ([self->_errorDelegate respondsToSelector:
-              @selector(authState:didEncounterTransientError:)]) {
-            [self->_errorDelegate authState:self didEncounterTransientError:error];
-          }
+        if ([self->_errorDelegate respondsToSelector:
+            @selector(authState:didEncounterTransientError:)]) {
+          [self->_errorDelegate authState:self didEncounterTransientError:error];
         }
       }
+    }
 
-      // nil the pending queue and process everything that was queued up
-      NSArray *actionsToProcess;
-      @synchronized(self->_pendingActionsSyncObject) {
-        actionsToProcess = self->_pendingActions;
-        self->_pendingActions = nil;
-      }
-      for (OIDAuthStateAction actionToProcess in actionsToProcess) {
-        actionToProcess(self.accessToken, self.idToken, error);
-      }
-    });
+    // nil the pending queue and process everything that was queued up
+    NSArray *actionsToProcess;
+    @synchronized(self->_pendingActionsSyncObject) {
+      actionsToProcess = self->_pendingActions;
+      self->_pendingActions = nil;
+    }
+    for (OIDAuthStatePendingAction* actionToProcess in actionsToProcess) {
+      dispatch_async(actionToProcess.dispatchQueue, ^{
+        actionToProcess.action(self.accessToken, self.idToken, error);
+      });
+    }
   }];
 }