Skip to content

Commit 6444d99

Browse files
itsmeichigoitsmeichigo
authored
Application password experiment: Clear flagged sites after 14 days (#16107)
2 parents e425e44 + 8e8839a commit 6444d99

File tree

4 files changed

+164
-23
lines changed

4 files changed

+164
-23
lines changed

‎Modules/Sources/NetworkingCore/Network/AlamofireNetwork.swift‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,10 @@ private extension AlamofireNetwork {
269269
.sink { [weak self] site, unsupportedList in
270270
guard let self else { return }
271271
guard let site, site.applicationPasswordAvailable,
272-
unsupportedList.contains(site.siteID) == false else {
272+
errorHandler.siteFlaggedAsUnsupported(
273+
siteID: site.siteID,
274+
unsupportedList: unsupportedList
275+
) == false else {
273276
requestConverter = RequestConverter(siteAddress: nil)
274277
requestAuthenticator.updateAuthenticator(DefaultRequestAuthenticator(credentials: credentials))
275278
requestAuthenticator.delegate = nil
@@ -357,8 +360,8 @@ extension Alamofire.DataResponse {
357360
// MARK: - Helper extension to save internal flag for app password availability
358361
//
359362
extension UserDefaults {
360-
@objc dynamic var applicationPasswordUnsupportedList: [Int64] {
361-
get { value(forKey: Key.applicationPasswordUnsupportedList.rawValue) as? [Int64] ?? [] }
363+
@objc dynamic var applicationPasswordUnsupportedList: [String: Date] {
364+
get { value(forKey: Key.applicationPasswordUnsupportedList.rawValue) as? [String: Date] ?? [:] }
362365
set { setValue(newValue, forKey: Key.applicationPasswordUnsupportedList.rawValue) }
363366
}
364367

‎Modules/Sources/NetworkingCore/Network/AlamofireNetworkErrorHandler.swift‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -135,23 +135,51 @@ final class AlamofireNetworkErrorHandler {
135135

136136
func flagSiteAsUnsupported(for siteID: Int64) {
137137
queue.sync(flags: .barrier) {
138-
let currentList = userDefaults.applicationPasswordUnsupportedList
139-
userDefaults.applicationPasswordUnsupportedList = currentList + [siteID]
138+
var currentList = userDefaults.applicationPasswordUnsupportedList
139+
currentList[String(siteID)] = Date()
140+
userDefaults.applicationPasswordUnsupportedList = currentList
140141
}
141142
}
142143

143-
// MARK: - Private methods
144+
func siteFlaggedAsUnsupported(siteID: Int64, unsupportedList: [String: Date]) -> Bool {
145+
guard let flagDate = unsupportedList[String(siteID)] else {
146+
return false
147+
}
148+
149+
let timeElapsed = Date().timeIntervalSince(flagDate)
150+
if timeElapsed < Constants.flagRefreshDuration {
151+
return true
152+
} else {
153+
clearUnsupportedFlag(for: siteID)
154+
return false
155+
}
156+
}
157+
}
144158

145-
private func incrementFailureCount(for siteID: Int64) {
159+
// MARK: Private helpers
160+
private extension AlamofireNetworkErrorHandler {
161+
func incrementFailureCount(for siteID: Int64) {
146162
let currentFailureCount = appPasswordFailures[siteID] ?? 0
147163
let updatedCount = currentFailureCount + 1
148164
if updatedCount == AppPasswordConstants.requestFailureThreshold {
149165
flagSiteAsUnsupported(for: siteID)
150166
}
151167
appPasswordFailures[siteID] = updatedCount
152168
}
153-
}
154169

170+
func clearUnsupportedFlag(for siteID: Int64) {
171+
queue.sync(flags: .barrier) {
172+
let currentList = userDefaults.applicationPasswordUnsupportedList
173+
userDefaults.applicationPasswordUnsupportedList = currentList.filter { flag in
174+
flag.key != String(siteID)
175+
}
176+
}
177+
}
178+
179+
enum Constants {
180+
static let flagRefreshDuration: Double = 60 * 60 * 24 * 14 // flag can be reset after 14 days.
181+
}
182+
}
155183
/// Helper type to keep track of retried requests with accompanied error
156184
struct RetriedJetpackRequest {
157185
let request: JetpackRequest

‎Modules/Tests/NetworkingTests/Network/AlamofireNetworkErrorHandlerTests.swift‎

Lines changed: 115 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase {
3333

3434
// Then - should not flag site as unsupported even after more failures
3535
simulateFailureCount(5, for: siteID) // Would normally reach threshold
36-
XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.contains(siteID))
36+
XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
3737
}
3838

3939
func test_shouldRetryJetpackRequest_returns_false_for_nil_credentials() {
@@ -122,20 +122,21 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase {
122122
errorHandler.flagSiteAsUnsupported(for: siteID)
123123

124124
// Then
125-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
125+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
126126
}
127127

128128
func test_flagSiteAsUnsupported_appends_to_existing_list() {
129129
// Given
130130
let existingSiteID: Int64 = 789
131131
let newSiteID: Int64 = 456
132-
userDefaults.applicationPasswordUnsupportedList = [existingSiteID]
132+
userDefaults.applicationPasswordUnsupportedList = [String(existingSiteID): Date()]
133133

134134
// When
135135
errorHandler.flagSiteAsUnsupported(for: newSiteID)
136136

137137
// Then
138-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [existingSiteID, newSiteID])
138+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(existingSiteID)))
139+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(newSiteID)))
139140
}
140141

141142
// MARK: - Thread Safety Tests
@@ -273,6 +274,114 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase {
273274
wait(for: [expectation], timeout: 3.0)
274275
}
275276

277+
// MARK: - siteFlaggedAsUnsupported Tests
278+
279+
func test_siteFlaggedAsUnsupported_returns_false_when_site_not_in_list() {
280+
// Given
281+
let siteID: Int64 = 123
282+
let unsupportedList: [String: Date] = [:]
283+
284+
// When
285+
let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: unsupportedList)
286+
287+
// Then
288+
XCTAssertFalse(isFlagged)
289+
}
290+
291+
func test_siteFlaggedAsUnsupported_returns_false_when_timestamp_string_invalid() {
292+
// Given
293+
let siteID: Int64 = 123
294+
let unsupportedList: [String: Date] = [String(siteID): Date(timeIntervalSince1970: 0)]
295+
296+
// When
297+
let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: unsupportedList)
298+
299+
// Then
300+
XCTAssertFalse(isFlagged)
301+
}
302+
303+
func test_siteFlaggedAsUnsupported_returns_true_when_flag_is_recent() {
304+
// Given
305+
let siteID: Int64 = 123
306+
let recentDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60)) // 1 hour ago
307+
let unsupportedList: [String: Date] = [String(siteID): recentDate]
308+
309+
// When
310+
let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: unsupportedList)
311+
312+
// Then
313+
XCTAssertTrue(isFlagged)
314+
}
315+
316+
func test_siteFlaggedAsUnsupported_returns_false_and_clears_flag_when_expired() {
317+
// Given
318+
let siteID: Int64 = 123
319+
let expiredDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60 * 24 * 15)) // 15 days ago (expired)
320+
userDefaults.applicationPasswordUnsupportedList = [String(siteID): expiredDate]
321+
322+
// When
323+
let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: userDefaults.applicationPasswordUnsupportedList)
324+
325+
// Then
326+
XCTAssertFalse(isFlagged)
327+
// Verify the flag was cleared from UserDefaults
328+
XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
329+
}
330+
331+
func test_siteFlaggedAsUnsupported_returns_true_for_flag_at_boundary_time() {
332+
// Given
333+
let siteID: Int64 = 123
334+
let boundaryDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60 * 24 * 7 - 1)) // Just under 7 days ago
335+
let unsupportedList: [String: Date] = [String(siteID): boundaryDate]
336+
337+
// When
338+
let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: unsupportedList)
339+
340+
// Then
341+
XCTAssertTrue(isFlagged)
342+
}
343+
344+
func test_siteFlaggedAsUnsupported_returns_false_for_flag_just_over_boundary() {
345+
// Given
346+
let siteID: Int64 = 123
347+
let expiredDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60 * 24 * 14 + 1)) // Just over 7 days ago
348+
userDefaults.applicationPasswordUnsupportedList = [String(siteID): expiredDate]
349+
350+
// When
351+
let isFlagged = errorHandler.siteFlaggedAsUnsupported(siteID: siteID, unsupportedList: userDefaults.applicationPasswordUnsupportedList)
352+
353+
// Then
354+
XCTAssertFalse(isFlagged)
355+
// Verify the flag was cleared from UserDefaults
356+
XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
357+
}
358+
359+
func test_siteFlaggedAsUnsupported_handles_multiple_sites_correctly() {
360+
// Given
361+
let siteID1: Int64 = 123
362+
let siteID2: Int64 = 456
363+
let siteID3: Int64 = 789
364+
let recentDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60)) // 1 hour ago
365+
let expiredDate = Date(timeIntervalSince1970: Date().timeIntervalSince1970 - (60 * 60 * 24 * 15)) // 15 days ago
366+
367+
userDefaults.applicationPasswordUnsupportedList = [
368+
String(siteID1): recentDate,
369+
String(siteID2): expiredDate,
370+
String(siteID3): recentDate
371+
]
372+
373+
// When & Then
374+
let list = userDefaults.applicationPasswordUnsupportedList
375+
XCTAssertTrue(errorHandler.siteFlaggedAsUnsupported(siteID: siteID1, unsupportedList: list))
376+
XCTAssertFalse(errorHandler.siteFlaggedAsUnsupported(siteID: siteID2, unsupportedList: userDefaults.applicationPasswordUnsupportedList))
377+
XCTAssertTrue(errorHandler.siteFlaggedAsUnsupported(siteID: siteID3, unsupportedList: userDefaults.applicationPasswordUnsupportedList))
378+
379+
// Verify expired flag was cleared but others remain
380+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID1)))
381+
XCTAssertFalse(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID2)))
382+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID3)))
383+
}
384+
276385
// MARK: - Integration Tests
277386

278387
func test_handleFailureForDirectRequestIfNeeded_calls_correct_callbacks() {
@@ -352,7 +461,7 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase {
352461
)
353462

354463
// Then
355-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
464+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
356465
}
357466

358467
func test_flagSiteAsUnsupportedForAppPasswordIfNeeded_handles_disabled_error_codes() {
@@ -383,7 +492,7 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase {
383492
)
384493

385494
// Then
386-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
495+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
387496
}
388497
}
389498

‎Modules/Tests/NetworkingTests/Network/AlamofireNetworkTests.swift‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -187,21 +187,22 @@ final class AlamofireNetworkTests: XCTestCase {
187187
network.didFailToAuthenticateRequestWithAppPassword(siteID: siteID)
188188

189189
// Then
190-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
190+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
191191
}
192192

193193
func test_didFailToAuthenticateRequestWithAppPassword_appends_to_existing_unsupported_list() {
194194
// Given
195195
let existingSiteID: Int64 = 456
196196
let newSiteID: Int64 = 123
197-
userDefaults.applicationPasswordUnsupportedList = [existingSiteID]
197+
userDefaults.applicationPasswordUnsupportedList = [String(existingSiteID): Date()]
198198
let network = AlamofireNetwork(credentials: nil, userDefaults: userDefaults)
199199

200200
// When
201201
network.didFailToAuthenticateRequestWithAppPassword(siteID: newSiteID)
202202

203203
// Then
204-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [existingSiteID, newSiteID])
204+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(existingSiteID)))
205+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(newSiteID)))
205206
}
206207

207208
// MARK: - Session Initialization Tests
@@ -328,7 +329,7 @@ final class AlamofireNetworkTests: XCTestCase {
328329
// Then
329330
XCTAssertNil(result.1)
330331
XCTAssertNotNil(result.0)
331-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
332+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
332333
}
333334

334335
func test_responseDataAndHeaders_retries_direct_request_when_converted_request_fails() async throws {
@@ -374,7 +375,7 @@ final class AlamofireNetworkTests: XCTestCase {
374375
// Then
375376
let responseDict = try JSONSerialization.jsonObject(with: result.0, options: []) as? [String: String]
376377
XCTAssertEqual(responseDict?["reports"], "data")
377-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
378+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
378379
}
379380

380381
func test_responseDataPublisher_retries_direct_request_when_converted_request_fails() {
@@ -430,7 +431,7 @@ final class AlamofireNetworkTests: XCTestCase {
430431

431432
// Then
432433
XCTAssertTrue(result.isSuccess)
433-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
434+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
434435
}
435436

436437
func test_uploadMultipartFormData_retries_direct_request_when_converted_request_fails() {
@@ -491,7 +492,7 @@ final class AlamofireNetworkTests: XCTestCase {
491492
// Then
492493
XCTAssertNil(result.1)
493494
XCTAssertNotNil(result.0)
494-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
495+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
495496
}
496497

497498
// MARK: - Application Password Error Code Tests
@@ -524,7 +525,7 @@ final class AlamofireNetworkTests: XCTestCase {
524525
// Then
525526
XCTAssertNil(result.1)
526527
XCTAssertNotNil(result.0)
527-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
528+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
528529
}
529530

530531
func test_responseData_increments_failure_count_when_jetpack_retry_succeeds_after_unknown_error() throws {
@@ -590,7 +591,7 @@ final class AlamofireNetworkTests: XCTestCase {
590591
}
591592

592593
// Then
593-
XCTAssertEqual(userDefaults.applicationPasswordUnsupportedList, [siteID])
594+
XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
594595
}
595596

596597
func test_responseData_does_not_retry_when_jetpack_request_not_available_as_rest() throws {

0 commit comments

Comments
 (0)