Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report Autofill failures #893

Merged
merged 5 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public enum PrivacyFeature: String {
case autoconsent
case clickToLoad
case autofill
case autofillBreakageReporter
case ampLinks
case trackingParameters
case customUserAgent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ public struct BrokenSiteReportEntry {
/// Object Creation + 30 days
let expiryDate: Date?

public init?(report: BrokenSiteReport, currentDate: Date) {
public init?(report: BrokenSiteReport, currentDate: Date, daysToExpiry: Int) {

guard let domainIdentifier = report.siteUrl.privacySafeDomainIdentifier,
let expiryDate = Calendar.current.date(byAdding: .day, value: 30, to: currentDate) else {
let expiryDate = Calendar.current.date(byAdding: .day, value: daysToExpiry, to: currentDate) else {
return nil
}
self.identifier = domainIdentifier
Expand All @@ -50,7 +50,7 @@ public struct BrokenSiteReportEntry {

}

fileprivate extension URL {
public extension URL {

/// A string containing the first 6 chars of the sha256 hash of the URL's domain part
var privacySafeDomainIdentifier: String? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,17 @@ public class BrokenSiteReporter {
public typealias PixelHandler = (_ parameters: [String: String]) -> Void
/// Pixels are sent by the main apps not by BSK, this is the closure called by the class when a pixel need to be sent
let pixelHandler: PixelHandler
let persistencyManager: ExpiryStorage
public let persistencyManager: ExpiryStorage

public init(pixelHandler: @escaping PixelHandler, keyValueStoring: KeyValueStoringDictionaryRepresentable) {
public init(pixelHandler: @escaping PixelHandler,
keyValueStoring: KeyValueStoringDictionaryRepresentable,
storageConfiguration: ExpiryStorageConfiguration = .defaultConfig) {
self.pixelHandler = pixelHandler
self.persistencyManager = ExpiryStorage(keyValueStoring: keyValueStoring)
self.persistencyManager = ExpiryStorage(keyValueStoring: keyValueStoring, configuration: storageConfiguration)
}

/// Report the site breakage
public func report(_ report: BrokenSiteReport, reportMode: BrokenSiteReport.Mode) throws {
public func report(_ report: BrokenSiteReport, reportMode: BrokenSiteReport.Mode, daysToExpiry: Int = 30) throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added daysToExpiry here since it will dynamic for autofill reports depending on the privacy config defined value


let now = Date()
let removedCount = persistencyManager.removeExpiredItems(currentDate: now)
Expand All @@ -61,7 +63,7 @@ public class BrokenSiteReporter {
var report = report

// Create history entry
guard let entry = BrokenSiteReportEntry(report: report, currentDate: now) else {
guard let entry = BrokenSiteReportEntry(report: report, currentDate: now, daysToExpiry: daysToExpiry) else {
os_log(.error, "Failed to create a history entry for broken site report")
throw BrokenSiteReporterError.failedToGenerateHistoryEntry
}
Expand Down
37 changes: 26 additions & 11 deletions Sources/PrivacyDashboard/BrokenSiteReporting/ExpiryStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,37 @@ import Persistence

public typealias KeyValueStoringDictionaryRepresentable = KeyValueStoring & DictionaryRepresentable

public struct ExpiryStorageConfiguration {

var expiryDatesStorageKey: String
var valueExpiryDateKey: String
var valueDataKey: String

public static let defaultConfig = ExpiryStorageConfiguration(
expiryDatesStorageKey: "com.duckduckgo.UserDefaultExpiryStorage",
valueExpiryDateKey: "com.duckduckgo.UserDefaultExpiryStorage.valueExpiryDate",
valueDataKey: "com.duckduckgo.UserDefaultExpiryStorage.valueData"
)

public static let autofillConfig = ExpiryStorageConfiguration(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created Autofill specific storage keys so that broken site reports and autofill breakage reports are stored separately

expiryDatesStorageKey: "com.duckduckgo.AutofillUserDefaultExpiryStorage",
valueExpiryDateKey: "com.duckduckgo.AutofillUserDefaultExpiryStorage.valueExpiryDate",
valueDataKey: "com.duckduckgo.AutofillUserDefaultExpiryStorage.valueData"
)
}

/// A storage solution were each entry has an expiry date and a function for removing all expired entries is provided.
/// Any persistency solution implementing `KeyValueStoringDictionaryRepresentable` can be used.
public class ExpiryStorage {

enum Keys: String {
case expiryDatesStorage = "com.duckduckgo.UserDefaultExpiryStorage"
case valueExpiryDate = "com.duckduckgo.UserDefaultExpiryStorage.valueExpiryDate"
case valueData = "com.duckduckgo.UserDefaultExpiryStorage.valueData"
}

let localStorage: KeyValueStoringDictionaryRepresentable
let config: ExpiryStorageConfiguration

/// Default initialiser
/// - Parameter keyValueStoring: An object managing the persistency of the key-value pairs that implements `KeyValueStoringDictionaryRepresentable`
public init(keyValueStoring: KeyValueStoringDictionaryRepresentable) {
public init(keyValueStoring: KeyValueStoringDictionaryRepresentable, configuration: ExpiryStorageConfiguration = .defaultConfig) {
self.localStorage = keyValueStoring
self.config = configuration
}

/// Store a value and the desired expiry date (or removes the value if nil is passed as the value) for the provided key
Expand All @@ -46,11 +61,11 @@ public class ExpiryStorage {
/// - expiryDate: A date stored alongside the value, used by `removeExpiredItems(...)` for removing expired values.
public func set(value: Any?, forKey key: String, expiryDate: Date) {

let valueDic = [Keys.valueExpiryDate.rawValue: expiryDate, Keys.valueData.rawValue: value]
let valueDic = [config.valueExpiryDateKey: expiryDate, config.valueDataKey: value]
localStorage.set(valueDic, forKey: key)
}

/// - Returns: The stored value assiciated to the key, nil if not existent
/// - Returns: The stored value associated to the key, nil if not existent
public func value(forKey key: String) -> Any? {

return entry(forKey: key)?.value
Expand All @@ -59,8 +74,8 @@ public class ExpiryStorage {
/// - Returns: The tuple expiryDate+value associated to the key, nil if they don't exist
public func entry(forKey key: String) -> (expiryDate: Date, value: Any)? {
guard let valueDic = localStorage.object(forKey: key) as? [String: Any],
let expiryDate = valueDic[Keys.valueExpiryDate.rawValue] as? Date,
let value = valueDic[Keys.valueData.rawValue]
let expiryDate = valueDic[config.valueExpiryDateKey] as? Date,
let value = valueDic[config.valueDataKey]
else {
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import XCTest

final class BrokenSiteReportHistoryEntryTests: XCTestCase {

private let daysToExpiry: Int = 30

func testDates() throws {
let testDate = Date(timeIntervalSince1970: 1704795829)
let entry = BrokenSiteReportEntry(report: BrokenSiteReportMocks.report, currentDate: testDate)
let entry = BrokenSiteReportEntry(report: BrokenSiteReportMocks.report, currentDate: testDate, daysToExpiry: daysToExpiry)

XCTAssertNotNil(entry)
XCTAssertEqual("2024-01-09", entry?.lastSentDayString)
Expand All @@ -32,11 +34,11 @@ final class BrokenSiteReportHistoryEntryTests: XCTestCase {

func testUniqueIdentifier() throws {
let testDate = Date(timeIntervalSince1970: 1704795829)
let entry = BrokenSiteReportEntry(report: BrokenSiteReportMocks.report, currentDate: testDate)
let entry2 = BrokenSiteReportEntry(report: BrokenSiteReportMocks.report2, currentDate: testDate)
let entry = BrokenSiteReportEntry(report: BrokenSiteReportMocks.report, currentDate: testDate, daysToExpiry: daysToExpiry)
let entry2 = BrokenSiteReportEntry(report: BrokenSiteReportMocks.report2, currentDate: testDate, daysToExpiry: daysToExpiry)
XCTAssertNotEqual(entry?.identifier, entry2?.identifier)

let entry3 = BrokenSiteReportEntry(report: BrokenSiteReportMocks.report, currentDate: testDate)
let entry3 = BrokenSiteReportEntry(report: BrokenSiteReportMocks.report, currentDate: testDate, daysToExpiry: daysToExpiry)
XCTAssertEqual(entry?.identifier, entry3?.identifier)
}

Expand Down
Loading