diff --git a/Modules/Sources/NetworkingCore/Network/AlamofireNetwork.swift b/Modules/Sources/NetworkingCore/Network/AlamofireNetwork.swift index 0f0095f774e..19f128c412d 100644 --- a/Modules/Sources/NetworkingCore/Network/AlamofireNetwork.swift +++ b/Modules/Sources/NetworkingCore/Network/AlamofireNetwork.swift @@ -269,7 +269,10 @@ private extension AlamofireNetwork { .sink { [weak self] site, unsupportedList in guard let self else { return } guard let site, site.applicationPasswordAvailable, - unsupportedList.contains(site.siteID) == false else { + errorHandler.siteFlaggedAsUnsupported( + siteID: site.siteID, + unsupportedList: unsupportedList + ) == false else { requestConverter = RequestConverter(siteAddress: nil) requestAuthenticator.updateAuthenticator(DefaultRequestAuthenticator(credentials: credentials)) requestAuthenticator.delegate = nil @@ -357,8 +360,8 @@ extension Alamofire.DataResponse { // MARK: - Helper extension to save internal flag for app password availability // extension UserDefaults { - @objc dynamic var applicationPasswordUnsupportedList: [Int64] { - get { value(forKey: Key.applicationPasswordUnsupportedList.rawValue) as? [Int64] ?? [] } + @objc dynamic var applicationPasswordUnsupportedList: [String: Date] { + get { value(forKey: Key.applicationPasswordUnsupportedList.rawValue) as? [String: Date] ?? [:] } set { setValue(newValue, forKey: Key.applicationPasswordUnsupportedList.rawValue) } } diff --git a/Modules/Sources/NetworkingCore/Network/AlamofireNetworkErrorHandler.swift b/Modules/Sources/NetworkingCore/Network/AlamofireNetworkErrorHandler.swift index fa5a842c95d..7bf5cb9f094 100644 --- a/Modules/Sources/NetworkingCore/Network/AlamofireNetworkErrorHandler.swift +++ b/Modules/Sources/NetworkingCore/Network/AlamofireNetworkErrorHandler.swift @@ -135,14 +135,30 @@ final class AlamofireNetworkErrorHandler { func flagSiteAsUnsupported(for siteID: Int64) { queue.sync(flags: .barrier) { - let currentList = userDefaults.applicationPasswordUnsupportedList - userDefaults.applicationPasswordUnsupportedList = currentList + [siteID] + var currentList = userDefaults.applicationPasswordUnsupportedList + currentList[String(siteID)] = Date() + userDefaults.applicationPasswordUnsupportedList = currentList } } - // MARK: - Private methods + func siteFlaggedAsUnsupported(siteID: Int64, unsupportedList: [String: Date]) -> Bool { + guard let flagDate = unsupportedList[String(siteID)] else { + return false + } + + let timeElapsed = Date().timeIntervalSince(flagDate) + if timeElapsed < Constants.flagRefreshDuration { + return true + } else { + clearUnsupportedFlag(for: siteID) + return false + } + } +} - private func incrementFailureCount(for siteID: Int64) { +// MARK: Private helpers +private extension AlamofireNetworkErrorHandler { + func incrementFailureCount(for siteID: Int64) { let currentFailureCount = appPasswordFailures[siteID] ?? 0 let updatedCount = currentFailureCount + 1 if updatedCount == AppPasswordConstants.requestFailureThreshold { @@ -150,8 +166,20 @@ final class AlamofireNetworkErrorHandler { } appPasswordFailures[siteID] = updatedCount } -} + func clearUnsupportedFlag(for siteID: Int64) { + queue.sync(flags: .barrier) { + let currentList = userDefaults.applicationPasswordUnsupportedList + userDefaults.applicationPasswordUnsupportedList = currentList.filter { flag in + flag.key != String(siteID) + } + } + } + + enum Constants { + static let flagRefreshDuration: Double = 60 * 60 * 24 * 14 // flag can be reset after 14 days. + } +} /// Helper type to keep track of retried requests with accompanied error struct RetriedJetpackRequest { let request: JetpackRequest diff --git a/Modules/Tests/NetworkingTests/Network/AlamofireNetworkErrorHandlerTests.swift b/Modules/Tests/NetworkingTests/Network/AlamofireNetworkErrorHandlerTests.swift index 8ad98b95c50..384ef26544c 100644 --- a/Modules/Tests/NetworkingTests/Network/AlamofireNetworkErrorHandlerTests.swift +++ b/Modules/Tests/NetworkingTests/Network/AlamofireNetworkErrorHandlerTests.swift @@ -33,7 +33,7 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase { // Then - should not flag site as unsupported even after more failures simulateFailureCount(5, for: siteID) // Would normally reach threshold - XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.contains(siteID)) + XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } func test_shouldRetryJetpackRequest_returns_false_for_nil_credentials() { @@ -122,20 +122,21 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase { errorHandler.flagSiteAsUnsupported(for: siteID) // Then - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } func test_flagSiteAsUnsupported_appends_to_existing_list() { // Given let existingSiteID: Int64 = 789 let newSiteID: Int64 = 456 - userDefaults.applicationPasswordUnsupportedList = [existingSiteID] + userDefaults.applicationPasswordUnsupportedList = [String(existingSiteID): Date()] // When errorHandler.flagSiteAsUnsupported(for: newSiteID) // Then - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [existingSiteID, newSiteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(existingSiteID))) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(newSiteID))) } // MARK: - Thread Safety Tests @@ -273,6 +274,114 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase { wait(for: [expectation], timeout: 3.0) } + // MARK: - siteFlaggedAsUnsupported Tests + + func test_siteFlaggedAsUnsupported_returns_false_when_site_not_in_list() { + // Given + let siteID: Int64 = 123 + let unsupportedList: [String: Date] = [:] + + // When + let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: unsupportedList) + + // Then + XCTAssertFalse(isFlagged) + } + + func test_siteFlaggedAsUnsupported_returns_false_when_timestamp_string_invalid() { + // Given + let siteID: Int64 = 123 + let unsupportedList: [String: Date] = [String(siteID): Date(timeIntervalSince1970: 0)] + + // When + let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: unsupportedList) + + // Then + XCTAssertFalse(isFlagged) + } + + func test_siteFlaggedAsUnsupported_returns_true_when_flag_is_recent() { + // Given + let siteID: Int64 = 123 + let recentDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60)) // 1 hour ago + let unsupportedList: [String: Date] = [String(siteID): recentDate] + + // When + let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: unsupportedList) + + // Then + XCTAssertTrue(isFlagged) + } + + func test_siteFlaggedAsUnsupported_returns_false_and_clears_flag_when_expired() { + // Given + let siteID: Int64 = 123 + let expiredDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60 * 24 * 15)) // 15 days ago (expired) + userDefaults.applicationPasswordUnsupportedList = [String(siteID): expiredDate] + + // When + let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: userDefaults.applicationPasswordUnsupportedList) + + // Then + XCTAssertFalse(isFlagged) + // Verify the flag was cleared from UserDefaults + XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) + } + + func test_siteFlaggedAsUnsupported_returns_true_for_flag_at_boundary_time() { + // Given + let siteID: Int64 = 123 + let boundaryDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60 * 24 * 7 - 1)) // Just under 7 days ago + let unsupportedList: [String: Date] = [String(siteID): boundaryDate] + + // When + let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: unsupportedList) + + // Then + XCTAssertTrue(isFlagged) + } + + func test_siteFlaggedAsUnsupported_returns_false_for_flag_just_over_boundary() { + // Given + let siteID: Int64 = 123 + let expiredDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60 * 24 * 14 + 1)) // Just over 7 days ago + userDefaults.applicationPasswordUnsupportedList = [String(siteID): expiredDate] + + // When + let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: userDefaults.applicationPasswordUnsupportedList) + + // Then + XCTAssertFalse(isFlagged) + // Verify the flag was cleared from UserDefaults + XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) + } + + func test_siteFlaggedAsUnsupported_handles_multiple_sites_correctly() { + // Given + let siteID1: Int64 = 123 + let siteID2: Int64 = 456 + let siteID3: Int64 = 789 + let recentDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60)) // 1 hour ago + let expiredDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60 * 24 * 15)) // 15 days ago + + userDefaults.applicationPasswordUnsupportedList = [ + String(siteID1): recentDate, + String(siteID2): expiredDate, + String(siteID3): recentDate + ] + + // When & Then + let list = userDefaults.applicationPasswordUnsupportedList + XCTAssertTrue(errorHandler.siteFlaggedAsUnsupported(siteID: siteID1, unsupportedList: list)) + XCTAssertFalse(errorHandler.siteFlaggedAsUnsupported(siteID: siteID2, unsupportedList: userDefaults.applicationPasswordUnsupportedList)) + XCTAssertTrue(errorHandler.siteFlaggedAsUnsupported(siteID: siteID3, unsupportedList: userDefaults.applicationPasswordUnsupportedList)) + + // Verify expired flag was cleared but others remain + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID1))) + XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID2))) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID3))) + } + // MARK: - Integration Tests func test_handleFailureForDirectRequestIfNeeded_calls_correct_callbacks() { @@ -352,7 +461,7 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase { ) // Then - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } func test_flagSiteAsUnsupportedForAppPasswordIfNeeded_handles_disabled_error_codes() { @@ -383,7 +492,7 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase { ) // Then - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } } diff --git a/Modules/Tests/NetworkingTests/Network/AlamofireNetworkTests.swift b/Modules/Tests/NetworkingTests/Network/AlamofireNetworkTests.swift index 7cd41344545..2b3149cf333 100644 --- a/Modules/Tests/NetworkingTests/Network/AlamofireNetworkTests.swift +++ b/Modules/Tests/NetworkingTests/Network/AlamofireNetworkTests.swift @@ -187,21 +187,22 @@ final class AlamofireNetworkTests: XCTestCase { network.didFailToAuthenticateRequestWithAppPassword(siteID: siteID) // Then - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } func test_didFailToAuthenticateRequestWithAppPassword_appends_to_existing_unsupported_list() { // Given let existingSiteID: Int64 = 456 let newSiteID: Int64 = 123 - userDefaults.applicationPasswordUnsupportedList = [existingSiteID] + userDefaults.applicationPasswordUnsupportedList = [String(existingSiteID): Date()] let network = AlamofireNetwork(credentials: nil, userDefaults: userDefaults) // When network.didFailToAuthenticateRequestWithAppPassword(siteID: newSiteID) // Then - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [existingSiteID, newSiteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(existingSiteID))) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(newSiteID))) } // MARK: - Session Initialization Tests @@ -328,7 +329,7 @@ final class AlamofireNetworkTests: XCTestCase { // Then XCTAssertNil(result.1) XCTAssertNotNil(result.0) - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } func test_responseDataAndHeaders_retries_direct_request_when_converted_request_fails() async throws { @@ -374,7 +375,7 @@ final class AlamofireNetworkTests: XCTestCase { // Then let responseDict = try JSONSerialization.jsonObject(with: result.0, options: []) as? [String: String] XCTAssertEqual(responseDict?["reports"], "data") - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } func test_responseDataPublisher_retries_direct_request_when_converted_request_fails() { @@ -430,7 +431,7 @@ final class AlamofireNetworkTests: XCTestCase { // Then XCTAssertTrue(result.isSuccess) - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } func test_uploadMultipartFormData_retries_direct_request_when_converted_request_fails() { @@ -491,7 +492,7 @@ final class AlamofireNetworkTests: XCTestCase { // Then XCTAssertNil(result.1) XCTAssertNotNil(result.0) - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } // MARK: - Application Password Error Code Tests @@ -524,7 +525,7 @@ final class AlamofireNetworkTests: XCTestCase { // Then XCTAssertNil(result.1) XCTAssertNotNil(result.0) - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } func test_responseData_increments_failure_count_when_jetpack_retry_succeeds_after_unknown_error() throws { @@ -590,7 +591,7 @@ final class AlamofireNetworkTests: XCTestCase { } // Then - XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID]) + XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID))) } func test_responseData_does_not_retry_when_jetpack_request_not_available_as_rest() throws {