From 874b312804598dc9a9068a09cf814b2c107483e7 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 15 Sep 2024 20:46:37 -0700 Subject: [PATCH] Filter out pixels older than 28 days. --- Core/PersistentPixel.swift | 22 +++++++- Core/PersistentPixelStoring.swift | 4 ++ DuckDuckGoTests/PersistentPixelTests.swift | 58 +++++++++++++++++++--- 3 files changed, 77 insertions(+), 7 deletions(-) diff --git a/Core/PersistentPixel.swift b/Core/PersistentPixel.swift index 934f9b35b1..605d740b6e 100644 --- a/Core/PersistentPixel.swift +++ b/Core/PersistentPixel.swift @@ -42,13 +42,19 @@ public final class PersistentPixel: PersistentPixelFiring { enum Constants { static let lastProcessingDateKey = "com.duckduckgo.ios.persistent-pixel.last-processing-timestamp" + +#if DEBUG + static let minimumProcessingInterval: TimeInterval = .minutes(1) +#else static let minimumProcessingInterval: TimeInterval = .hours(1) +#endif } private let pixelFiring: PixelFiring.Type private let dailyPixelFiring: DailyPixelFiring.Type private let persistentPixelStorage: PersistentPixelStoring private let lastProcessingDateStorage: KeyValueStoring + private let calendar: Calendar private let dateGenerator: () -> Date private let pixelProcessingLock = NSLock() @@ -72,11 +78,13 @@ public final class PersistentPixel: PersistentPixelFiring { dailyPixelFiring: DailyPixelFiring.Type, persistentPixelStorage: PersistentPixelStoring, lastProcessingDateStorage: KeyValueStoring, + calendar: Calendar = .current, dateGenerator: @escaping () -> Date = { Date() }) { self.pixelFiring = pixelFiring self.dailyPixelFiring = dailyPixelFiring self.persistentPixelStorage = persistentPixelStorage self.lastProcessingDateStorage = lastProcessingDateStorage + self.calendar = calendar self.dateGenerator = dateGenerator } @@ -194,7 +202,7 @@ public final class PersistentPixel: PersistentPixelFiring { if let lastProcessingDate = lastProcessingDateStorage.object(forKey: Constants.lastProcessingDateKey) as? Date { let threshold = dateGenerator().addingTimeInterval(-Constants.minimumProcessingInterval) - if threshold < lastProcessingDate { + if threshold <= lastProcessingDate { pixelProcessingLock.unlock() completion(nil) return @@ -239,8 +247,20 @@ public final class PersistentPixel: PersistentPixelFiring { let failedPixelsAccessQueue = DispatchQueue(label: "Failed Pixel Retry Attempt Metadata Queue") var failedPixels: [PersistentPixelMetadata] = [] + let currentDate = dateGenerator() for pixelMetadata in queuedPixels { + if let originalSendDateString = pixelMetadata.timestamp, + let originalSendDate = dateFormatter.date(from: originalSendDateString), + let date28DaysAgo = calendar.date(byAdding: .day, value: -28, to: currentDate) { + if originalSendDate < date28DaysAgo { + continue + } + } else { + // If we don't have a timestamp for some reason, ignore the retry - retries are only useful if they have a timestamp attached + continue + } + var pixelParameters = pixelMetadata.additionalParameters pixelParameters[PixelParameters.retriedPixel] = "1" diff --git a/Core/PersistentPixelStoring.swift b/Core/PersistentPixelStoring.swift index 027b83ece6..b28802f561 100644 --- a/Core/PersistentPixelStoring.swift +++ b/Core/PersistentPixelStoring.swift @@ -39,6 +39,10 @@ public struct PersistentPixelMetadata: Codable, Equatable { case .regular: return eventName } } + + var timestamp: String? { + return additionalParameters[PixelParameters.originalPixelTimestamp] + } } protocol PersistentPixelStoring { diff --git a/DuckDuckGoTests/PersistentPixelTests.swift b/DuckDuckGoTests/PersistentPixelTests.swift index dc66cf2e6a..96532c298d 100644 --- a/DuckDuckGoTests/PersistentPixelTests.swift +++ b/DuckDuckGoTests/PersistentPixelTests.swift @@ -29,7 +29,9 @@ final class PersistentPixelTests: XCTestCase { var currentStorageURL: URL! var persistentStorage: DefaultPersistentPixelStorage! var timestampStorage: KeyValueStoring! - let testDateString = "2024-01-01T12:00:00Z" + + var testDateString: String! + var oldDateString: String! override func setUp() { super.setUp() @@ -40,6 +42,11 @@ final class PersistentPixelTests: XCTestCase { PixelFiringMock.tearDown() DelayedPixelFiringMock.tearDown() + + let formatter = ISO8601DateFormatter() + formatter.formatOptions = [.withInternetDateTime] + testDateString = formatter.string(from: Date()) + oldDateString = formatter.string(from: Date().addingTimeInterval(-.days(30))) } override func tearDown() { @@ -179,7 +186,7 @@ final class PersistentPixelTests: XCTestCase { let pixel = PersistentPixelMetadata( eventName: "test", pixelType: .daily, - additionalParameters: ["key": "value"], + additionalParameters: ["key": "value", PixelParameters.originalPixelTimestamp: testDateString], includedParameters: [.appVersion] ) @@ -194,7 +201,11 @@ final class PersistentPixelTests: XCTestCase { XCTAssert(storedPixels.isEmpty) XCTAssertEqual(PixelFiringMock.lastPixelName, "test_d") - XCTAssertEqual(PixelFiringMock.lastDailyPixelInfo?.params, ["key": "value", PixelParameters.retriedPixel: "1"]) + XCTAssertEqual(PixelFiringMock.lastDailyPixelInfo?.params, [ + "key": "value", + PixelParameters.retriedPixel: "1", + PixelParameters.originalPixelTimestamp: testDateString + ]) XCTAssertEqual(PixelFiringMock.lastDailyPixelInfo?.includedParams, [.appVersion]) } @@ -205,7 +216,7 @@ final class PersistentPixelTests: XCTestCase { let pixel = PersistentPixelMetadata( eventName: "test", pixelType: .count, - additionalParameters: ["key": "value"], + additionalParameters: ["key": "value", PixelParameters.originalPixelTimestamp: testDateString], includedParameters: [.appVersion] ) @@ -220,10 +231,39 @@ final class PersistentPixelTests: XCTestCase { XCTAssert(storedPixels.isEmpty) XCTAssertEqual(PixelFiringMock.lastPixelName, "test_c") - XCTAssertEqual(PixelFiringMock.lastDailyPixelInfo?.params, ["key": "value", PixelParameters.retriedPixel: "1"]) + XCTAssertEqual(PixelFiringMock.lastDailyPixelInfo?.params, [ + "key": "value", + PixelParameters.retriedPixel: "1", + PixelParameters.originalPixelTimestamp: testDateString + ]) XCTAssertEqual(PixelFiringMock.lastDailyPixelInfo?.includedParams, [.appVersion]) } + func testWhenPixelIsStored_AndSendQueuedPixelsIsCalled_AndPixelIsOlderThan28Days_ThenPixelIsNotSent_AndPixelIsNoLongerStored() throws { + let persistentPixel = createPersistentPixel() + let expectation = expectation(description: "sendQueuedPixels") + + let pixel = PersistentPixelMetadata( + eventName: "test", + pixelType: .daily, + additionalParameters: ["key": "value", PixelParameters.originalPixelTimestamp: oldDateString], + includedParameters: [.appVersion] + ) + + try persistentStorage.replaceStoredPixels(with: [pixel]) + persistentPixel.sendQueuedPixels { _ in + expectation.fulfill() + } + + wait(for: [expectation], timeout: 3.0) + + let storedPixels = try persistentStorage.storedPixels() + XCTAssert(storedPixels.isEmpty) + + XCTAssertNil(PixelFiringMock.lastPixelName) + XCTAssertNil(PixelFiringMock.lastDailyPixelInfo) + } + func testWhenPixelQueueIsProcessing_AndProcessingSucceeds_AndNewFailedPixelIsReceived_ThenPixelIsNotStoredUntilProcessingIsComplete() throws { PixelFiringMock.expectedCountPixelFireError = NSError(domain: "PixelFailure", code: 1) @@ -273,7 +313,13 @@ final class PersistentPixelTests: XCTestCase { DelayedPixelFiringMock.completionError = NSError(domain: "PixelFailure", code: 1) let persistentPixel = createPersistentPixel(pixelFiring: DelayedPixelFiringMock.self) - let initialPixel = PersistentPixelMetadata(eventName: "test", pixelType: .count, additionalParameters: [:], includedParameters: [.appVersion]) + let initialPixel = PersistentPixelMetadata( + eventName: "test", + pixelType: .count, + additionalParameters: [PixelParameters.originalPixelTimestamp: testDateString], + includedParameters: [.appVersion] + ) + try persistentStorage.replaceStoredPixels(with: [initialPixel]) let sendQueuedPixelsExpectation = expectation(description: "sendQueuedPixels")