Asychronously renew OAuth access tokens as needed for any 401 Unauthorized response from Feedly and automatically retry the request. Fixes #1859

Frameworks/Account/Feedly/FeedlyAPICaller.swift

@@ -9,6 +9,12 @@
 import Foundation
 import RSWeb
 
+protocol FeedlyAPICallerDelegate: class {
+	/// Implemented by the `FeedlyAccountDelegate` reauthorize the client with a fresh OAuth token so the client can retry the unauthorized request.
+	/// Pass `true` to the completion handler if the failing request should be retried with a fresh token or `false` if the unauthorized request should complete with the original failure error.
+	func reauthorizeFeedlyAPICaller(_ caller: FeedlyAPICaller, completionHandler: @escaping (Bool) -> ())
+}
+
 final class FeedlyAPICaller {
 	
 	enum API {
@@ -47,6 +53,8 @@ final class FeedlyAPICaller {
 		self.baseUrlComponents = api.baseUrlComponents
 	}
 	
+	weak var delegate: FeedlyAPICallerDelegate?
+	
 	var credentials: Credentials?
 	
 	var server: String? {
@@ -69,6 +77,54 @@ final class FeedlyAPICaller {
 		isSuspended = false
 	}
 	
+	func send<R: Decodable>(request: URLRequest, resultType: R.Type, dateDecoding: JSONDecoder.DateDecodingStrategy = .iso8601, keyDecoding: JSONDecoder.KeyDecodingStrategy = .useDefaultKeys, completion: @escaping (Result<(HTTPURLResponse, R?), Error>) -> Void) {
+		transport.send(request: request, resultType: resultType, dateDecoding: dateDecoding, keyDecoding: keyDecoding) { [weak self] result in
+			assert(Thread.isMainThread)
+			
+			switch result {
+			case .success:
+				completion(result)
+			case .failure(let error):
+				switch error {
+				case TransportError.httpError(let statusCode) where statusCode == 401:
+					
+					assert(self == nil ? true : self?.delegate != nil, "Check the delegate is set to \(FeedlyAccountDelegate.self).")
+					
+					guard let self = self, let delegate = self.delegate else {
+						completion(result)
+						return
+					}
+					
+					/// Capture the credentials before the reauthorization to check for a change.
+					let credentialsBefore = self.credentials
+					
+					delegate.reauthorizeFeedlyAPICaller(self) { [weak self] isReauthorizedAndShouldRetry in
+						assert(Thread.isMainThread)
+						
+						guard isReauthorizedAndShouldRetry, let self = self else {
+							completion(result)
+							return
+						}
+						
+						// Check for a change. Not only would it help debugging, but it'll also catch an infinitely recursive attempt to refresh.
+						guard let accessToken = self.credentials?.secret, accessToken != credentialsBefore?.secret else {
+							assertionFailure("Could not update the request with a new OAuth token. Did \(String(describing: self.delegate)) set them on \(self)?")
+							completion(result)
+							return
+						}
+						
+						var reauthorizedRequest = request
+						reauthorizedRequest.setValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
+						
+						self.send(request: reauthorizedRequest, resultType: resultType, dateDecoding: dateDecoding, keyDecoding: keyDecoding, completion: completion)
+					}
+				default:
+					completion(result)
+				}
+			}
+		}
+	}
+	
 	func importOpml(_ opmlData: Data, completion: @escaping (Result<Void, Error>) -> ()) {
 		guard !isSuspended else {
 			return DispatchQueue.main.async {
@@ -95,7 +151,7 @@ final class FeedlyAPICaller {
 		request.addValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
 		request.httpBody = opmlData
 		
-		transport.send(request: request, resultType: String.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: String.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (httpResponse, _)):
 				if httpResponse.statusCode == 200 {
@@ -147,7 +203,7 @@ final class FeedlyAPICaller {
 			}
 		}
 		
-		transport.send(request: request, resultType: [FeedlyCollection].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: [FeedlyCollection].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (httpResponse, collections)):
 				if httpResponse.statusCode == 200, let collection = collections?.first {
@@ -200,7 +256,7 @@ final class FeedlyAPICaller {
 			}
 		}
 		
-		transport.send(request: request, resultType: [FeedlyCollection].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: [FeedlyCollection].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (httpResponse, collections)):
 				if httpResponse.statusCode == 200, let collection = collections?.first {
@@ -248,7 +304,7 @@ final class FeedlyAPICaller {
 		request.addValue("application/json", forHTTPHeaderField: "Accept-Type")
 		request.addValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
 		
-		transport.send(request: request, resultType: String.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: String.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (httpResponse, _)):
 				if httpResponse.statusCode == 200 {
@@ -307,7 +363,7 @@ final class FeedlyAPICaller {
 			}
 		}
 		
-		transport.send(request: request, resultType: [FeedlyFeed].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: [FeedlyFeed].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success((let httpResponse, _)):
 				if httpResponse.statusCode == 200 {
@@ -369,7 +425,7 @@ extension FeedlyAPICaller: FeedlyAddFeedToCollectionService {
 			}
 		}
 		
-		transport.send(request: request, resultType: [FeedlyFeed].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: [FeedlyFeed].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success((_, let collectionFeeds)):
 				if let feeds = collectionFeeds {
@@ -435,7 +491,7 @@ extension FeedlyAPICaller: OAuthAuthorizationCodeGrantRequesting {
 			return
 		}
 		
-		transport.send(request: request, resultType: AccessTokenResponse.self, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: AccessTokenResponse.self, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (_, tokenResponse)):
 				if let response = tokenResponse {
@@ -482,7 +538,7 @@ extension FeedlyAPICaller: OAuthAcessTokenRefreshRequesting {
 			return
 		}
 		
-		transport.send(request: request, resultType: AccessTokenResponse.self, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: AccessTokenResponse.self, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (_, tokenResponse)):
 				if let response = tokenResponse {
@@ -523,7 +579,7 @@ extension FeedlyAPICaller: FeedlyGetCollectionsService {
 		request.addValue("application/json", forHTTPHeaderField: "Accept-Type")
 		request.addValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
 		
-		transport.send(request: request, resultType: [FeedlyCollection].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: [FeedlyCollection].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (_, collections)):
 				if let response = collections {
@@ -591,7 +647,7 @@ extension FeedlyAPICaller: FeedlyGetStreamContentsService {
 		request.addValue("application/json", forHTTPHeaderField: "Accept-Type")
 		request.addValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
 		
-		transport.send(request: request, resultType: FeedlyStream.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: FeedlyStream.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (_, collections)):
 				if let response = collections {
@@ -659,7 +715,7 @@ extension FeedlyAPICaller: FeedlyGetStreamIdsService {
 		request.addValue("application/json", forHTTPHeaderField: "Accept-Type")
 		request.addValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
 		
-		transport.send(request: request, resultType: FeedlyStreamIds.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: FeedlyStreamIds.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (_, collections)):
 				if let response = collections {
@@ -714,7 +770,7 @@ extension FeedlyAPICaller: FeedlyGetEntriesService {
 		request.addValue("application/json", forHTTPHeaderField: "Accept-Type")
 		request.addValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
 		
-		transport.send(request: request, resultType: [FeedlyEntry].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: [FeedlyEntry].self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (_, entries)):
 				if let response = entries {
@@ -773,7 +829,7 @@ extension FeedlyAPICaller: FeedlyMarkArticlesService {
 			}
 		}
 		
-		transport.send(request: request, resultType: String.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: String.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (httpResponse, _)):
 				if httpResponse.statusCode == 200 {
@@ -817,7 +873,7 @@ extension FeedlyAPICaller: FeedlySearchService {
 		request.addValue("application/json", forHTTPHeaderField: HTTPRequestHeader.contentType)
 		request.addValue("application/json", forHTTPHeaderField: "Accept-Type")
 		
-		transport.send(request: request, resultType: FeedlyFeedsSearchResponse.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: FeedlyFeedsSearchResponse.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (_, searchResponse)):
 				if let response = searchResponse {
@@ -859,7 +915,7 @@ extension FeedlyAPICaller: FeedlyLogoutService {
 		request.addValue("application/json", forHTTPHeaderField: "Accept-Type")
 		request.addValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
 		
-		transport.send(request: request, resultType: String.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
+		send(request: request, resultType: String.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in
 			switch result {
 			case .success(let (httpResponse, _)):
 				if httpResponse.statusCode == 200 {

Frameworks/Account/Feedly/FeedlyAccountDelegate.swift

@@ -37,12 +37,14 @@ final class FeedlyAccountDelegate: AccountDelegate {
 	
 	var credentials: Credentials? {
 		didSet {
+			#if DEBUG
 			// https://developer.feedly.com/v3/developer/
 			if let devToken = ProcessInfo.processInfo.environment["FEEDLY_DEV_ACCESS_TOKEN"], !devToken.isEmpty {
 				caller.credentials = Credentials(type: .oauthAccessToken, username: "Developer", secret: devToken)
-			} else {
+				return
-				caller.credentials = credentials
 			}
+			#endif
+			 caller.credentials = credentials
 		}
 	}
 	
@@ -52,6 +54,10 @@ final class FeedlyAccountDelegate: AccountDelegate {
 	
 	var refreshProgress = DownloadProgress(numberOfTasks: 0)
 	
+	/// Set on `accountDidInitialize` for the purposes of refreshing OAuth tokens when they expire.
+	/// See the implementation for `FeedlyAPICallerDelegate`.
+	private weak var initializedAccount: Account?
+	
 	internal let caller: FeedlyAPICaller
 	
 	private let log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "Feedly")
@@ -91,6 +97,8 @@ final class FeedlyAccountDelegate: AccountDelegate {
 		let databaseFilePath = (dataFolder as NSString).appendingPathComponent("Sync.sqlite3")
 		self.database = SyncDatabase(databaseFilePath: databaseFilePath)
 		self.oauthAuthorizationClient = api.oauthAuthorizationClient
+		
+		self.caller.delegate = self
 	}
 	
 	// MARK: Account API
@@ -112,17 +120,10 @@ final class FeedlyAccountDelegate: AccountDelegate {
 		
 		let log = self.log
 		
-		let refreshAccessToken = FeedlyRefreshAccessTokenOperation(account: account, service: self, oauthClient: oauthAuthorizationClient, refreshDate: Date(), log: log)
-		refreshAccessToken.downloadProgress = refreshProgress
-		operationQueue.add(refreshAccessToken)
-		
 		let syncAllOperation = FeedlySyncAllOperation(account: account, feedlyUserId: credentials.username, caller: caller, database: database, lastSuccessfulFetchStartDate: accountMetadata?.lastArticleFetchStartTime, downloadProgress: refreshProgress, log: log)
 		
 		syncAllOperation.downloadProgress = refreshProgress
 		
-		// Ensure the sync uses the latest credential.
-		syncAllOperation.addDependency(refreshAccessToken)
-		
 		let date = Date()
 		syncAllOperation.syncCompletionHandler = { [weak self] result in
 			if case .success = result {
@@ -500,6 +501,7 @@ final class FeedlyAccountDelegate: AccountDelegate {
 	}
 	
 	func accountDidInitialize(_ account: Account) {
+		initializedAccount = account
 		credentials = try? account.retrieveCredentials(type: .oauthAccessToken)
 	}
 	
@@ -533,3 +535,37 @@ final class FeedlyAccountDelegate: AccountDelegate {
 		caller.resume()
 	}
 }
+
+extension FeedlyAccountDelegate: FeedlyAPICallerDelegate {
+	
+	func reauthorizeFeedlyAPICaller(_ caller: FeedlyAPICaller, completionHandler: @escaping (Bool) -> ()) {
+		guard let account = initializedAccount else {
+			completionHandler(false)
+			return
+		}
+		
+		/// Captures a failure to refresh a token, assuming that it was refreshed unless told otherwise.
+		final class RefreshAccessTokenOperationDelegate: FeedlyOperationDelegate {
+			
+			private(set) var didReauthorize = true
+			
+			func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) {
+				didReauthorize = false
+			}
+		}
+		
+		let refreshAccessToken = FeedlyRefreshAccessTokenOperation(account: account, service: self, oauthClient: oauthAuthorizationClient, refreshDate: Date(), log: log)
+		refreshAccessToken.downloadProgress = refreshProgress
+		
+		/// This must be strongly referenced by the completionBlock of the `FeedlyRefreshAccessTokenOperation`.
+		let refreshAccessTokenDelegate = RefreshAccessTokenOperationDelegate()
+		refreshAccessToken.delegate = refreshAccessTokenDelegate
+		
+		refreshAccessToken.completionBlock = { operation in
+			assert(Thread.isMainThread)
+			completionHandler(refreshAccessTokenDelegate.didReauthorize && !operation.isCanceled)
+		}
+		
+		MainThreadOperationQueue.shared.add(refreshAccessToken)
+	}
+}