Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#22] Implement retry queue and add retry limit #23

Merged
merged 4 commits into from
Sep 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Cartfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@

github "Alamofire/Alamofire"
github "Alamofire/Alamofire" "5.0.0-beta.7"
github "OAuthSwift/OAuthSwift"
2 changes: 1 addition & 1 deletion Cartfile.resolved
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
github "Alamofire/Alamofire" "5.0.0-beta.6"
github "Alamofire/Alamofire" "5.0.0-beta.7"
github "OAuthSwift/OAuthSwift" "2.0.0"
4 changes: 4 additions & 0 deletions OAuthSwift-Alamofire.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@
C4D7F93F1BB7585900A6D40E /* OAuthSwiftAlamofire.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = OAuthSwiftAlamofire.framework; sourceTree = BUILT_PRODUCTS_DIR; };
C4D7F9421BB7585900A6D40E /* OAuthSwift-Alamofire.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "OAuthSwift-Alamofire.h"; sourceTree = "<group>"; };
C4D7F9441BB7585900A6D40E /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
FF9B7D1023197A3D0073E77E /* Cartfile */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = Cartfile; sourceTree = "<group>"; };
FF9B7D1223197A4E0073E77E /* Cartfile.resolved */ = {isa = PBXFileReference; lastKnownFileType = text; path = Cartfile.resolved; sourceTree = "<group>"; };
/* End PBXFileReference section */

/* Begin PBXFrameworksBuildPhase section */
Expand Down Expand Up @@ -204,6 +206,8 @@
C4D7F9351BB7585900A6D40E = {
isa = PBXGroup;
children = (
FF9B7D1023197A3D0073E77E /* Cartfile */,
FF9B7D1223197A4E0073E77E /* Cartfile.resolved */,
C48204611DA532E300A69572 /* Sources */,
C40C96EB1DA6677400197628 /* Frameworks */,
C40C96DA1DA6495600197628 /* Xcode */,
Expand Down
25 changes: 25 additions & 0 deletions Package.resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"object": {
"pins": [
{
"package": "Alamofire",
"repositoryURL": "https://github.com/Alamofire/Alamofire.git",
"state": {
"branch": null,
"revision": "82cc60d703dbced153baa04e26c6296ba9690a2d",
"version": "5.0.0-rc.1"
}
},
{
"package": "OAuthSwift",
"repositoryURL": "https://github.com/OAuthSwift/OAuthSwift.git",
"state": {
"branch": null,
"revision": "868b06cafb474fa809ca7cf486ccb1a958c8d245",
"version": "1.4.1"
}
}
]
},
"version": 1
}
13 changes: 9 additions & 4 deletions Package.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version:4.0
// swift-tools-version:5.0
// Package.swift
/*
The MIT License (MIT)
Expand All @@ -24,15 +24,20 @@ import PackageDescription

let package = Package(
name: "OAuthSwiftAlamofire",
platforms: [.macOS(.v10_12),
.iOS(.v10),
.tvOS(.v10),
.watchOS(.v3)],
products: [
.library(name: "OAuthSwiftAlamofire", targets: ["OAuthSwiftAlamofire"]),
],
dependencies: [
.package(url: "https://github.com/OAuthSwift/OAuthSwift.git", from: "1.0.0"),
.package(url: "https://github.com/Alamofire/Alamofire.git", from: "4.0.0"),
.package(url: "https://github.com/OAuthSwift/OAuthSwift.git", .branch("2.1.0")),
.package(url: "https://github.com/Alamofire/Alamofire.git", .branch("5.0.0-rc.1")),
],
targets: [
.target(name: "OAuthSwiftAlamofire", dependencies: ["OAuthSwift", "Alamofire"], path: "Sources"),
.testTarget(name: "OAuthSwiftAlamofireTests", dependencies: ["OAuthSwiftAlamofire"], path: "Tests"),
]
],
swiftLanguageVersions: [.v5]
)
2 changes: 1 addition & 1 deletion Sources/HTTPMethod.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public extension Alamofire.HTTPMethod {
public extension OAuthSwiftHTTPRequest.Method {

var alamofire: Alamofire.HTTPMethod {
return Alamofire.HTTPMethod(rawValue: self.rawValue)!
return Alamofire.HTTPMethod(rawValue: self.rawValue)
}

}
40 changes: 24 additions & 16 deletions Sources/OAuthSwiftRequestInterceptor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@ open class OAuthSwiftRequestInterceptor: RequestInterceptor {
fileprivate let oauthSwift: OAuthSwift
public var paramsLocation: OAuthSwiftHTTPRequest.ParamsLocation = .authorizationHeader
public var dataEncoding: String.Encoding = .utf8
public var retryLimit = 1

fileprivate var requestsToRetry: [(RetryResult) -> Void] = []

public init(_ oauthSwift: OAuthSwift) {
self.oauthSwift = oauthSwift
}

open func adapt(_ urlRequest: URLRequest, for session: Session, completion: @escaping (AFResult<URLRequest>) -> Void) {
open func adapt(_ urlRequest: URLRequest, for session: Session, completion: @escaping (Result<URLRequest, Error>) -> Void) {
var config = OAuthSwiftHTTPRequest.Config(
urlRequest: urlRequest,
paramsLocation: paramsLocation,
Expand Down Expand Up @@ -49,25 +52,34 @@ open class OAuthSwift2RequestInterceptor: OAuthSwiftRequestInterceptor {

fileprivate var oauth2Swift: OAuth2Swift { return oauthSwift as! OAuth2Swift } // swiftlint:disable:this force_cast

private let lock = NSLock() // XXX remove the lock? (due to new implementation of alamofire)
private let lock = NSLock() // lock required to manage requestToRetry access
private var isRefreshing = false

open override func retry(_ request: Request, for session: Session, dueTo error: Error, completion: @escaping (RetryResult) -> Void) {
lock.lock() ; defer { lock.unlock() }

if mustRetry(request: request, dueTo: error) {
// queue requests so they can all be retried when token refresh is done
requestsToRetry.append(completion)

if !isRefreshing {
refreshTokens { [weak self] result in
guard let strongSelf = self else { return }

strongSelf.lock.lock() ; defer { strongSelf.lock.unlock() }

var shouldRetry: RetryResult

switch result {
case .success:
completion(.retry)
shouldRetry = .retry
case .failure(let error):
completion(.doNotRetryWithError(error))
shouldRetry = .doNotRetryWithError(error)
}

// process any previously queued requests
strongSelf.requestsToRetry.forEach { $0(shouldRetry) }
strongSelf.requestsToRetry.removeAll()
}
}
} else {
Expand All @@ -77,7 +89,7 @@ open class OAuthSwift2RequestInterceptor: OAuthSwiftRequestInterceptor {

/// Determine if must retry or not ie. refresh token
open func mustRetry(request: Request, dueTo error: Error) -> Bool {
if let response = request.task?.response as? HTTPURLResponse, response.statusCode == 401 {
if let response = request.task?.response as? HTTPURLResponse, response.statusCode == 401 && request.retryCount < retryLimit {
return true
}
return false
Expand All @@ -89,23 +101,19 @@ open class OAuthSwift2RequestInterceptor: OAuthSwiftRequestInterceptor {
isRefreshing = true

oauth2Swift.renewAccessToken(withRefreshToken: oauth2Swift.client.credential.oauthRefreshToken) { [weak self] result in
switch result {
case .success:
guard let strongSelf = self else { return }
completion(.success(()))
strongSelf.isRefreshing = false
case .failure(let error):
guard let strongSelf = self else { return }
completion(.failure(error))
strongSelf.isRefreshing = false
}
guard let strongSelf = self else { return }

// map success result from TokenSuccess to Void, and failure from OAuthSwiftError to Error
let refreshResult = result.map { _ in () }.mapError { $0 as Error }
completion(refreshResult)

strongSelf.isRefreshing = false
}
}

}

extension OAuth1Swift {

open var requestInterceptor: OAuthSwiftRequestInterceptor {
return OAuthSwiftRequestInterceptor(self)
}
Expand Down
31 changes: 30 additions & 1 deletion Tests/OAuthSwiftAlamofireTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//

import XCTest
import OAuthSwiftAlamofire
@testable import OAuthSwiftAlamofire
import OAuthSwift
import Alamofire

Expand Down Expand Up @@ -80,6 +80,35 @@ class OAuthSwiftAlamofireTests: XCTestCase {
}
waitForExpectations(timeout: 20, handler: nil)
}

// Note: oauthbin.com doesn't seem to exist anymore (domain name expire) and couldn't find another easily usable test server
// This test case needs to be updated if and when oauthbin.com comes back.
func testMultipleRequests() {
let exp1 = self.expectation(description: "auth should retry 1st request")
let exp2 = self.expectation(description: "auth should retry 2nd request")

let oauth2 = OAuth2Swift(
consumerKey: "tbd",
consumerSecret: "tbd",
authorizeUrl: "tbd",
accessTokenUrl: "tbd",
responseType: "code")

let interceptor = oauth2.requestInterceptor
let session = Session(interceptor: interceptor)

session.request("tbd", method: .get).validate().response { response in
XCTAssert(response.response?.statusCode == 200, "Failed request 1 auth")
exp1.fulfill()
}

session.request("tbd").validate().response { response in
XCTAssert(response.response?.statusCode == 200, "Failed request 2 auth")
exp2.fulfill()
}

waitForExpectations(timeout: 5.0, handler: nil)
}

}

Expand Down