Skip to content

Commit

Permalink
Filter out pixels older than 28 days.
Browse files Browse the repository at this point in the history
  • Loading branch information
samsymons committed Sep 16, 2024
1 parent 9866e5a commit 874b312
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 7 deletions.
22 changes: 21 additions & 1 deletion Core/PersistentPixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"

Expand Down
4 changes: 4 additions & 0 deletions Core/PersistentPixelStoring.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ public struct PersistentPixelMetadata: Codable, Equatable {
case .regular: return eventName
}
}

var timestamp: String? {
return additionalParameters[PixelParameters.originalPixelTimestamp]
}
}

protocol PersistentPixelStoring {
Expand Down
58 changes: 52 additions & 6 deletions DuckDuckGoTests/PersistentPixelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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() {
Expand Down Expand Up @@ -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]
)

Expand All @@ -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])
}

Expand All @@ -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]
)

Expand All @@ -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)

Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit 874b312

Please sign in to comment.