From a847babb6709a6efc839ccfa025a250de51a20d6 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Thu, 23 May 2024 15:47:57 -0300 Subject: [PATCH 1/4] Implement exponential backoff for optout retries --- .../OperationPreferredDateCalculator.swift | 8 +- ...perationPreferredDateCalculatorTests.swift | 80 ++++++++++++++++++- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift index ca42eaedda..4bb542d01e 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift @@ -78,7 +78,7 @@ struct OperationPreferredDateCalculator { return currentPreferredRunDate } case .error: - return date.now.addingTimeInterval(schedulingConfig.retryError.hoursToSeconds) + return date.now.addingTimeInterval(calculateNextRunDate(schedulingConfig: schedulingConfig, historyEvents: historyEvents)) case .optOutStarted, .scanStarted, .noMatchFound: return currentPreferredRunDate case .optOutConfirmed, .optOutRequested: @@ -97,4 +97,10 @@ struct OperationPreferredDateCalculator { let lastRemovalEventDate = lastRemovalEvent.date.addingTimeInterval(schedulingConfig.maintenanceScan.hoursToSeconds) return lastRemovalEventDate < Date() } + + private func calculateNextRunDate(schedulingConfig: DataBrokerScheduleConfig, + historyEvents: [HistoryEvent]) -> TimeInterval { + let pastTries = historyEvents.filter { $0.isError }.count + return min(Int(pow(2.0, Double(pastTries))), schedulingConfig.retryError).hoursToSeconds + } } diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/OperationPreferredDateCalculatorTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/OperationPreferredDateCalculatorTests.swift index 959f9555b4..df8b19acff 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/OperationPreferredDateCalculatorTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/OperationPreferredDateCalculatorTests.swift @@ -23,7 +23,7 @@ import XCTest final class OperationPreferredDateCalculatorTests: XCTestCase { private let schedulingConfig = DataBrokerScheduleConfig( - retryError: 1000, + retryError: 48, confirmOptOutScan: 2000, maintenanceScan: 3000 ) @@ -322,17 +322,89 @@ final class OperationPreferredDateCalculatorTests: XCTestCase { XCTAssertTrue(areDatesEqualIgnoringSeconds(date1: expectedOptOutDate, date2: actualOptOutDate)) } - func testError_thenOptOutDateIsRetry() throws { - let expectedOptOutDate = Date().addingTimeInterval(schedulingConfig.retryError.hoursToSeconds) - + func testWhenOptOutFailedOnce_thenWeRetryInTwoHours() throws { + let expectedOptOutDate = Calendar.current.date(byAdding: .hour, value: 2, to: Date())! let historyEvents = [ HistoryEvent(extractedProfileId: 1, brokerId: 1, profileQueryId: 1, type: .error(error: DataBrokerProtectionError.malformedURL))] + let calculator = OperationPreferredDateCalculator() + let actualOptOutDate = try calculator.dateForOptOutOperation(currentPreferredRunDate: nil, + historyEvents: historyEvents, + extractedProfileID: nil, + schedulingConfig: schedulingConfig) + + XCTAssertTrue(areDatesEqualIgnoringSeconds(date1: expectedOptOutDate, date2: actualOptOutDate)) + } + func testWhenOptOutFailedTwice_thenWeRetryInFourHours() throws { + let expectedOptOutDate = Calendar.current.date(byAdding: .hour, value: 4, to: Date())! + let historyEvents: [HistoryEvent] = [ + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)) + ] let calculator = OperationPreferredDateCalculator() + let actualOptOutDate = try calculator.dateForOptOutOperation(currentPreferredRunDate: nil, + historyEvents: historyEvents, + extractedProfileID: nil, + schedulingConfig: schedulingConfig) + XCTAssertTrue(areDatesEqualIgnoringSeconds(date1: expectedOptOutDate, date2: actualOptOutDate)) + } + + func testWhenOptOutFailedThreeTimes_thenWeRetryInEightHours() throws { + let expectedOptOutDate = Calendar.current.date(byAdding: .hour, value: 8, to: Date())! + let historyEvents: [HistoryEvent] = [ + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)) + ] + let calculator = OperationPreferredDateCalculator() + let actualOptOutDate = try calculator.dateForOptOutOperation(currentPreferredRunDate: nil, + historyEvents: historyEvents, + extractedProfileID: nil, + schedulingConfig: schedulingConfig) + + XCTAssertTrue(areDatesEqualIgnoringSeconds(date1: expectedOptOutDate, date2: actualOptOutDate)) + } + + func testWhenOptOutFailedSixTimes_thenWeRetryInTwoDays() throws { + let expectedOptOutDate = Calendar.current.date(byAdding: .day, value: 2, to: Date())! + let historyEvents: [HistoryEvent] = [ + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)) + ] + let calculator = OperationPreferredDateCalculator() + let actualOptOutDate = try calculator.dateForOptOutOperation(currentPreferredRunDate: nil, + historyEvents: historyEvents, + extractedProfileID: nil, + schedulingConfig: schedulingConfig) + + XCTAssertTrue(areDatesEqualIgnoringSeconds(date1: expectedOptOutDate, date2: actualOptOutDate)) + } + + func testWhenOptOutFailedMoreThanTheThreshold_thenWeRetryAtTheSchedulingRetry() throws { + let expectedOptOutDate = Date().addingTimeInterval(schedulingConfig.retryError.hoursToSeconds) + let historyEvents: [HistoryEvent] = [ + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)) + ] + let calculator = OperationPreferredDateCalculator() let actualOptOutDate = try calculator.dateForOptOutOperation(currentPreferredRunDate: nil, historyEvents: historyEvents, extractedProfileID: nil, From c5407a0ec90c746b7d77a74ba422acffb81a1c1e Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Tue, 4 Jun 2024 21:38:10 -0300 Subject: [PATCH 2/4] Address feedback --- .../OperationPreferredDateCalculator.swift | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift index 4bb542d01e..54acf53812 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift @@ -29,17 +29,17 @@ struct SystemDate: DateProtocol { } struct OperationPreferredDateCalculator { - + func dateForScanOperation(currentPreferredRunDate: Date?, historyEvents: [HistoryEvent], extractedProfileID: Int64?, schedulingConfig: DataBrokerScheduleConfig, isDeprecated: Bool = false) throws -> Date? { - + guard let lastEvent = historyEvents.last else { throw DataBrokerProtectionError.cantCalculatePreferredRunDate } - + switch lastEvent.type { case .optOutConfirmed: if isDeprecated { @@ -57,17 +57,17 @@ struct OperationPreferredDateCalculator { return Date().addingTimeInterval(schedulingConfig.confirmOptOutScan.hoursToSeconds) } } - + func dateForOptOutOperation(currentPreferredRunDate: Date?, historyEvents: [HistoryEvent], extractedProfileID: Int64?, schedulingConfig: DataBrokerScheduleConfig, date: DateProtocol = SystemDate()) throws -> Date? { - + guard let lastEvent = historyEvents.last else { throw DataBrokerProtectionError.cantCalculatePreferredRunDate } - + switch lastEvent.type { case .matchesFound, .reAppearence: if let extractedProfileID = extractedProfileID, shouldScheduleNewOptOut(events: historyEvents, @@ -78,14 +78,14 @@ struct OperationPreferredDateCalculator { return currentPreferredRunDate } case .error: - return date.now.addingTimeInterval(calculateNextRunDate(schedulingConfig: schedulingConfig, historyEvents: historyEvents)) + return date.now.addingTimeInterval(calculateNextRunDateOnError(schedulingConfig: schedulingConfig, historyEvents: historyEvents)) case .optOutStarted, .scanStarted, .noMatchFound: return currentPreferredRunDate case .optOutConfirmed, .optOutRequested: return nil } } - + // If the time elapsed since the last profile removal exceeds the current date plus maintenance period (expired), we should proceed with scheduling a new opt-out request as the broker has failed to honor the previous one. private func shouldScheduleNewOptOut(events: [HistoryEvent], extractedProfileId: Int64, @@ -93,13 +93,13 @@ struct OperationPreferredDateCalculator { guard let lastRemovalEvent = events.last(where: { $0.type == .optOutRequested && $0.extractedProfileId == extractedProfileId }) else { return false } - + let lastRemovalEventDate = lastRemovalEvent.date.addingTimeInterval(schedulingConfig.maintenanceScan.hoursToSeconds) return lastRemovalEventDate < Date() } - - private func calculateNextRunDate(schedulingConfig: DataBrokerScheduleConfig, - historyEvents: [HistoryEvent]) -> TimeInterval { + + private func calculateNextRunDateOnError(schedulingConfig: DataBrokerScheduleConfig, + historyEvents: [HistoryEvent]) -> TimeInterval { let pastTries = historyEvents.filter { $0.isError }.count return min(Int(pow(2.0, Double(pastTries))), schedulingConfig.retryError).hoursToSeconds } From 8f389a009a2d1c4f3ce8dd383fa728d8087ceeb9 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Tue, 4 Jun 2024 21:41:05 -0300 Subject: [PATCH 3/4] SwiftLint --- .../OperationPreferredDateCalculator.swift | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift index 54acf53812..06b9f0b7e9 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Operations/OperationPreferredDateCalculator.swift @@ -29,17 +29,16 @@ struct SystemDate: DateProtocol { } struct OperationPreferredDateCalculator { - + func dateForScanOperation(currentPreferredRunDate: Date?, historyEvents: [HistoryEvent], extractedProfileID: Int64?, schedulingConfig: DataBrokerScheduleConfig, isDeprecated: Bool = false) throws -> Date? { - guard let lastEvent = historyEvents.last else { throw DataBrokerProtectionError.cantCalculatePreferredRunDate } - + switch lastEvent.type { case .optOutConfirmed: if isDeprecated { @@ -57,17 +56,16 @@ struct OperationPreferredDateCalculator { return Date().addingTimeInterval(schedulingConfig.confirmOptOutScan.hoursToSeconds) } } - + func dateForOptOutOperation(currentPreferredRunDate: Date?, historyEvents: [HistoryEvent], extractedProfileID: Int64?, schedulingConfig: DataBrokerScheduleConfig, date: DateProtocol = SystemDate()) throws -> Date? { - guard let lastEvent = historyEvents.last else { throw DataBrokerProtectionError.cantCalculatePreferredRunDate } - + switch lastEvent.type { case .matchesFound, .reAppearence: if let extractedProfileID = extractedProfileID, shouldScheduleNewOptOut(events: historyEvents, @@ -85,7 +83,7 @@ struct OperationPreferredDateCalculator { return nil } } - + // If the time elapsed since the last profile removal exceeds the current date plus maintenance period (expired), we should proceed with scheduling a new opt-out request as the broker has failed to honor the previous one. private func shouldScheduleNewOptOut(events: [HistoryEvent], extractedProfileId: Int64, @@ -93,11 +91,11 @@ struct OperationPreferredDateCalculator { guard let lastRemovalEvent = events.last(where: { $0.type == .optOutRequested && $0.extractedProfileId == extractedProfileId }) else { return false } - + let lastRemovalEventDate = lastRemovalEvent.date.addingTimeInterval(schedulingConfig.maintenanceScan.hoursToSeconds) return lastRemovalEventDate < Date() } - + private func calculateNextRunDateOnError(schedulingConfig: DataBrokerScheduleConfig, historyEvents: [HistoryEvent]) -> TimeInterval { let pastTries = historyEvents.filter { $0.isError }.count From 8f6c57129d4d5c005a672ed1c1e82c5ecc892790 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Tue, 4 Jun 2024 21:49:50 -0300 Subject: [PATCH 4/4] Add more tests --- ...perationPreferredDateCalculatorTests.swift | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/OperationPreferredDateCalculatorTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/OperationPreferredDateCalculatorTests.swift index df8b19acff..f491a389c9 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/OperationPreferredDateCalculatorTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/OperationPreferredDateCalculatorTests.swift @@ -369,6 +369,41 @@ final class OperationPreferredDateCalculatorTests: XCTestCase { XCTAssertTrue(areDatesEqualIgnoringSeconds(date1: expectedOptOutDate, date2: actualOptOutDate)) } + func testWhenOptOutFailedThreeTimes_thenWeRetryInSixteenHours() throws { + let expectedOptOutDate = Calendar.current.date(byAdding: .hour, value: 16, to: Date())! + let historyEvents: [HistoryEvent] = [ + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)) + ] + let calculator = OperationPreferredDateCalculator() + let actualOptOutDate = try calculator.dateForOptOutOperation(currentPreferredRunDate: nil, + historyEvents: historyEvents, + extractedProfileID: nil, + schedulingConfig: schedulingConfig) + + XCTAssertTrue(areDatesEqualIgnoringSeconds(date1: expectedOptOutDate, date2: actualOptOutDate)) + } + + func testWhenOptOutFailedThreeTimes_thenWeRetryInThirtyTwoHours() throws { + let expectedOptOutDate = Calendar.current.date(byAdding: .hour, value: 32, to: Date())! + let historyEvents: [HistoryEvent] = [ + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)), + .init(brokerId: 1, profileQueryId: 1, type: .error(error: .malformedURL)) + ] + let calculator = OperationPreferredDateCalculator() + let actualOptOutDate = try calculator.dateForOptOutOperation(currentPreferredRunDate: nil, + historyEvents: historyEvents, + extractedProfileID: nil, + schedulingConfig: schedulingConfig) + + XCTAssertTrue(areDatesEqualIgnoringSeconds(date1: expectedOptOutDate, date2: actualOptOutDate)) + } + func testWhenOptOutFailedSixTimes_thenWeRetryInTwoDays() throws { let expectedOptOutDate = Calendar.current.date(byAdding: .day, value: 2, to: Date())! let historyEvents: [HistoryEvent] = [