Simplify error handling for RequestProcessor

Author: itsmeichigo

Modules/Sources/NetworkingCore/ApplicationPassword/RequestProcessor.swift

@@ -16,8 +16,6 @@ final class RequestProcessor: RequestInterceptor {
 
     private let notificationCenter: NotificationCenter
 
-    private var isAuthenticatedWithWPCom = false
-
     private var currentSiteID: Int64?
 
     weak var delegate: RequestProcessorDelegate?
@@ -30,12 +28,6 @@ final class RequestProcessor: RequestInterceptor {
 
     func updateAuthenticator(_ authenticator: RequestAuthenticator) {
         requestAuthenticator = authenticator
-        isAuthenticatedWithWPCom = {
-            switch authenticator.credentials {
-            case .some(.wpcom): true
-            default: false
-            }
-        }()
         currentSiteID = authenticator.jetpackSiteID
     }
 }
@@ -88,8 +80,9 @@ private extension RequestProcessor {
                 // Post a notification for tracking
                 notificationCenter.post(name: .ApplicationPasswordsGenerationFailed, object: error, userInfo: nil)
 
-                if isAuthenticatedWithWPCom, let currentSiteID {
-                    await retryPasswordGenerationIfNeeded(error: error, siteID: currentSiteID)
+                let shouldRetry = await checkIfRetryingGenerationIsNeeded(error: error)
+                if shouldRetry {
+                    generateApplicationPassword()
                 } else {
                     isAuthenticating = false
                     completeRequests(false)
@@ -98,26 +91,28 @@ private extension RequestProcessor {
         }
     }
 
+    /// Checks error code to retry or mark site as unsupported for app password.
+    /// Returns whether retry is needed.
     @MainActor
-    func retryPasswordGenerationIfNeeded(error: Error, siteID: Int64) async {
-        defer {
-            isAuthenticating = false
+    func checkIfRetryingGenerationIsNeeded(error: Error) async -> Bool {
+        guard let currentSiteID else {
+            return false
         }
         switch error {
-        case AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 409)):
+        case NetworkError.unacceptableStatusCode(let statusCode, _) where statusCode == 409:
             /// Password with the same name already exists. Request deletion remotely and retry.
             do {
                 try await requestAuthenticator.deleteApplicationPassword()
-                generateApplicationPassword()
+                return true
             } catch {
-                completeRequests(false)
+                return false
             }
-        case AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 404)):
+        case NetworkError.notFound:
             /// Site doesn't support application password
-            completeRequests(false)
-            delegate?.didFailToAuthenticateRequestWithApplicationPassword(siteID: siteID)
+            delegate?.didFailToAuthenticateRequestWithApplicationPassword(siteID: currentSiteID)
+            return false
         default:
-            completeRequests(false)
+            return false
         }
     }
 

Modules/Tests/NetworkingTests/ApplicationPassword/RequestProcessorTests.swift

@@ -301,13 +301,14 @@ final class RequestProcessorTests: XCTestCase {
         let session = Alamofire.Session(configuration: URLSessionConfiguration.default)
         let request = try mockRequest()
         let wpcomCredentials = Networking.Credentials.wpcom(username: "test", authToken: "token", siteAddress: "test.com")
+        let mockDelegate = MockRequestProcessorDelegate()
+
         mockRequestAuthenticator.credentials = wpcomCredentials
         mockRequestAuthenticator.jetpackSiteID = 123
-        let mockDelegate = MockRequestProcessorDelegate()
         sut.updateAuthenticator(mockRequestAuthenticator)
         sut.delegate = mockDelegate
 
-        let notFound404Error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 404))
+        let notFound404Error = NetworkError.notFound(response: nil)
         mockRequestAuthenticator.mockErrorWhileGeneratingPassword = notFound404Error
 
         // When
@@ -330,7 +331,9 @@ final class RequestProcessorTests: XCTestCase {
         let session = Alamofire.Session(configuration: URLSessionConfiguration.default)
         let request = try mockRequest()
         let wpcomCredentials = Networking.Credentials.wpcom(username: "test", authToken: "token", siteAddress: "test.com")
+
         mockRequestAuthenticator.credentials = wpcomCredentials
+        mockRequestAuthenticator.jetpackSiteID = 123
         sut.updateAuthenticator(mockRequestAuthenticator)
 
         let genericError = NSError(domain: "TestError", code: 500, userInfo: nil)
@@ -354,10 +357,12 @@ final class RequestProcessorTests: XCTestCase {
         let session = Alamofire.Session(configuration: URLSessionConfiguration.default)
         let request = try mockRequest()
         let wpcomCredentials = Networking.Credentials.wpcom(username: "test", authToken: "token", siteAddress: "test.com")
+
         mockRequestAuthenticator.credentials = wpcomCredentials
+        mockRequestAuthenticator.jetpackSiteID = 123
         sut.updateAuthenticator(mockRequestAuthenticator)
 
-        let conflict409Error = AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: 409))
+        let conflict409Error = NetworkError.unacceptableStatusCode(statusCode: 409, response: nil)
         let deletionError = NSError(domain: "TestError", code: 500, userInfo: nil)
         mockRequestAuthenticator.mockErrorWhileGeneratingPassword = conflict409Error
         mockRequestAuthenticator.mockErrorWhileDeletingPassword = deletionError

WooCommerce/WooCommerceTests/Yosemite/StoresManagerTests.swift

@@ -437,6 +437,7 @@ final class StoresManagerTests: XCTestCase {
 
     /// Verifies that user is logged out when application password regeneration fails if authenticated with wporg
     ///
+    @MainActor
     func test_it_deauthenticates_upon_receiving_application_password_generation_failure_notification_when_authenticated_without_wpcom() {
         // Given
         let manager = DefaultStoresManager.testingInstance
@@ -445,15 +446,14 @@ final class StoresManagerTests: XCTestCase {
             isLoggedInValues.append(isLoggedIn)
         }
         manager.authenticate(credentials: SessionSettings.wporgCredentials)
+        XCTAssertTrue(manager.isAuthenticated)
 
         // When
         let error = ApplicationPasswordUseCaseError.unauthorizedRequest
         MockNotificationCenter.testingInstance.post(name: .ApplicationPasswordsGenerationFailed, object: error, userInfo: nil)
 
         // Assert
-        waitUntil {
-            manager.isAuthenticated == false
-        }
+        XCTAssertFalse(manager.isAuthenticated)
         XCTAssertEqual(isLoggedInValues, [false, true, false])
     }