Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
import Alamofire
import Foundation

enum AppPasswordFailureReason {
case notSupported
case unknown
}

protocol RequestProcessorDelegate: AnyObject {
func didFailToAuthenticateRequestWithAppPassword(siteID: Int64, reason: AppPasswordFailureReason)
func didFailToAuthenticateRequestWithAppPassword(siteID: Int64)
}

/// Authenticates and retries requests
Expand Down Expand Up @@ -92,7 +87,7 @@ private extension RequestProcessor {
isAuthenticating = false
completeRequests(false)
if let currentSiteID {
notifyFailure(error, for: currentSiteID)
notifyFailureIfNeeded(error, for: currentSiteID)
}
}
}
Expand Down Expand Up @@ -120,22 +115,24 @@ private extension RequestProcessor {
}
}

func notifyFailure(_ error: Error, for siteID: Int64) {
let reason: AppPasswordFailureReason = {
func notifyFailureIfNeeded(_ error: Error, for siteID: Int64) {
let appPasswordNotSupported: Bool = {
switch error {
case NetworkError.notFound:
return .notSupported
return true
case let networkError as NetworkError:
if let code = networkError.errorCode,
AppPasswordConstants.disabledCodes.contains(code) {
return .notSupported
return true
}
return .unknown
return false
default:
return .unknown
return false
}
}()
delegate?.didFailToAuthenticateRequestWithAppPassword(siteID: siteID, reason: reason)
if appPasswordNotSupported {
delegate?.didFailToAuthenticateRequestWithAppPassword(siteID: siteID)
}
}

func shouldRetry(_ error: Error) -> Bool {
Expand Down
70 changes: 43 additions & 27 deletions Modules/Sources/NetworkingCore/Network/AlamofireNetwork.swift
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,26 @@ private extension AlamofireNetwork {
func shouldRetryJetpackRequest(originalRequest: URLRequestConvertible,
convertedRequest: URLRequestConvertible,
failure: Error?) -> Bool {
if let request = originalRequest as? JetpackRequest,
convertedRequest is RESTRequest,
case .some(.wpcom) = credentials,
let failure = failure as? NetworkError {
let retriedRequest = RetriedJetpackRequest(request: request, error: failure)
guard let error = failure,
let request = originalRequest as? JetpackRequest,
convertedRequest is RESTRequest,
case .some(.wpcom) = credentials else {
return false
}

let isExpectedError: Bool = {
switch error {
case AFError.requestAdaptationFailed:
return true
case _ as NetworkError:
return true
default:
return false
}
}()

if isExpectedError {
let retriedRequest = RetriedJetpackRequest(request: request, error: error)
retriedJetpackRequests.append(retriedRequest)
return true
}
Expand All @@ -336,12 +351,14 @@ private extension AlamofireNetwork {
let siteID = retriedJetpackRequests[index].request.siteID
let originalFailure = retriedJetpackRequests[index].error
switch originalFailure {
case .unacceptableStatusCode(statusCode: 401, _),
.unacceptableStatusCode(statusCode: 403, _),
.unacceptableStatusCode(statusCode: 429, _):
case NetworkError.unacceptableStatusCode(statusCode: 401, _),
NetworkError.unacceptableStatusCode(statusCode: 403, _),
NetworkError.unacceptableStatusCode(statusCode: 429, _):
flagSiteAsUnsupported(for: siteID)
default:
if let code = originalFailure.errorCode, AppPasswordConstants.disabledCodes.contains(code) {
if let networkError = originalFailure as? NetworkError,
let code = networkError.errorCode,
AppPasswordConstants.disabledCodes.contains(code) {
flagSiteAsUnsupported(for: siteID)
} else {
incrementFailureCount(for: siteID)
Expand Down Expand Up @@ -372,36 +389,31 @@ private extension AlamofireNetwork {
///
struct RetriedJetpackRequest {
let request: JetpackRequest
let error: NetworkError
let error: Error
}

func incrementFailureCount(for siteID: Int64) {
let currentFailureCount = appPasswordFailures[siteID] ?? 0
let updatedCount = currentFailureCount + 1
if updatedCount == AppPasswordConstants.requestFailureThreshold {
let currentList = userDefaults.applicationPasswordUnsupportedList
userDefaults.applicationPasswordUnsupportedList = currentList + [siteID]
}
appPasswordFailures[siteID] = updatedCount
}
}

// MARK: `RequestProcessorDelegate` conformance
//
extension AlamofireNetwork: RequestProcessorDelegate {
func didFailToAuthenticateRequestWithAppPassword(siteID: Int64, reason: AppPasswordFailureReason) {
switch reason {
case .notSupported:
flagSiteAsUnsupported(for: siteID)
case .unknown:
incrementFailureCount(for: siteID)
}
func didFailToAuthenticateRequestWithAppPassword(siteID: Int64) {
flagSiteAsUnsupported(for: siteID)
}

func flagSiteAsUnsupported(for siteID: Int64) {
let currentList = userDefaults.applicationPasswordUnsupportedList
userDefaults.applicationPasswordUnsupportedList = currentList + [siteID]
}

func incrementFailureCount(for siteID: Int64) {
let currentFailureCount = appPasswordFailures[siteID] ?? 0
let updatedCount = currentFailureCount + 1
if updatedCount == AppPasswordConstants.requestFailureThreshold {
let currentList = userDefaults.applicationPasswordUnsupportedList
userDefaults.applicationPasswordUnsupportedList = currentList + [siteID]
}
appPasswordFailures[siteID] = updatedCount
}
}

// MARK: - Constants for direct request error handling
Expand Down Expand Up @@ -450,6 +462,10 @@ extension Alamofire.DataResponse {
return error
}

if case .some(AFError.requestAdaptationFailed) = error?.asAFError {
return error
}

return response.flatMap { response in
NetworkError(responseData: data,
statusCode: response.statusCode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,7 @@ final class RequestProcessorTests: XCTestCase {
// Then
XCTAssertFalse(shouldRetry.retryRequired)
waitUntil {
mockDelegate.didFailToAuthenticateRequestForSiteID == 123 &&
mockDelegate.didFailToAuthenticateRequestWithReason == .notSupported
mockDelegate.didFailToAuthenticateRequestForSiteID == 123
}
}

Expand Down Expand Up @@ -355,8 +354,7 @@ final class RequestProcessorTests: XCTestCase {
// Then
XCTAssertFalse(shouldRetry.retryRequired)
waitUntil {
mockDelegate.didFailToAuthenticateRequestForSiteID == 123 &&
mockDelegate.didFailToAuthenticateRequestWithReason == .notSupported
mockDelegate.didFailToAuthenticateRequestForSiteID == 123
}
}

Expand Down Expand Up @@ -388,8 +386,7 @@ final class RequestProcessorTests: XCTestCase {
// Then
XCTAssertFalse(shouldRetry.retryRequired)
waitUntil {
mockDelegate.didFailToAuthenticateRequestForSiteID == 123 &&
mockDelegate.didFailToAuthenticateRequestWithReason == .notSupported
mockDelegate.didFailToAuthenticateRequestForSiteID == 123
}
}

Expand Down Expand Up @@ -419,10 +416,6 @@ final class RequestProcessorTests: XCTestCase {
// Then
XCTAssertFalse(shouldRetry.retryRequired)
XCTAssertFalse(mockRequestAuthenticator.deleteApplicationPasswordCalled)
waitUntil {
mockDelegate.didFailToAuthenticateRequestForSiteID == 123 &&
mockDelegate.didFailToAuthenticateRequestWithReason == .unknown
}
}

func test_request_not_retried_when_password_deletion_fails_after_409_error_with_wpcom_authentication() throws {
Expand Down Expand Up @@ -531,10 +524,8 @@ private class MockRequest: Alamofire.DataRequest, @unchecked Sendable {

private class MockRequestProcessorDelegate: RequestProcessorDelegate {
private(set) var didFailToAuthenticateRequestForSiteID: Int64?
private(set) var didFailToAuthenticateRequestWithReason: AppPasswordFailureReason?

func didFailToAuthenticateRequestWithAppPassword(siteID: Int64, reason: AppPasswordFailureReason) {
func didFailToAuthenticateRequestWithAppPassword(siteID: Int64) {
didFailToAuthenticateRequestForSiteID = siteID
didFailToAuthenticateRequestWithReason = reason
}
}
56 changes: 4 additions & 52 deletions Modules/Tests/NetworkingTests/Network/AlamofireNetworkTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -177,81 +177,33 @@ final class AlamofireNetworkTests: XCTestCase {

// MARK: - `didFailToAuthenticateRequestWithAppPassword`

func test_didFailToAuthenticateRequestWithAppPassword_notSupported_adds_siteID_to_unsupported_list() {
func test_didFailToAuthenticateRequestWithAppPassword_adds_siteID_to_unsupported_list() {
// Given
let siteID: Int64 = 123
let network = AlamofireNetwork(credentials: nil, userDefaults: userDefaults)
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.isEmpty)

// When
network.didFailToAuthenticateRequestWithAppPassword(siteID: siteID, reason: .notSupported)
network.didFailToAuthenticateRequestWithAppPassword(siteID: siteID)

// Then
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
}

func test_didFailToAuthenticateRequestWithAppPassword_notSupported_appends_to_existing_unsupported_list() {
func test_didFailToAuthenticateRequestWithAppPassword_appends_to_existing_unsupported_list() {
// Given
let existingSiteID: Int64 = 456
let newSiteID: Int64 = 123
userDefaults.applicationPasswordUnsupportedList = [existingSiteID]
let network = AlamofireNetwork(credentials: nil, userDefaults: userDefaults)

// When
network.didFailToAuthenticateRequestWithAppPassword(siteID: newSiteID, reason: .notSupported)
network.didFailToAuthenticateRequestWithAppPassword(siteID: newSiteID)

// Then
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [existingSiteID, newSiteID])
}

func test_didFailToAuthenticateRequestWithAppPassword_unknown_below_threshold_does_not_add_to_unsupported_list() {
// Given
let siteID: Int64 = 123
let network = AlamofireNetwork(credentials: nil, userDefaults: userDefaults)
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.isEmpty)

// When - Call 9 times (below threshold of 10)
for _ in 1...9 {
network.didFailToAuthenticateRequestWithAppPassword(siteID: siteID, reason: .unknown)
}

// Then
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.isEmpty)
}

func test_didFailToAuthenticateRequestWithAppPassword_unknown_at_threshold_adds_siteID_to_unsupported_list() {
// Given
let siteID: Int64 = 123
let network = AlamofireNetwork(credentials: nil, userDefaults: userDefaults)
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.isEmpty)

// When - Call exactly 10 times (threshold)
for _ in 1...10 {
network.didFailToAuthenticateRequestWithAppPassword(siteID: siteID, reason: .unknown)
}

// Then
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
}

func test_didFailToAuthenticateRequestWithAppPassword_unknown_multiple_sites_tracks_separately() {
// Given
let siteID1: Int64 = 123
let siteID2: Int64 = 456
let network = AlamofireNetwork(credentials: nil, userDefaults: userDefaults)

// When - Call site1 5 times, site2 10 times
for _ in 1...5 {
network.didFailToAuthenticateRequestWithAppPassword(siteID: siteID1, reason: .unknown)
}
for _ in 1...10 {
network.didFailToAuthenticateRequestWithAppPassword(siteID: siteID2, reason: .unknown)
}

// Then - Only site2 should be in unsupported list
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID2])
}

// MARK: - Session Initialization Tests

func test_concurrent_requests_do_not_fail_with_sessionDeinitialized_error_when_ensuresSessionManagerIsInitialized_is_true() async throws {
Expand Down