From f0fc2676d34961be3823959598ff680f63127b39 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 24 Nov 2024 17:07:31 +0100 Subject: [PATCH 01/48] Add PrivacyStats module --- .../BrowserServicesKit-Package.xcscheme | 14 ++++ Package.swift | 14 ++++ Sources/Common/Extensions/DateExtension.swift | 7 +- .../.xccurrentversion | 8 +++ .../PrivacyStats.xcdatamodel/contents | 15 ++++ Sources/PrivacyStats/PrivacyStatsEntity.swift | 71 +++++++++++++++++++ 6 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 Sources/PrivacyStats/PrivacyStats.xcdatamodeld/.xccurrentversion create mode 100644 Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents create mode 100644 Sources/PrivacyStats/PrivacyStatsEntity.swift diff --git a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme index 56a2ef845..e7803535f 100644 --- a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme +++ b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme @@ -539,6 +539,20 @@ ReferencedContainer = "container:"> + + + + + + + + _XCCurrentVersionName + PrivacyStats.xcdatamodel + + diff --git a/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents b/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents new file mode 100644 index 000000000..4dd9ac827 --- /dev/null +++ b/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/Sources/PrivacyStats/PrivacyStatsEntity.swift b/Sources/PrivacyStats/PrivacyStatsEntity.swift new file mode 100644 index 000000000..dfe693f8e --- /dev/null +++ b/Sources/PrivacyStats/PrivacyStatsEntity.swift @@ -0,0 +1,71 @@ +// +// PrivacyStatsEntity.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Common +import CoreData + +@objc(PrivacyStatsEntity) +public class PrivacyStatsEntity: NSManagedObject { + + @nonobjc public class func fetchRequest() -> NSFetchRequest { + return NSFetchRequest(entityName: "PrivacyStatsEntity") + } + + public class func entity(in context: NSManagedObjectContext) -> NSEntityDescription { + return NSEntityDescription.entity(forEntityName: "PrivacyStatsEntity", in: context)! + } + + @NSManaged public var blockedTrackersDictionary: [String: Int] + @NSManaged public var timestamp: Date + + public convenience init(context moc: NSManagedObjectContext) { + self.init(entity: PrivacyStatsEntity.entity(in: moc), insertInto: moc) + } + + public static func make(name: String, timestamp: Date = Date(), context: NSManagedObjectContext) -> PrivacyStatsEntity { + let object = PrivacyStatsEntity(context: context) + object.blockedTrackersDictionary = [:] + object.timestamp = timestamp.startOfHour + return object + } + + public override func validateForInsert() throws { + try super.validateForInsert() + try validate() + } + + public override func validateForUpdate() throws { + try super.validateForUpdate() + try validate() + } +} + +// MARK: Validation +extension PrivacyStatsEntity { + + func validate() throws { + try validateFavoritesFolder() + } + + func validateFavoritesFolder() throws { + let uuids = Set(favoriteFoldersSet.compactMap(\.uuid)) + guard uuids.isSubset(of: Constants.favoriteFoldersIDs) else { + throw Error.invalidFavoritesFolder + } + } +} From 7514bf28ff636beef8ec81850ca2ae0c966c827f Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 24 Nov 2024 21:26:38 +0100 Subject: [PATCH 02/48] Implement storing privacy stats --- Package.swift | 3 +- .../PrivacyStats/Logger+PrivacyStats.swift | 24 +++ Sources/PrivacyStats/PrivacyStats.swift | 156 ++++++++++++++++++ Sources/PrivacyStats/PrivacyStatsEntity.swift | 27 +-- Sources/PrivacyStats/PrivacyStatsUtils.swift | 39 +++++ 5 files changed, 222 insertions(+), 27 deletions(-) create mode 100644 Sources/PrivacyStats/Logger+PrivacyStats.swift create mode 100644 Sources/PrivacyStats/PrivacyStats.swift create mode 100644 Sources/PrivacyStats/PrivacyStatsUtils.swift diff --git a/Package.swift b/Package.swift index c21937179..0e3cf8b19 100644 --- a/Package.swift +++ b/Package.swift @@ -450,7 +450,8 @@ let package = Package( name: "PrivacyStats", dependencies: [ "Common", - "Persistence" + "Persistence", + "TrackerRadarKit" ], resources: [ .process("PrivacyStats.xcdatamodeld") diff --git a/Sources/PrivacyStats/Logger+PrivacyStats.swift b/Sources/PrivacyStats/Logger+PrivacyStats.swift new file mode 100644 index 000000000..e8649a6af --- /dev/null +++ b/Sources/PrivacyStats/Logger+PrivacyStats.swift @@ -0,0 +1,24 @@ +// +// Logger+PrivacyStats.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import os.log + +public extension Logger { + static var privacyStats = { Logger(subsystem: "Privacy Stats", category: "") }() +} diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift new file mode 100644 index 000000000..7864d1f7d --- /dev/null +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -0,0 +1,156 @@ +// +// PrivacyStats.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Combine +import Foundation +import os.log +import Persistence +import TrackerRadarKit +#if os(macOS) +import AppKit +#elseif os(iOS) +import UIKit +#endif + +public protocol PrivacyStatsDatabaseProviding { + func initializeDatabase() -> CoreDataDatabase +} + +public protocol PrivacyStatsCollecting { + func recordBlockedTracker(_ name: String) + func commitChanges() + var topCompanies: Set { get } + var currentStats: [String: Int] { get } +} + +public protocol TrackerDataProviding { + var trackerData: TrackerData { get } + var trackerDataUpdatesPublisher: AnyPublisher { get } +} + +public final class PrivacyStats: PrivacyStatsCollecting { + + public static let bundle = Bundle.module + + public let trackerDataProvider: TrackerDataProviding + public private(set) var topCompanies: Set = [] + public private(set) var currentStats: [String: Int] = [:] + + private var cancellables: Set = [] + private let db: CoreDataDatabase + private let context: NSManagedObjectContext + private var currentTimestamp: Date? + private var currentStatsObject: PrivacyStatsEntity? + private let currentStatsLock = NSLock() + + private var commitTimer: Timer? + + public func recordBlockedTracker(_ name: String) { + currentStatsLock.withLock { + let timestamp = Date().startOfHour + if timestamp.startOfHour != currentTimestamp { + commitChanges() + createNewStatsObject(for: timestamp) + } + + let count = currentStats[name] ?? 0 + currentStats[name] = count + 1 + + commitTimer?.invalidate() + commitTimer = Timer.scheduledTimer(withTimeInterval: .seconds(1), repeats: false, block: { [weak self] _ in + self?.commitChanges() + }) + } + } + + public func commitChanges() { + context.performAndWait { + do { + currentStatsObject?.blockedTrackersDictionary = currentStats + try context.save() + Logger.privacyStats.debug("Saved stats \(String(reflecting: self.currentTimestamp)) \(self.currentStats)") + } catch { + Logger.privacyStats.error("Save error: \(error)") + } + } + } + + // swiftlint:disable:next force_cast + public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { + self.db = databaseProvider.initializeDatabase() + self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") + + self.trackerDataProvider = trackerDataProvider + trackerDataProvider.trackerDataUpdatesPublisher + .sink { [weak self] in + self?.refreshTopCompanies() + } + .store(in: &cancellables) + refreshTopCompanies() + loadCurrentStats() + +#if os(iOS) + let notificationName = UIApplication.willTerminateNotification +#elseif os(macOS) + let notificationName = NSApplication.willTerminateNotification +#endif + + NotificationCenter.default.addObserver(self, selector: #selector(applicationWillTerminate(_:)), name: notificationName, object: nil) + } + + private func refreshTopCompanies() { + struct TrackerWithPrevalence { + let name: String + let prevalence: Double + } + + let trackers: [TrackerWithPrevalence] = trackerDataProvider.trackerData.entities.values.compactMap { entity in + guard let displayName = entity.displayName, let prevalence = entity.prevalence else { + return nil + } + return TrackerWithPrevalence(name: displayName, prevalence: prevalence) + } + + let topTrackersArray = trackers.sorted(by: { $0.prevalence > $1.prevalence }).prefix(100).map(\.name) + Logger.privacyStats.debug("top tracker companies: \(topTrackersArray)") + topCompanies = Set(topTrackersArray) + } + + private func loadCurrentStats() { + currentStatsLock.withLock { + context.performAndWait { + currentStatsObject = PrivacyStatsUtils.loadStats(in: context) + currentStats = currentStatsObject?.blockedTrackersDictionary ?? [:] + currentTimestamp = currentStatsObject?.timestamp + Logger.privacyStats.debug("Loaded stats \(String(reflecting: self.currentTimestamp)) \(self.currentStats)") + } + } + } + + private func createNewStatsObject(for timestamp: Date) { + context.performAndWait { + currentStatsObject = PrivacyStatsEntity.make(timestamp: timestamp, context: context) + } + currentStats = [:] + currentTimestamp = timestamp + } + + @objc private func applicationWillTerminate(_: Notification) { + commitChanges() + } +} diff --git a/Sources/PrivacyStats/PrivacyStatsEntity.swift b/Sources/PrivacyStats/PrivacyStatsEntity.swift index dfe693f8e..ba132fc55 100644 --- a/Sources/PrivacyStats/PrivacyStatsEntity.swift +++ b/Sources/PrivacyStats/PrivacyStatsEntity.swift @@ -37,35 +37,10 @@ public class PrivacyStatsEntity: NSManagedObject { self.init(entity: PrivacyStatsEntity.entity(in: moc), insertInto: moc) } - public static func make(name: String, timestamp: Date = Date(), context: NSManagedObjectContext) -> PrivacyStatsEntity { + public static func make(timestamp: Date = Date(), context: NSManagedObjectContext) -> PrivacyStatsEntity { let object = PrivacyStatsEntity(context: context) object.blockedTrackersDictionary = [:] object.timestamp = timestamp.startOfHour return object } - - public override func validateForInsert() throws { - try super.validateForInsert() - try validate() - } - - public override func validateForUpdate() throws { - try super.validateForUpdate() - try validate() - } -} - -// MARK: Validation -extension PrivacyStatsEntity { - - func validate() throws { - try validateFavoritesFolder() - } - - func validateFavoritesFolder() throws { - let uuids = Set(favoriteFoldersSet.compactMap(\.uuid)) - guard uuids.isSubset(of: Constants.favoriteFoldersIDs) else { - throw Error.invalidFavoritesFolder - } - } } diff --git a/Sources/PrivacyStats/PrivacyStatsUtils.swift b/Sources/PrivacyStats/PrivacyStatsUtils.swift new file mode 100644 index 000000000..2ff65e281 --- /dev/null +++ b/Sources/PrivacyStats/PrivacyStatsUtils.swift @@ -0,0 +1,39 @@ +// +// PrivacyStatsUtils.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Common +import CoreData +import Foundation + +final class PrivacyStatsUtils { + + static func loadStats(for date: Date = Date(), in context: NSManagedObjectContext) -> PrivacyStatsEntity { + let timestamp = date.startOfHour + + let request = PrivacyStatsEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K == %@", #keyPath(PrivacyStatsEntity.timestamp), timestamp as NSDate) + request.fetchLimit = 1 + request.returnsObjectsAsFaults = false + + var statsObject = ((try? context.fetch(request)) ?? []).first + if statsObject == nil { + statsObject = PrivacyStatsEntity.make(timestamp: date, context: context) + } + return statsObject! + } +} From c429cb46015b987e13f0f7b6ccb456aa47bc6b36 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 24 Nov 2024 23:24:09 +0100 Subject: [PATCH 03/48] Add API to calculate 7 day privacy stats --- Sources/Common/Extensions/DateExtension.swift | 8 +++++ Sources/PrivacyStats/PrivacyStats.swift | 35 ++++++++++++++++++- Sources/PrivacyStats/PrivacyStatsUtils.swift | 24 +++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/Sources/Common/Extensions/DateExtension.swift b/Sources/Common/Extensions/DateExtension.swift index 1f8f01a97..a0268835f 100644 --- a/Sources/Common/Extensions/DateExtension.swift +++ b/Sources/Common/Extensions/DateExtension.swift @@ -54,6 +54,10 @@ public extension Date { return Calendar.current.isDate(date1, inSameDayAs: date2) } + static func isSameHour(_ date1: Date, _ date2: Date) -> Bool { + return Calendar.current.isDate(date1, equalTo: date2, toGranularity: .hour) + } + static var startOfDayTomorrow: Date { let tomorrow = Calendar.current.date(byAdding: .day, value: 1, to: Date())! return Calendar.current.startOfDay(for: tomorrow) @@ -72,6 +76,10 @@ public extension Date { return Calendar.current.date(from: dateComponents)! } + func daysAgo(_ days: Int) -> Date { + return Calendar.current.date(byAdding: .day, value: -days, to: self)! + } + static var startOfMinuteNow: Date { let date = Calendar.current.date(bySetting: .second, value: 0, of: Date())! let start = Calendar.current.date(byAdding: .minute, value: -1, to: date)! diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 7864d1f7d..73c029ca5 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -36,6 +36,10 @@ public protocol PrivacyStatsCollecting { func commitChanges() var topCompanies: Set { get } var currentStats: [String: Int] { get } + + var currentStatsPublisher: AnyPublisher<[String: Int], Never> { get } + + func fetchPrivacyStats() -> [String: Int] } public protocol TrackerDataProviding { @@ -49,17 +53,26 @@ public final class PrivacyStats: PrivacyStatsCollecting { public let trackerDataProvider: TrackerDataProviding public private(set) var topCompanies: Set = [] - public private(set) var currentStats: [String: Int] = [:] + + @Published public private(set) var currentStats: [String: Int] = [:] // current pack + public var currentStatsPublisher: AnyPublisher<[String : Int], Never> { + $currentStats.dropFirst().eraseToAnyPublisher() + } private var cancellables: Set = [] private let db: CoreDataDatabase private let context: NSManagedObjectContext + + // current pack timestamp private var currentTimestamp: Date? private var currentStatsObject: PrivacyStatsEntity? private let currentStatsLock = NSLock() private var commitTimer: Timer? + private var cached7DayStats: [String: Int] = [:] + private var cached7DayStatsLastFetchTimestamp: Date? + public func recordBlockedTracker(_ name: String) { currentStatsLock.withLock { let timestamp = Date().startOfHour @@ -90,6 +103,26 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } + public func fetchPrivacyStats() -> [String: Int] { + let isCacheValid: Bool = { + guard let cached7DayStatsLastFetchTimestamp else { + return false + } + return Date.isSameHour(Date(), cached7DayStatsLastFetchTimestamp) + }() + if !isCacheValid { + refresh7DayCache() + } + return cached7DayStats.merging(currentStats, uniquingKeysWith: +).filter { topCompanies.contains($0.key) } + } + + private func refresh7DayCache() { + context.performAndWait { + cached7DayStats = PrivacyStatsUtils.load7DayStats(in: context) + cached7DayStatsLastFetchTimestamp = Date() + } + } + // swiftlint:disable:next force_cast public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { self.db = databaseProvider.initializeDatabase() diff --git a/Sources/PrivacyStats/PrivacyStatsUtils.swift b/Sources/PrivacyStats/PrivacyStatsUtils.swift index 2ff65e281..633751467 100644 --- a/Sources/PrivacyStats/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/PrivacyStatsUtils.swift @@ -36,4 +36,28 @@ final class PrivacyStatsUtils { } return statsObject! } + + static func load7DayStats(until date: Date = Date(), in context: NSManagedObjectContext) -> [String: Int] { + let lastTimestamp = date.startOfHour + let firstTimestamp = lastTimestamp.daysAgo(7) + + return loadStats(from: firstTimestamp, to: lastTimestamp, in: context) + } + + static func loadStats(from startDate: Date, to endDate: Date, in context: NSManagedObjectContext) -> [String: Int] { + let request = PrivacyStatsEntity.fetchRequest() + request.predicate = NSPredicate( + format: "%K >= %@ AND %K < %@", + #keyPath(PrivacyStatsEntity.timestamp), + startDate as NSDate, + #keyPath(PrivacyStatsEntity.timestamp), + endDate as NSDate + ) + request.returnsObjectsAsFaults = false + + let statsObjects = (try? context.fetch(request)) ?? [] + return statsObjects.reduce(into: [String: Int]()) { partialResult, stats in + partialResult.merge(stats.blockedTrackersDictionary, uniquingKeysWith: +) + } + } } From 8646940521a7b57bcc77057b36e02cd847dece28 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Sun, 24 Nov 2024 23:59:22 +0100 Subject: [PATCH 04/48] Add deleting outdated entries --- Sources/PrivacyStats/PrivacyStats.swift | 20 +++++++++++++++++--- Sources/PrivacyStats/PrivacyStatsUtils.swift | 11 ++++++++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 73c029ca5..7e418802b 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -55,7 +55,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { public private(set) var topCompanies: Set = [] @Published public private(set) var currentStats: [String: Int] = [:] // current pack - public var currentStatsPublisher: AnyPublisher<[String : Int], Never> { + public var currentStatsPublisher: AnyPublisher<[String: Int], Never> { $currentStats.dropFirst().eraseToAnyPublisher() } @@ -93,8 +93,8 @@ public final class PrivacyStats: PrivacyStatsCollecting { public func commitChanges() { context.performAndWait { + currentStatsObject?.blockedTrackersDictionary = currentStats do { - currentStatsObject?.blockedTrackersDictionary = currentStats try context.save() Logger.privacyStats.debug("Saved stats \(String(reflecting: self.currentTimestamp)) \(self.currentStats)") } catch { @@ -112,6 +112,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { }() if !isCacheValid { refresh7DayCache() + deleteOldEntries() } return cached7DayStats.merging(currentStats, uniquingKeysWith: +).filter { topCompanies.contains($0.key) } } @@ -123,7 +124,20 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } - // swiftlint:disable:next force_cast + private func deleteOldEntries() { + context.performAndWait { + PrivacyStatsUtils.deleteOutdatedStats(in: context) + if context.hasChanges { + do { + try context.save() + Logger.privacyStats.debug("Deleted outdated entries") + } catch { + Logger.privacyStats.error("Save error: \(error)") + } + } + } + } + public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { self.db = databaseProvider.initializeDatabase() self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") diff --git a/Sources/PrivacyStats/PrivacyStatsUtils.swift b/Sources/PrivacyStats/PrivacyStatsUtils.swift index 633751467..2171c5241 100644 --- a/Sources/PrivacyStats/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/PrivacyStatsUtils.swift @@ -47,7 +47,7 @@ final class PrivacyStatsUtils { static func loadStats(from startDate: Date, to endDate: Date, in context: NSManagedObjectContext) -> [String: Int] { let request = PrivacyStatsEntity.fetchRequest() request.predicate = NSPredicate( - format: "%K >= %@ AND %K < %@", + format: "%K > %@ AND %K < %@", #keyPath(PrivacyStatsEntity.timestamp), startDate as NSDate, #keyPath(PrivacyStatsEntity.timestamp), @@ -60,4 +60,13 @@ final class PrivacyStatsUtils { partialResult.merge(stats.blockedTrackersDictionary, uniquingKeysWith: +) } } + + static func deleteOutdatedStats(olderThan date: Date = Date(), in context: NSManagedObjectContext) { + let thisHour = date.startOfHour + let oldestValidTimestamp = thisHour.daysAgo(7) + + let request = PrivacyStatsEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K <= %@", #keyPath(PrivacyStatsEntity.timestamp), oldestValidTimestamp as NSDate) + context.deleteAll(matching: request) + } } From 92204a7630988b00482e1de19767b33392ece302 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 25 Nov 2024 00:24:50 +0100 Subject: [PATCH 05/48] Use context.perform where possible --- Sources/PrivacyStats/PrivacyStats.swift | 27 +++++++++++++------------ 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 7e418802b..d1ad24e3c 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -33,7 +33,6 @@ public protocol PrivacyStatsDatabaseProviding { public protocol PrivacyStatsCollecting { func recordBlockedTracker(_ name: String) - func commitChanges() var topCompanies: Set { get } var currentStats: [String: Int] { get } @@ -76,7 +75,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { public func recordBlockedTracker(_ name: String) { currentStatsLock.withLock { let timestamp = Date().startOfHour - if timestamp.startOfHour != currentTimestamp { + if timestamp != currentTimestamp { commitChanges() createNewStatsObject(for: timestamp) } @@ -91,7 +90,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } - public func commitChanges() { + private func commitChanges() { context.performAndWait { currentStatsObject?.blockedTrackersDictionary = currentStats do { @@ -125,11 +124,11 @@ public final class PrivacyStats: PrivacyStatsCollecting { } private func deleteOldEntries() { - context.performAndWait { - PrivacyStatsUtils.deleteOutdatedStats(in: context) - if context.hasChanges { + context.perform { + PrivacyStatsUtils.deleteOutdatedStats(in: self.context) + if self.context.hasChanges { do { - try context.save() + try self.context.save() Logger.privacyStats.debug("Deleted outdated entries") } catch { Logger.privacyStats.error("Save error: \(error)") @@ -179,11 +178,11 @@ public final class PrivacyStats: PrivacyStatsCollecting { } private func loadCurrentStats() { - currentStatsLock.withLock { - context.performAndWait { - currentStatsObject = PrivacyStatsUtils.loadStats(in: context) - currentStats = currentStatsObject?.blockedTrackersDictionary ?? [:] - currentTimestamp = currentStatsObject?.timestamp + context.perform { + self.currentStatsObject = PrivacyStatsUtils.loadStats(in: self.context) + self.currentStatsLock.withLock { + self.currentStats = self.currentStatsObject?.blockedTrackersDictionary ?? [:] + self.currentTimestamp = self.currentStatsObject?.timestamp Logger.privacyStats.debug("Loaded stats \(String(reflecting: self.currentTimestamp)) \(self.currentStats)") } } @@ -198,6 +197,8 @@ public final class PrivacyStats: PrivacyStatsCollecting { } @objc private func applicationWillTerminate(_: Notification) { - commitChanges() + currentStatsLock.withLock { + commitChanges() + } } } From 9d96447eb464976a58168990d0018740e7ae5400 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Mon, 25 Nov 2024 14:37:51 +0100 Subject: [PATCH 06/48] Rewrite PrivacyStats using structured concurrency --- Sources/PrivacyStats/PrivacyStats.swift | 254 ++++++++++++++++-------- 1 file changed, 166 insertions(+), 88 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index d1ad24e3c..07fd1b68c 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -32,13 +32,11 @@ public protocol PrivacyStatsDatabaseProviding { } public protocol PrivacyStatsCollecting { - func recordBlockedTracker(_ name: String) + func recordBlockedTracker(_ name: String) async var topCompanies: Set { get } - var currentStats: [String: Int] { get } - var currentStatsPublisher: AnyPublisher<[String: Int], Never> { get } - - func fetchPrivacyStats() -> [String: Int] + var currentStatsUpdatePublisher: AnyPublisher { get } + func fetchPrivacyStats() async -> [String: Int] } public protocol TrackerDataProviding { @@ -46,63 +44,142 @@ public protocol TrackerDataProviding { var trackerDataUpdatesPublisher: AnyPublisher { get } } +actor CurrentStats { + private(set) var timestamp: Date? + private(set) var trackers: [String: Int] = [:] + private(set) lazy var commitChangesPublisher: AnyPublisher<([String: Int], Date), Never> = commitChangesSubject.eraseToAnyPublisher() + + private let commitChangesSubject = PassthroughSubject<([String: Int], Date), Never>() + private var commitTask: Task? + + func set(_ trackers: [String: Int], for timestamp: Date) { + self.timestamp = timestamp + self.trackers = trackers + } + + func recordBlockedTracker(_ name: String) { + + let currentTimestamp = Date().startOfHour + if let timestamp, currentTimestamp != timestamp { + commitChangesSubject.send((trackers, timestamp)) + resetStats(andSet: currentTimestamp) + } + + let count = trackers[name] ?? 0 + trackers[name] = count + 1 + + commitTask?.cancel() + commitTask = Task { + do { + try await Task.sleep(nanoseconds: 1000000000) + + if let timestamp = self.timestamp { + Logger.privacyStats.debug("Storing trackers state") + commitChangesSubject.send((trackers, timestamp)) + } + } catch { + // commit task got cancelled + } + } + } + + private func resetStats(andSet newTimestamp: Date) { + timestamp = newTimestamp + trackers = [:] + } +} + public final class PrivacyStats: PrivacyStatsCollecting { public static let bundle = Bundle.module public let trackerDataProvider: TrackerDataProviding public private(set) var topCompanies: Set = [] - - @Published public private(set) var currentStats: [String: Int] = [:] // current pack - public var currentStatsPublisher: AnyPublisher<[String: Int], Never> { - $currentStats.dropFirst().eraseToAnyPublisher() - } - private var cancellables: Set = [] private let db: CoreDataDatabase private let context: NSManagedObjectContext + public let currentStatsUpdatePublisher: AnyPublisher + + private let currentStatsUpdateSubject = PassthroughSubject() + // current pack timestamp - private var currentTimestamp: Date? private var currentStatsObject: PrivacyStatsEntity? - private let currentStatsLock = NSLock() + private var currentStatsActor: CurrentStats private var commitTimer: Timer? private var cached7DayStats: [String: Int] = [:] private var cached7DayStatsLastFetchTimestamp: Date? - public func recordBlockedTracker(_ name: String) { - currentStatsLock.withLock { - let timestamp = Date().startOfHour - if timestamp != currentTimestamp { - commitChanges() - createNewStatsObject(for: timestamp) - } + public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { + self.db = databaseProvider.initializeDatabase() + self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") + self.currentStatsActor = CurrentStats() + currentStatsUpdatePublisher = currentStatsUpdateSubject.eraseToAnyPublisher() - let count = currentStats[name] ?? 0 - currentStats[name] = count + 1 + self.trackerDataProvider = trackerDataProvider + trackerDataProvider.trackerDataUpdatesPublisher + .sink { [weak self] in + self?.refreshTopCompanies() + } + .store(in: &cancellables) - commitTimer?.invalidate() - commitTimer = Timer.scheduledTimer(withTimeInterval: .seconds(1), repeats: false, block: { [weak self] _ in - self?.commitChanges() - }) + refreshTopCompanies() + Task { + await loadCurrentStats() + await currentStatsActor.commitChangesPublisher + .sink { [weak self] stats, timestamp in + Task { + await self?.commitChanges(stats, timestamp: timestamp) + } + } + .store(in: &cancellables) } + +#if os(iOS) + let notificationName = UIApplication.willTerminateNotification +#elseif os(macOS) + let notificationName = NSApplication.willTerminateNotification +#endif + + NotificationCenter.default.addObserver(self, selector: #selector(applicationWillTerminate(_:)), name: notificationName, object: nil) } - private func commitChanges() { - context.performAndWait { - currentStatsObject?.blockedTrackersDictionary = currentStats - do { - try context.save() - Logger.privacyStats.debug("Saved stats \(String(reflecting: self.currentTimestamp)) \(self.currentStats)") - } catch { - Logger.privacyStats.error("Save error: \(error)") + public func recordBlockedTracker(_ name: String) async { + await currentStatsActor.recordBlockedTracker(name) + } + + private func commitChanges(_ trackers: [String: Int], timestamp: Date) async { + await withCheckedContinuation { continuation in + context.perform { [weak self] in + guard let self else { + continuation.resume() + return + } + if let currentStatsObject, timestamp != currentStatsObject.timestamp { + self.currentStatsObject = PrivacyStatsEntity.make(timestamp: timestamp, context: context) + } + + currentStatsObject?.blockedTrackersDictionary = trackers + guard context.hasChanges else { + continuation.resume() + return + } + + do { + try context.save() + Logger.privacyStats.debug("Saved stats \(timestamp) \(trackers)") + currentStatsUpdateSubject.send() + } catch { + Logger.privacyStats.error("Save error: \(error)") + } + continuation.resume() } } } - public func fetchPrivacyStats() -> [String: Int] { + public func fetchPrivacyStats() async -> [String: Int] { let isCacheValid: Bool = { guard let cached7DayStatsLastFetchTimestamp else { return false @@ -110,53 +187,49 @@ public final class PrivacyStats: PrivacyStatsCollecting { return Date.isSameHour(Date(), cached7DayStatsLastFetchTimestamp) }() if !isCacheValid { - refresh7DayCache() - deleteOldEntries() - } - return cached7DayStats.merging(currentStats, uniquingKeysWith: +).filter { topCompanies.contains($0.key) } - } - - private func refresh7DayCache() { - context.performAndWait { - cached7DayStats = PrivacyStatsUtils.load7DayStats(in: context) - cached7DayStatsLastFetchTimestamp = Date() + await refresh7DayCache() + Task { + await deleteOldEntries() + } } + let currentStats = await currentStatsActor.trackers + return cached7DayStats.merging(currentStats, uniquingKeysWith: +) } - private func deleteOldEntries() { - context.perform { - PrivacyStatsUtils.deleteOutdatedStats(in: self.context) - if self.context.hasChanges { - do { - try self.context.save() - Logger.privacyStats.debug("Deleted outdated entries") - } catch { - Logger.privacyStats.error("Save error: \(error)") + private func refresh7DayCache() async { + await withCheckedContinuation { continuation in + context.perform { [weak self] in + guard let self else { + continuation.resume() + return } + cached7DayStats = PrivacyStatsUtils.load7DayStats(in: context) + cached7DayStatsLastFetchTimestamp = Date() + continuation.resume() } } } - public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { - self.db = databaseProvider.initializeDatabase() - self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") + private func deleteOldEntries() async { + await withCheckedContinuation { continuation in + context.perform { [weak self] in + guard let self else { + continuation.resume() + return + } - self.trackerDataProvider = trackerDataProvider - trackerDataProvider.trackerDataUpdatesPublisher - .sink { [weak self] in - self?.refreshTopCompanies() + PrivacyStatsUtils.deleteOutdatedStats(in: context) + if context.hasChanges { + do { + try context.save() + Logger.privacyStats.debug("Deleted outdated entries") + } catch { + Logger.privacyStats.error("Save error: \(error)") + } + } + continuation.resume() } - .store(in: &cancellables) - refreshTopCompanies() - loadCurrentStats() - -#if os(iOS) - let notificationName = UIApplication.willTerminateNotification -#elseif os(macOS) - let notificationName = NSApplication.willTerminateNotification -#endif - - NotificationCenter.default.addObserver(self, selector: #selector(applicationWillTerminate(_:)), name: notificationName, object: nil) + } } private func refreshTopCompanies() { @@ -177,28 +250,33 @@ public final class PrivacyStats: PrivacyStatsCollecting { topCompanies = Set(topTrackersArray) } - private func loadCurrentStats() { - context.perform { - self.currentStatsObject = PrivacyStatsUtils.loadStats(in: self.context) - self.currentStatsLock.withLock { - self.currentStats = self.currentStatsObject?.blockedTrackersDictionary ?? [:] - self.currentTimestamp = self.currentStatsObject?.timestamp - Logger.privacyStats.debug("Loaded stats \(String(reflecting: self.currentTimestamp)) \(self.currentStats)") + private func loadCurrentStats() async { + let result = await withCheckedContinuation { (continuation: CheckedContinuation<([String: Int], Date)?, Never>) in + context.perform { [weak self] in + guard let self else { + continuation.resume(returning: nil) + return + } + let privacyStatsEntity = PrivacyStatsUtils.loadStats(in: context) + currentStatsObject = privacyStatsEntity + Logger.privacyStats.debug("Loaded stats \(privacyStatsEntity.timestamp) \(privacyStatsEntity.blockedTrackersDictionary)") + continuation.resume(returning: (privacyStatsEntity.blockedTrackersDictionary, privacyStatsEntity.timestamp)) } } - } - - private func createNewStatsObject(for timestamp: Date) { - context.performAndWait { - currentStatsObject = PrivacyStatsEntity.make(timestamp: timestamp, context: context) + if let (blockedTrackersDictionary, timestamp) = result { + await currentStatsActor.set(blockedTrackersDictionary, for: timestamp) } - currentStats = [:] - currentTimestamp = timestamp } @objc private func applicationWillTerminate(_: Notification) { - currentStatsLock.withLock { - commitChanges() + let condition = RunLoop.ResumeCondition() + Task { + if let timestamp = await currentStatsActor.timestamp { + await commitChanges(await currentStatsActor.trackers, timestamp: timestamp) + } + condition.resolve() } + // Run the loop until changes are saved + RunLoop.current.run(until: condition) } } From b3267f8d360402ea73aa9e28c215dcfddf4f120b Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 26 Nov 2024 15:42:29 +0100 Subject: [PATCH 07/48] Add API to delete all privacy stats entities --- Sources/PrivacyStats/PrivacyStats.swift | 25 ++++++++++++++++++++ Sources/PrivacyStats/PrivacyStatsUtils.swift | 5 ++++ 2 files changed, 30 insertions(+) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 07fd1b68c..68e8ad828 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -37,6 +37,7 @@ public protocol PrivacyStatsCollecting { var currentStatsUpdatePublisher: AnyPublisher { get } func fetchPrivacyStats() async -> [String: Int] + func clearPrivacyStats() async } public protocol TrackerDataProviding { @@ -57,6 +58,10 @@ actor CurrentStats { self.trackers = trackers } + func resetStats() { + self.trackers = [:] + } + func recordBlockedTracker(_ name: String) { let currentTimestamp = Date().startOfHour @@ -196,6 +201,26 @@ public final class PrivacyStats: PrivacyStatsCollecting { return cached7DayStats.merging(currentStats, uniquingKeysWith: +) } + public func clearPrivacyStats() async { + await withCheckedContinuation { continuation in + context.perform { [weak self] in + guard let self else { + continuation.resume() + return + } + PrivacyStatsUtils.deleteAllStats(in: context) + do { + try context.save() + Logger.privacyStats.debug("Deleted outdated entries") + } catch { + Logger.privacyStats.error("Save error: \(error)") + } + continuation.resume() + } + } + await currentStatsActor.resetStats() + } + private func refresh7DayCache() async { await withCheckedContinuation { continuation in context.perform { [weak self] in diff --git a/Sources/PrivacyStats/PrivacyStatsUtils.swift b/Sources/PrivacyStats/PrivacyStatsUtils.swift index 2171c5241..ea4371a66 100644 --- a/Sources/PrivacyStats/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/PrivacyStatsUtils.swift @@ -19,6 +19,7 @@ import Common import CoreData import Foundation +import Persistence final class PrivacyStatsUtils { @@ -69,4 +70,8 @@ final class PrivacyStatsUtils { request.predicate = NSPredicate(format: "%K <= %@", #keyPath(PrivacyStatsEntity.timestamp), oldestValidTimestamp as NSDate) context.deleteAll(matching: request) } + + static func deleteAllStats(in context: NSManagedObjectContext) { + context.deleteAll(matching: PrivacyStatsEntity.fetchRequest()) + } } From dc6857feb0b262ae88574c8840e84dddc05a1b8e Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 26 Nov 2024 17:54:16 +0100 Subject: [PATCH 08/48] Nullify current stats object when clearing data --- Sources/PrivacyStats/PrivacyStats.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 68e8ad828..a98eee9d1 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -209,6 +209,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { return } PrivacyStatsUtils.deleteAllStats(in: context) + currentStatsObject = nil do { try context.save() Logger.privacyStats.debug("Deleted outdated entries") From a36718b900141791653a531f53b4add097b5051c Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Tue, 26 Nov 2024 18:03:32 +0100 Subject: [PATCH 09/48] Reload empty stats after clearing data --- Sources/PrivacyStats/PrivacyStats.swift | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index a98eee9d1..8e6bc789f 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -58,10 +58,6 @@ actor CurrentStats { self.trackers = trackers } - func resetStats() { - self.trackers = [:] - } - func recordBlockedTracker(_ name: String) { let currentTimestamp = Date().startOfHour @@ -209,7 +205,6 @@ public final class PrivacyStats: PrivacyStatsCollecting { return } PrivacyStatsUtils.deleteAllStats(in: context) - currentStatsObject = nil do { try context.save() Logger.privacyStats.debug("Deleted outdated entries") @@ -219,7 +214,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume() } } - await currentStatsActor.resetStats() + await loadCurrentStats() } private func refresh7DayCache() async { From 14e7955e7276d6b8e3acce9df0ccb6a17d3b8b74 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 09:31:34 +0100 Subject: [PATCH 10/48] Code renaming and refactoring --- Sources/PrivacyStats/PrivacyStats.swift | 84 +++++-------------- .../PrivacyStats.xcdatamodel/contents | 2 +- Sources/PrivacyStats/PrivacyStatsPack.swift | 69 +++++++++++++++ ...ity.swift => PrivacyStatsPackEntity.swift} | 18 ++-- Sources/PrivacyStats/PrivacyStatsUtils.swift | 20 ++--- 5 files changed, 108 insertions(+), 85 deletions(-) create mode 100644 Sources/PrivacyStats/PrivacyStatsPack.swift rename Sources/PrivacyStats/{PrivacyStatsEntity.swift => PrivacyStatsPackEntity.swift} (73%) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 8e6bc789f..6b1247c9f 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -35,7 +35,7 @@ public protocol PrivacyStatsCollecting { func recordBlockedTracker(_ name: String) async var topCompanies: Set { get } - var currentStatsUpdatePublisher: AnyPublisher { get } + var statsUpdatePublisher: AnyPublisher { get } func fetchPrivacyStats() async -> [String: Int] func clearPrivacyStats() async } @@ -45,51 +45,6 @@ public protocol TrackerDataProviding { var trackerDataUpdatesPublisher: AnyPublisher { get } } -actor CurrentStats { - private(set) var timestamp: Date? - private(set) var trackers: [String: Int] = [:] - private(set) lazy var commitChangesPublisher: AnyPublisher<([String: Int], Date), Never> = commitChangesSubject.eraseToAnyPublisher() - - private let commitChangesSubject = PassthroughSubject<([String: Int], Date), Never>() - private var commitTask: Task? - - func set(_ trackers: [String: Int], for timestamp: Date) { - self.timestamp = timestamp - self.trackers = trackers - } - - func recordBlockedTracker(_ name: String) { - - let currentTimestamp = Date().startOfHour - if let timestamp, currentTimestamp != timestamp { - commitChangesSubject.send((trackers, timestamp)) - resetStats(andSet: currentTimestamp) - } - - let count = trackers[name] ?? 0 - trackers[name] = count + 1 - - commitTask?.cancel() - commitTask = Task { - do { - try await Task.sleep(nanoseconds: 1000000000) - - if let timestamp = self.timestamp { - Logger.privacyStats.debug("Storing trackers state") - commitChangesSubject.send((trackers, timestamp)) - } - } catch { - // commit task got cancelled - } - } - } - - private func resetStats(andSet newTimestamp: Date) { - timestamp = newTimestamp - trackers = [:] - } -} - public final class PrivacyStats: PrivacyStatsCollecting { public static let bundle = Bundle.module @@ -100,13 +55,12 @@ public final class PrivacyStats: PrivacyStatsCollecting { private let db: CoreDataDatabase private let context: NSManagedObjectContext - public let currentStatsUpdatePublisher: AnyPublisher - - private let currentStatsUpdateSubject = PassthroughSubject() + public let statsUpdatePublisher: AnyPublisher + private let statsUpdateSubject = PassthroughSubject() // current pack timestamp - private var currentStatsObject: PrivacyStatsEntity? - private var currentStatsActor: CurrentStats + private var currentStatsObject: PrivacyStatsPackEntity? + private var currentStatsActor: CurrentPack private var commitTimer: Timer? @@ -116,8 +70,8 @@ public final class PrivacyStats: PrivacyStatsCollecting { public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { self.db = databaseProvider.initializeDatabase() self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") - self.currentStatsActor = CurrentStats() - currentStatsUpdatePublisher = currentStatsUpdateSubject.eraseToAnyPublisher() + self.currentStatsActor = CurrentPack() + statsUpdatePublisher = statsUpdateSubject.eraseToAnyPublisher() self.trackerDataProvider = trackerDataProvider trackerDataProvider.trackerDataUpdatesPublisher @@ -130,9 +84,9 @@ public final class PrivacyStats: PrivacyStatsCollecting { Task { await loadCurrentStats() await currentStatsActor.commitChangesPublisher - .sink { [weak self] stats, timestamp in + .sink { [weak self] pack in Task { - await self?.commitChanges(stats, timestamp: timestamp) + await self?.commitChanges(pack) } } .store(in: &cancellables) @@ -151,18 +105,18 @@ public final class PrivacyStats: PrivacyStatsCollecting { await currentStatsActor.recordBlockedTracker(name) } - private func commitChanges(_ trackers: [String: Int], timestamp: Date) async { + private func commitChanges(_ pack: PrivacyStatsPack) async { await withCheckedContinuation { continuation in context.perform { [weak self] in guard let self else { continuation.resume() return } - if let currentStatsObject, timestamp != currentStatsObject.timestamp { - self.currentStatsObject = PrivacyStatsEntity.make(timestamp: timestamp, context: context) + if let currentStatsObject, pack.timestamp != currentStatsObject.timestamp { + self.currentStatsObject = PrivacyStatsPackEntity.make(timestamp: pack.timestamp, context: context) } - currentStatsObject?.blockedTrackersDictionary = trackers + currentStatsObject?.blockedTrackersDictionary = pack.trackers guard context.hasChanges else { continuation.resume() return @@ -170,8 +124,8 @@ public final class PrivacyStats: PrivacyStatsCollecting { do { try context.save() - Logger.privacyStats.debug("Saved stats \(timestamp) \(trackers)") - currentStatsUpdateSubject.send() + Logger.privacyStats.debug("Saved stats \(pack.timestamp) \(pack.trackers)") + statsUpdateSubject.send() } catch { Logger.privacyStats.error("Save error: \(error)") } @@ -193,8 +147,8 @@ public final class PrivacyStats: PrivacyStatsCollecting { await deleteOldEntries() } } - let currentStats = await currentStatsActor.trackers - return cached7DayStats.merging(currentStats, uniquingKeysWith: +) + let currentPack = await currentStatsActor.pack + return cached7DayStats.merging(currentPack?.trackers ?? [:], uniquingKeysWith: +) } public func clearPrivacyStats() async { @@ -292,8 +246,8 @@ public final class PrivacyStats: PrivacyStatsCollecting { @objc private func applicationWillTerminate(_: Notification) { let condition = RunLoop.ResumeCondition() Task { - if let timestamp = await currentStatsActor.timestamp { - await commitChanges(await currentStatsActor.trackers, timestamp: timestamp) + if let pack = await currentStatsActor.pack { + await commitChanges(pack) } condition.resolve() } diff --git a/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents b/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents index 4dd9ac827..35beec224 100644 --- a/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents +++ b/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents @@ -1,6 +1,6 @@ - + diff --git a/Sources/PrivacyStats/PrivacyStatsPack.swift b/Sources/PrivacyStats/PrivacyStatsPack.swift new file mode 100644 index 000000000..847192281 --- /dev/null +++ b/Sources/PrivacyStats/PrivacyStatsPack.swift @@ -0,0 +1,69 @@ +// +// PrivacyStatsPack.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Combine +import Foundation +import os.log + +struct PrivacyStatsPack { + let timestamp: Date + var trackers: [String: Int] +} + +actor CurrentPack { + private(set) var pack: PrivacyStatsPack? + private(set) lazy var commitChangesPublisher: AnyPublisher = commitChangesSubject.eraseToAnyPublisher() + + private let commitChangesSubject = PassthroughSubject() + private var commitTask: Task? + + func set(_ trackers: [String: Int], for timestamp: Date) { + pack = .init(timestamp: timestamp, trackers: trackers) + } + + func recordBlockedTracker(_ name: String) { + + let currentTimestamp = Date().startOfHour + if let pack, currentTimestamp != pack.timestamp { + commitChangesSubject.send(pack) + resetStats(andSet: currentTimestamp) + } + + let count = pack?.trackers[name] ?? 0 + pack?.trackers[name] = count + 1 + + commitTask?.cancel() + commitTask = Task { + do { + try await Task.sleep(nanoseconds: 1000000000) + + if let pack { + Logger.privacyStats.debug("Storing trackers state") + commitChangesSubject.send(pack) + } + } catch { + // commit task got cancelled + } + } + } + + private func resetStats(andSet newTimestamp: Date) { + pack = PrivacyStatsPack(timestamp: newTimestamp, trackers: [:]) + } +} diff --git a/Sources/PrivacyStats/PrivacyStatsEntity.swift b/Sources/PrivacyStats/PrivacyStatsPackEntity.swift similarity index 73% rename from Sources/PrivacyStats/PrivacyStatsEntity.swift rename to Sources/PrivacyStats/PrivacyStatsPackEntity.swift index ba132fc55..9636f3f9a 100644 --- a/Sources/PrivacyStats/PrivacyStatsEntity.swift +++ b/Sources/PrivacyStats/PrivacyStatsPackEntity.swift @@ -1,5 +1,5 @@ // -// PrivacyStatsEntity.swift +// PrivacyStatsPackEntity.swift // // Copyright © 2024 DuckDuckGo. All rights reserved. // @@ -19,26 +19,26 @@ import Common import CoreData -@objc(PrivacyStatsEntity) -public class PrivacyStatsEntity: NSManagedObject { +@objc(PrivacyStatsPackEntity) +public class PrivacyStatsPackEntity: NSManagedObject { - @nonobjc public class func fetchRequest() -> NSFetchRequest { - return NSFetchRequest(entityName: "PrivacyStatsEntity") + @nonobjc public class func fetchRequest() -> NSFetchRequest { + return NSFetchRequest(entityName: "PrivacyStatsPackEntity") } public class func entity(in context: NSManagedObjectContext) -> NSEntityDescription { - return NSEntityDescription.entity(forEntityName: "PrivacyStatsEntity", in: context)! + return NSEntityDescription.entity(forEntityName: "PrivacyStatsPackEntity", in: context)! } @NSManaged public var blockedTrackersDictionary: [String: Int] @NSManaged public var timestamp: Date public convenience init(context moc: NSManagedObjectContext) { - self.init(entity: PrivacyStatsEntity.entity(in: moc), insertInto: moc) + self.init(entity: PrivacyStatsPackEntity.entity(in: moc), insertInto: moc) } - public static func make(timestamp: Date = Date(), context: NSManagedObjectContext) -> PrivacyStatsEntity { - let object = PrivacyStatsEntity(context: context) + public static func make(timestamp: Date = Date(), context: NSManagedObjectContext) -> PrivacyStatsPackEntity { + let object = PrivacyStatsPackEntity(context: context) object.blockedTrackersDictionary = [:] object.timestamp = timestamp.startOfHour return object diff --git a/Sources/PrivacyStats/PrivacyStatsUtils.swift b/Sources/PrivacyStats/PrivacyStatsUtils.swift index ea4371a66..82941b390 100644 --- a/Sources/PrivacyStats/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/PrivacyStatsUtils.swift @@ -23,17 +23,17 @@ import Persistence final class PrivacyStatsUtils { - static func loadStats(for date: Date = Date(), in context: NSManagedObjectContext) -> PrivacyStatsEntity { + static func loadStats(for date: Date = Date(), in context: NSManagedObjectContext) -> PrivacyStatsPackEntity { let timestamp = date.startOfHour - let request = PrivacyStatsEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K == %@", #keyPath(PrivacyStatsEntity.timestamp), timestamp as NSDate) + let request = PrivacyStatsPackEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K == %@", #keyPath(PrivacyStatsPackEntity.timestamp), timestamp as NSDate) request.fetchLimit = 1 request.returnsObjectsAsFaults = false var statsObject = ((try? context.fetch(request)) ?? []).first if statsObject == nil { - statsObject = PrivacyStatsEntity.make(timestamp: date, context: context) + statsObject = PrivacyStatsPackEntity.make(timestamp: date, context: context) } return statsObject! } @@ -46,12 +46,12 @@ final class PrivacyStatsUtils { } static func loadStats(from startDate: Date, to endDate: Date, in context: NSManagedObjectContext) -> [String: Int] { - let request = PrivacyStatsEntity.fetchRequest() + let request = PrivacyStatsPackEntity.fetchRequest() request.predicate = NSPredicate( format: "%K > %@ AND %K < %@", - #keyPath(PrivacyStatsEntity.timestamp), + #keyPath(PrivacyStatsPackEntity.timestamp), startDate as NSDate, - #keyPath(PrivacyStatsEntity.timestamp), + #keyPath(PrivacyStatsPackEntity.timestamp), endDate as NSDate ) request.returnsObjectsAsFaults = false @@ -66,12 +66,12 @@ final class PrivacyStatsUtils { let thisHour = date.startOfHour let oldestValidTimestamp = thisHour.daysAgo(7) - let request = PrivacyStatsEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K <= %@", #keyPath(PrivacyStatsEntity.timestamp), oldestValidTimestamp as NSDate) + let request = PrivacyStatsPackEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K <= %@", #keyPath(PrivacyStatsPackEntity.timestamp), oldestValidTimestamp as NSDate) context.deleteAll(matching: request) } static func deleteAllStats(in context: NSManagedObjectContext) { - context.deleteAll(matching: PrivacyStatsEntity.fetchRequest()) + context.deleteAll(matching: PrivacyStatsPackEntity.fetchRequest()) } } From f5cb69f7cab37a5fc2ca80d2611fb949ea7f95eb Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 11:59:08 +0100 Subject: [PATCH 11/48] Further refactoring --- Sources/PrivacyStats/PrivacyStats.swift | 36 ++++++++-------- Sources/PrivacyStats/PrivacyStatsUtils.swift | 8 +++- .../{ => internal}/Logger+PrivacyStats.swift | 2 +- .../{ => internal}/PrivacyStatsPack.swift | 42 +++++++++++++------ 4 files changed, 56 insertions(+), 32 deletions(-) rename Sources/PrivacyStats/{ => internal}/Logger+PrivacyStats.swift (96%) rename Sources/PrivacyStats/{ => internal}/PrivacyStatsPack.swift (63%) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 6b1247c9f..ca1af19f7 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -82,7 +82,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { refreshTopCompanies() Task { - await loadCurrentStats() + await loadCurrentPack() await currentStatsActor.commitChangesPublisher .sink { [weak self] pack in Task { @@ -112,8 +112,12 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume() return } - if let currentStatsObject, pack.timestamp != currentStatsObject.timestamp { - self.currentStatsObject = PrivacyStatsPackEntity.make(timestamp: pack.timestamp, context: context) + if let currentStatsObject { + if pack.timestamp != currentStatsObject.timestamp { + self.currentStatsObject = PrivacyStatsPackEntity.make(timestamp: pack.timestamp, context: context) + } + } else { + currentStatsObject = PrivacyStatsPackEntity.make(timestamp: pack.timestamp, context: context) } currentStatsObject?.blockedTrackersDictionary = pack.trackers @@ -148,7 +152,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } let currentPack = await currentStatsActor.pack - return cached7DayStats.merging(currentPack?.trackers ?? [:], uniquingKeysWith: +) + return cached7DayStats.merging(currentPack.trackers, uniquingKeysWith: +) } public func clearPrivacyStats() async { @@ -168,7 +172,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume() } } - await loadCurrentStats() + await loadCurrentPack() } private func refresh7DayCache() async { @@ -193,7 +197,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { return } - PrivacyStatsUtils.deleteOutdatedStats(in: context) + PrivacyStatsUtils.deleteOutdatedPacks(in: context) if context.hasChanges { do { try context.save() @@ -225,30 +229,28 @@ public final class PrivacyStats: PrivacyStatsCollecting { topCompanies = Set(topTrackersArray) } - private func loadCurrentStats() async { - let result = await withCheckedContinuation { (continuation: CheckedContinuation<([String: Int], Date)?, Never>) in + private func loadCurrentPack() async { + let currentPack = await withCheckedContinuation { (continuation: CheckedContinuation) in context.perform { [weak self] in guard let self else { continuation.resume(returning: nil) return } - let privacyStatsEntity = PrivacyStatsUtils.loadStats(in: context) - currentStatsObject = privacyStatsEntity - Logger.privacyStats.debug("Loaded stats \(privacyStatsEntity.timestamp) \(privacyStatsEntity.blockedTrackersDictionary)") - continuation.resume(returning: (privacyStatsEntity.blockedTrackersDictionary, privacyStatsEntity.timestamp)) + let privacyStatsPackEntity = PrivacyStatsUtils.loadCurrentPack(in: context) + currentStatsObject = privacyStatsPackEntity + Logger.privacyStats.debug("Loaded stats \(privacyStatsPackEntity.timestamp) \(privacyStatsPackEntity.blockedTrackersDictionary)") + continuation.resume(returning: .init(privacyStatsPackEntity)) } } - if let (blockedTrackersDictionary, timestamp) = result { - await currentStatsActor.set(blockedTrackersDictionary, for: timestamp) + if let currentPack { + await currentStatsActor.updatePack(currentPack) } } @objc private func applicationWillTerminate(_: Notification) { let condition = RunLoop.ResumeCondition() Task { - if let pack = await currentStatsActor.pack { - await commitChanges(pack) - } + await commitChanges(currentStatsActor.pack) condition.resolve() } // Run the loop until changes are saved diff --git a/Sources/PrivacyStats/PrivacyStatsUtils.swift b/Sources/PrivacyStats/PrivacyStatsUtils.swift index 82941b390..555883d03 100644 --- a/Sources/PrivacyStats/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/PrivacyStatsUtils.swift @@ -23,7 +23,11 @@ import Persistence final class PrivacyStatsUtils { - static func loadStats(for date: Date = Date(), in context: NSManagedObjectContext) -> PrivacyStatsPackEntity { + static func loadCurrentPack(in context: NSManagedObjectContext) -> PrivacyStatsPackEntity { + loadPack(for: Date(), in: context) + } + + static func loadPack(for date: Date, in context: NSManagedObjectContext) -> PrivacyStatsPackEntity { let timestamp = date.startOfHour let request = PrivacyStatsPackEntity.fetchRequest() @@ -62,7 +66,7 @@ final class PrivacyStatsUtils { } } - static func deleteOutdatedStats(olderThan date: Date = Date(), in context: NSManagedObjectContext) { + static func deleteOutdatedPacks(olderThan date: Date = Date(), in context: NSManagedObjectContext) { let thisHour = date.startOfHour let oldestValidTimestamp = thisHour.daysAgo(7) diff --git a/Sources/PrivacyStats/Logger+PrivacyStats.swift b/Sources/PrivacyStats/internal/Logger+PrivacyStats.swift similarity index 96% rename from Sources/PrivacyStats/Logger+PrivacyStats.swift rename to Sources/PrivacyStats/internal/Logger+PrivacyStats.swift index e8649a6af..dc4b6215b 100644 --- a/Sources/PrivacyStats/Logger+PrivacyStats.swift +++ b/Sources/PrivacyStats/internal/Logger+PrivacyStats.swift @@ -19,6 +19,6 @@ import Foundation import os.log -public extension Logger { +extension Logger { static var privacyStats = { Logger(subsystem: "Privacy Stats", category: "") }() } diff --git a/Sources/PrivacyStats/PrivacyStatsPack.swift b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift similarity index 63% rename from Sources/PrivacyStats/PrivacyStatsPack.swift rename to Sources/PrivacyStats/internal/PrivacyStatsPack.swift index 847192281..d208f9ea5 100644 --- a/Sources/PrivacyStats/PrivacyStatsPack.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift @@ -21,42 +21,54 @@ import Combine import Foundation import os.log -struct PrivacyStatsPack { +struct PrivacyStatsPack: Sendable { let timestamp: Date var trackers: [String: Int] + + init(timestamp: Date, trackers: [String: Int]) { + self.timestamp = timestamp + self.trackers = trackers + } + + init(_ packEntity: PrivacyStatsPackEntity) { + timestamp = packEntity.timestamp + trackers = packEntity.blockedTrackersDictionary + } } actor CurrentPack { - private(set) var pack: PrivacyStatsPack? + var pack: PrivacyStatsPack private(set) lazy var commitChangesPublisher: AnyPublisher = commitChangesSubject.eraseToAnyPublisher() private let commitChangesSubject = PassthroughSubject() private var commitTask: Task? - func set(_ trackers: [String: Int], for timestamp: Date) { - pack = .init(timestamp: timestamp, trackers: trackers) + init() { + pack = .init(timestamp: .currentTimestamp, trackers: [:]) + } + + func updatePack(_ pack: PrivacyStatsPack) { + self.pack = pack } func recordBlockedTracker(_ name: String) { - let currentTimestamp = Date().startOfHour - if let pack, currentTimestamp != pack.timestamp { + let currentTimestamp = Date.currentTimestamp + if currentTimestamp != pack.timestamp { commitChangesSubject.send(pack) resetStats(andSet: currentTimestamp) } - let count = pack?.trackers[name] ?? 0 - pack?.trackers[name] = count + 1 + let count = pack.trackers[name] ?? 0 + pack.trackers[name] = count + 1 commitTask?.cancel() commitTask = Task { do { try await Task.sleep(nanoseconds: 1000000000) - if let pack { - Logger.privacyStats.debug("Storing trackers state") - commitChangesSubject.send(pack) - } + Logger.privacyStats.debug("Storing trackers state") + commitChangesSubject.send(pack) } catch { // commit task got cancelled } @@ -67,3 +79,9 @@ actor CurrentPack { pack = PrivacyStatsPack(timestamp: newTimestamp, trackers: [:]) } } + +private extension Date { + static var currentTimestamp: Date { + Date().startOfHour + } +} From a8be3d495c4d2d32587001b881cf17b61e5722aa Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 14:30:13 +0100 Subject: [PATCH 12/48] Switch to storing blocked tracker stats by company --- .../PrivacyStats/DailyBlockedTrackers.swift | 47 ++++++++++ Sources/PrivacyStats/PrivacyStats.swift | 32 +++---- .../PrivacyStats.xcdatamodel/contents | 9 +- .../PrivacyStats/PrivacyStatsPackEntity.swift | 46 ---------- Sources/PrivacyStats/PrivacyStatsUtils.swift | 88 +++++++++++++------ .../internal/PrivacyStatsPack.swift | 9 +- 6 files changed, 132 insertions(+), 99 deletions(-) create mode 100644 Sources/PrivacyStats/DailyBlockedTrackers.swift delete mode 100644 Sources/PrivacyStats/PrivacyStatsPackEntity.swift diff --git a/Sources/PrivacyStats/DailyBlockedTrackers.swift b/Sources/PrivacyStats/DailyBlockedTrackers.swift new file mode 100644 index 000000000..d4b209d62 --- /dev/null +++ b/Sources/PrivacyStats/DailyBlockedTrackers.swift @@ -0,0 +1,47 @@ +// +// DailyBlockedTrackers.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Common +import CoreData + +@objc(DailyBlockedTrackersEntity) +final class DailyBlockedTrackersEntity: NSManagedObject { + + @nonobjc class func fetchRequest() -> NSFetchRequest { + return NSFetchRequest(entityName: "DailyBlockedTrackersEntity") + } + + class func entity(in context: NSManagedObjectContext) -> NSEntityDescription { + return NSEntityDescription.entity(forEntityName: "DailyBlockedTrackersEntity", in: context)! + } + + @NSManaged var companyName: String + @NSManaged var count: Int64 + @NSManaged var timestamp: Date + + convenience init(context moc: NSManagedObjectContext) { + self.init(entity: DailyBlockedTrackersEntity.entity(in: moc), insertInto: moc) + } + + static func make(timestamp: Date = Date(), companyName: String, context: NSManagedObjectContext) -> DailyBlockedTrackersEntity { + let object = DailyBlockedTrackersEntity(context: context) + object.timestamp = timestamp.startOfHour + object.companyName = companyName + return object + } +} diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index ca1af19f7..cbb2b3fbe 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -36,7 +36,7 @@ public protocol PrivacyStatsCollecting { var topCompanies: Set { get } var statsUpdatePublisher: AnyPublisher { get } - func fetchPrivacyStats() async -> [String: Int] + func fetchPrivacyStats() async -> [String: Int64] func clearPrivacyStats() async } @@ -58,13 +58,11 @@ public final class PrivacyStats: PrivacyStatsCollecting { public let statsUpdatePublisher: AnyPublisher private let statsUpdateSubject = PassthroughSubject() - // current pack timestamp - private var currentStatsObject: PrivacyStatsPackEntity? private var currentStatsActor: CurrentPack private var commitTimer: Timer? - private var cached7DayStats: [String: Int] = [:] + private var cached7DayStats: [String: Int64] = [:] private var cached7DayStatsLastFetchTimestamp: Date? public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { @@ -82,7 +80,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { refreshTopCompanies() Task { - await loadCurrentPack() + await loadCurrentPacks() await currentStatsActor.commitChangesPublisher .sink { [weak self] pack in Task { @@ -112,15 +110,14 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume() return } - if let currentStatsObject { - if pack.timestamp != currentStatsObject.timestamp { - self.currentStatsObject = PrivacyStatsPackEntity.make(timestamp: pack.timestamp, context: context) + + let statsObjects = PrivacyStatsUtils.fetchOrInsertCurrentPacks(for: Set(pack.trackers.keys), in: context) + statsObjects.forEach { stats in + if let count = pack.trackers[stats.companyName] { + stats.count += count } - } else { - currentStatsObject = PrivacyStatsPackEntity.make(timestamp: pack.timestamp, context: context) } - currentStatsObject?.blockedTrackersDictionary = pack.trackers guard context.hasChanges else { continuation.resume() return @@ -138,7 +135,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } - public func fetchPrivacyStats() async -> [String: Int] { + public func fetchPrivacyStats() async -> [String: Int64] { let isCacheValid: Bool = { guard let cached7DayStatsLastFetchTimestamp else { return false @@ -172,7 +169,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume() } } - await loadCurrentPack() + await loadCurrentPacks() } private func refresh7DayCache() async { @@ -229,17 +226,16 @@ public final class PrivacyStats: PrivacyStatsCollecting { topCompanies = Set(topTrackersArray) } - private func loadCurrentPack() async { + private func loadCurrentPacks() async { let currentPack = await withCheckedContinuation { (continuation: CheckedContinuation) in context.perform { [weak self] in guard let self else { continuation.resume(returning: nil) return } - let privacyStatsPackEntity = PrivacyStatsUtils.loadCurrentPack(in: context) - currentStatsObject = privacyStatsPackEntity - Logger.privacyStats.debug("Loaded stats \(privacyStatsPackEntity.timestamp) \(privacyStatsPackEntity.blockedTrackersDictionary)") - continuation.resume(returning: .init(privacyStatsPackEntity)) + let currentPack = PrivacyStatsUtils.fetchCurrentPackStats(in: context) + Logger.privacyStats.debug("Loaded stats \(currentPack.timestamp) \(currentPack.trackers)") + continuation.resume(returning: currentPack) } } if let currentPack { diff --git a/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents b/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents index 35beec224..39798857a 100644 --- a/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents +++ b/Sources/PrivacyStats/PrivacyStats.xcdatamodeld/PrivacyStats.xcdatamodel/contents @@ -1,14 +1,17 @@ - - + + + - + + + diff --git a/Sources/PrivacyStats/PrivacyStatsPackEntity.swift b/Sources/PrivacyStats/PrivacyStatsPackEntity.swift deleted file mode 100644 index 9636f3f9a..000000000 --- a/Sources/PrivacyStats/PrivacyStatsPackEntity.swift +++ /dev/null @@ -1,46 +0,0 @@ -// -// PrivacyStatsPackEntity.swift -// -// Copyright © 2024 DuckDuckGo. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -import Common -import CoreData - -@objc(PrivacyStatsPackEntity) -public class PrivacyStatsPackEntity: NSManagedObject { - - @nonobjc public class func fetchRequest() -> NSFetchRequest { - return NSFetchRequest(entityName: "PrivacyStatsPackEntity") - } - - public class func entity(in context: NSManagedObjectContext) -> NSEntityDescription { - return NSEntityDescription.entity(forEntityName: "PrivacyStatsPackEntity", in: context)! - } - - @NSManaged public var blockedTrackersDictionary: [String: Int] - @NSManaged public var timestamp: Date - - public convenience init(context moc: NSManagedObjectContext) { - self.init(entity: PrivacyStatsPackEntity.entity(in: moc), insertInto: moc) - } - - public static func make(timestamp: Date = Date(), context: NSManagedObjectContext) -> PrivacyStatsPackEntity { - let object = PrivacyStatsPackEntity(context: context) - object.blockedTrackersDictionary = [:] - object.timestamp = timestamp.startOfHour - return object - } -} diff --git a/Sources/PrivacyStats/PrivacyStatsUtils.swift b/Sources/PrivacyStats/PrivacyStatsUtils.swift index 555883d03..3b481ef3d 100644 --- a/Sources/PrivacyStats/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/PrivacyStatsUtils.swift @@ -23,59 +23,97 @@ import Persistence final class PrivacyStatsUtils { - static func loadCurrentPack(in context: NSManagedObjectContext) -> PrivacyStatsPackEntity { - loadPack(for: Date(), in: context) + static func fetchCurrentPackStats(in context: NSManagedObjectContext) -> PrivacyStatsPack { + let timestamp = Date().startOfHour + let request = DailyBlockedTrackersEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K == %@", #keyPath(DailyBlockedTrackersEntity.timestamp), timestamp as NSDate) + request.returnsObjectsAsFaults = false + + let statsObjects = (try? context.fetch(request)) ?? [] + + var pack = PrivacyStatsPack(timestamp: timestamp, trackers: [:]) + statsObjects.forEach { object in + pack.trackers[object.companyName] = object.count + } + + return pack + } + + static func fetchOrInsertCurrentPacks(for companyNames: Set, in context: NSManagedObjectContext) -> [DailyBlockedTrackersEntity] { + fetchOrInsertPacks(for: Date(), companyNames: companyNames, in: context) } - static func loadPack(for date: Date, in context: NSManagedObjectContext) -> PrivacyStatsPackEntity { + static func fetchOrInsertPacks(for date: Date, companyNames: Set, in context: NSManagedObjectContext) -> [DailyBlockedTrackersEntity] { let timestamp = date.startOfHour - let request = PrivacyStatsPackEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K == %@", #keyPath(PrivacyStatsPackEntity.timestamp), timestamp as NSDate) - request.fetchLimit = 1 + let request = DailyBlockedTrackersEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K == %@ AND %K in %@", + #keyPath(DailyBlockedTrackersEntity.timestamp), timestamp as NSDate, + #keyPath(DailyBlockedTrackersEntity.companyName), companyNames) request.returnsObjectsAsFaults = false - var statsObject = ((try? context.fetch(request)) ?? []).first - if statsObject == nil { - statsObject = PrivacyStatsPackEntity.make(timestamp: date, context: context) + var statsObjects = (try? context.fetch(request)) ?? [] + let missingCompanyNames = companyNames.subtracting(statsObjects.map(\.companyName)) + + for companyName in missingCompanyNames { + statsObjects.append(DailyBlockedTrackersEntity.make(timestamp: date, companyName: companyName, context: context)) } - return statsObject! + return statsObjects } - static func load7DayStats(until date: Date = Date(), in context: NSManagedObjectContext) -> [String: Int] { + static func load7DayStats(until date: Date = Date(), in context: NSManagedObjectContext) -> [String: Int64] { let lastTimestamp = date.startOfHour let firstTimestamp = lastTimestamp.daysAgo(7) - return loadStats(from: firstTimestamp, to: lastTimestamp, in: context) + return loadStats2(from: firstTimestamp, to: lastTimestamp, in: context) } - static func loadStats(from startDate: Date, to endDate: Date, in context: NSManagedObjectContext) -> [String: Int] { - let request = PrivacyStatsPackEntity.fetchRequest() + static func loadStats2(from startDate: Date, to endDate: Date, in context: NSManagedObjectContext) -> [String: Int64] { + let request = NSFetchRequest(entityName: "DailyBlockedTrackersEntity") + + // Predicate to filter by date range request.predicate = NSPredicate( format: "%K > %@ AND %K < %@", - #keyPath(PrivacyStatsPackEntity.timestamp), - startDate as NSDate, - #keyPath(PrivacyStatsPackEntity.timestamp), - endDate as NSDate + #keyPath(DailyBlockedTrackersEntity.timestamp), startDate as NSDate, + #keyPath(DailyBlockedTrackersEntity.timestamp), endDate as NSDate ) - request.returnsObjectsAsFaults = false - let statsObjects = (try? context.fetch(request)) ?? [] - return statsObjects.reduce(into: [String: Int]()) { partialResult, stats in - partialResult.merge(stats.blockedTrackersDictionary, uniquingKeysWith: +) + // Expression description for the sum of count + let countExpression = NSExpression(forKeyPath: #keyPath(DailyBlockedTrackersEntity.count)) + let sumExpression = NSExpression(forFunction: "sum:", arguments: [countExpression]) + + let sumExpressionDescription = NSExpressionDescription() + sumExpressionDescription.name = "totalCount" + sumExpressionDescription.expression = sumExpression + sumExpressionDescription.expressionResultType = .integer64AttributeType + + // Configure the fetch request for aggregation + request.propertiesToGroupBy = [#keyPath(DailyBlockedTrackersEntity.companyName)] + request.propertiesToFetch = [#keyPath(DailyBlockedTrackersEntity.companyName), sumExpressionDescription] + request.resultType = .dictionaryResultType + + let results = ((try? context.fetch(request)) as? [[String: Any]]) ?? [] + + let groupedResults = results.reduce(into: [String: Int64]()) { partialResult, result in + if let companyName = result[#keyPath(DailyBlockedTrackersEntity.companyName)] as? String, + let totalCount = result["totalCount"] as? Int64 { + partialResult[companyName] = totalCount + } } + + return groupedResults } static func deleteOutdatedPacks(olderThan date: Date = Date(), in context: NSManagedObjectContext) { let thisHour = date.startOfHour let oldestValidTimestamp = thisHour.daysAgo(7) - let request = PrivacyStatsPackEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K <= %@", #keyPath(PrivacyStatsPackEntity.timestamp), oldestValidTimestamp as NSDate) + let request = DailyBlockedTrackersEntity.fetchRequest() + request.predicate = NSPredicate(format: "%K <= %@", #keyPath(DailyBlockedTrackersEntity.timestamp), oldestValidTimestamp as NSDate) context.deleteAll(matching: request) } static func deleteAllStats(in context: NSManagedObjectContext) { - context.deleteAll(matching: PrivacyStatsPackEntity.fetchRequest()) + context.deleteAll(matching: DailyBlockedTrackersEntity.fetchRequest()) } } diff --git a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift index d208f9ea5..b0de37080 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift @@ -23,17 +23,12 @@ import os.log struct PrivacyStatsPack: Sendable { let timestamp: Date - var trackers: [String: Int] + var trackers: [String: Int64] - init(timestamp: Date, trackers: [String: Int]) { + init(timestamp: Date, trackers: [String: Int64] = [:]) { self.timestamp = timestamp self.trackers = trackers } - - init(_ packEntity: PrivacyStatsPackEntity) { - timestamp = packEntity.timestamp - trackers = packEntity.blockedTrackersDictionary - } } actor CurrentPack { From 7a7cd5b2232e774e1739ae553a37c54404a3e62a Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 15:44:38 +0100 Subject: [PATCH 13/48] Remove in-memory cache --- .../{internal => }/Logger+PrivacyStats.swift | 2 +- Sources/PrivacyStats/PrivacyStats.swift | 39 +++++-------------- .../{ => internal}/DailyBlockedTrackers.swift | 0 .../{ => internal}/PrivacyStatsUtils.swift | 12 ++---- 4 files changed, 14 insertions(+), 39 deletions(-) rename Sources/PrivacyStats/{internal => }/Logger+PrivacyStats.swift (96%) rename Sources/PrivacyStats/{ => internal}/DailyBlockedTrackers.swift (100%) rename Sources/PrivacyStats/{ => internal}/PrivacyStatsUtils.swift (90%) diff --git a/Sources/PrivacyStats/internal/Logger+PrivacyStats.swift b/Sources/PrivacyStats/Logger+PrivacyStats.swift similarity index 96% rename from Sources/PrivacyStats/internal/Logger+PrivacyStats.swift rename to Sources/PrivacyStats/Logger+PrivacyStats.swift index dc4b6215b..e8649a6af 100644 --- a/Sources/PrivacyStats/internal/Logger+PrivacyStats.swift +++ b/Sources/PrivacyStats/Logger+PrivacyStats.swift @@ -19,6 +19,6 @@ import Foundation import os.log -extension Logger { +public extension Logger { static var privacyStats = { Logger(subsystem: "Privacy Stats", category: "") }() } diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index cbb2b3fbe..36457acc3 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -62,9 +62,6 @@ public final class PrivacyStats: PrivacyStatsCollecting { private var commitTimer: Timer? - private var cached7DayStats: [String: Int64] = [:] - private var cached7DayStatsLastFetchTimestamp: Date? - public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { self.db = databaseProvider.initializeDatabase() self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") @@ -114,7 +111,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { let statsObjects = PrivacyStatsUtils.fetchOrInsertCurrentPacks(for: Set(pack.trackers.keys), in: context) statsObjects.forEach { stats in if let count = pack.trackers[stats.companyName] { - stats.count += count + stats.count = count } } @@ -136,20 +133,16 @@ public final class PrivacyStats: PrivacyStatsCollecting { } public func fetchPrivacyStats() async -> [String: Int64] { - let isCacheValid: Bool = { - guard let cached7DayStatsLastFetchTimestamp else { - return false - } - return Date.isSameHour(Date(), cached7DayStatsLastFetchTimestamp) - }() - if !isCacheValid { - await refresh7DayCache() - Task { - await deleteOldEntries() + return await withCheckedContinuation { continuation in + context.perform { [weak self] in + guard let self else { + continuation.resume(returning: [:]) + return + } + let stats = PrivacyStatsUtils.load7DayStats(in: context) + continuation.resume(returning: stats) } } - let currentPack = await currentStatsActor.pack - return cached7DayStats.merging(currentPack.trackers, uniquingKeysWith: +) } public func clearPrivacyStats() async { @@ -172,20 +165,6 @@ public final class PrivacyStats: PrivacyStatsCollecting { await loadCurrentPacks() } - private func refresh7DayCache() async { - await withCheckedContinuation { continuation in - context.perform { [weak self] in - guard let self else { - continuation.resume() - return - } - cached7DayStats = PrivacyStatsUtils.load7DayStats(in: context) - cached7DayStatsLastFetchTimestamp = Date() - continuation.resume() - } - } - } - private func deleteOldEntries() async { await withCheckedContinuation { continuation in context.perform { [weak self] in diff --git a/Sources/PrivacyStats/DailyBlockedTrackers.swift b/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift similarity index 100% rename from Sources/PrivacyStats/DailyBlockedTrackers.swift rename to Sources/PrivacyStats/internal/DailyBlockedTrackers.swift diff --git a/Sources/PrivacyStats/PrivacyStatsUtils.swift b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift similarity index 90% rename from Sources/PrivacyStats/PrivacyStatsUtils.swift rename to Sources/PrivacyStats/internal/PrivacyStatsUtils.swift index 3b481ef3d..5ca4548e5 100644 --- a/Sources/PrivacyStats/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift @@ -63,20 +63,16 @@ final class PrivacyStatsUtils { static func load7DayStats(until date: Date = Date(), in context: NSManagedObjectContext) -> [String: Int64] { let lastTimestamp = date.startOfHour - let firstTimestamp = lastTimestamp.daysAgo(7) + let firstTimestamp = lastTimestamp.daysAgo(6) - return loadStats2(from: firstTimestamp, to: lastTimestamp, in: context) + return loadStats(since: firstTimestamp, in: context) } - static func loadStats2(from startDate: Date, to endDate: Date, in context: NSManagedObjectContext) -> [String: Int64] { + static func loadStats(since date: Date, in context: NSManagedObjectContext) -> [String: Int64] { let request = NSFetchRequest(entityName: "DailyBlockedTrackersEntity") // Predicate to filter by date range - request.predicate = NSPredicate( - format: "%K > %@ AND %K < %@", - #keyPath(DailyBlockedTrackersEntity.timestamp), startDate as NSDate, - #keyPath(DailyBlockedTrackersEntity.timestamp), endDate as NSDate - ) + request.predicate = NSPredicate(format: "%K >= %@", #keyPath(DailyBlockedTrackersEntity.timestamp), date as NSDate) // Expression description for the sum of count let countExpression = NSExpression(forKeyPath: #keyPath(DailyBlockedTrackersEntity.count)) From 8e4d16032ed23a3aa206d39c88dd6c41766f4150 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 16:34:38 +0100 Subject: [PATCH 14/48] Clean up the code --- .../internal/DailyBlockedTrackers.swift | 2 +- .../internal/Date+PrivacyStats.swift | 31 +++++++++++++++++++ .../internal/PrivacyStatsPack.swift | 10 ++---- .../internal/PrivacyStatsUtils.swift | 23 +++++--------- 4 files changed, 41 insertions(+), 25 deletions(-) create mode 100644 Sources/PrivacyStats/internal/Date+PrivacyStats.swift diff --git a/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift b/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift index d4b209d62..6a388915d 100644 --- a/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift +++ b/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift @@ -40,7 +40,7 @@ final class DailyBlockedTrackersEntity: NSManagedObject { static func make(timestamp: Date = Date(), companyName: String, context: NSManagedObjectContext) -> DailyBlockedTrackersEntity { let object = DailyBlockedTrackersEntity(context: context) - object.timestamp = timestamp.startOfHour + object.timestamp = timestamp.privacyStatsPackTimestamp object.companyName = companyName return object } diff --git a/Sources/PrivacyStats/internal/Date+PrivacyStats.swift b/Sources/PrivacyStats/internal/Date+PrivacyStats.swift new file mode 100644 index 000000000..6b848a60f --- /dev/null +++ b/Sources/PrivacyStats/internal/Date+PrivacyStats.swift @@ -0,0 +1,31 @@ +// +// Date+PrivacyStats.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Common +import Foundation + +extension Date { + + var privacyStatsPackTimestamp: Date { + startOfHour + } + + var privacyStatsOldestPackTimestamp: Date { + privacyStatsPackTimestamp.daysAgo(6) + } +} diff --git a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift index b0de37080..bfe0b4baf 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift @@ -39,7 +39,7 @@ actor CurrentPack { private var commitTask: Task? init() { - pack = .init(timestamp: .currentTimestamp, trackers: [:]) + pack = .init(timestamp: Date().privacyStatsPackTimestamp, trackers: [:]) } func updatePack(_ pack: PrivacyStatsPack) { @@ -48,7 +48,7 @@ actor CurrentPack { func recordBlockedTracker(_ name: String) { - let currentTimestamp = Date.currentTimestamp + let currentTimestamp = Date().privacyStatsPackTimestamp if currentTimestamp != pack.timestamp { commitChangesSubject.send(pack) resetStats(andSet: currentTimestamp) @@ -74,9 +74,3 @@ actor CurrentPack { pack = PrivacyStatsPack(timestamp: newTimestamp, trackers: [:]) } } - -private extension Date { - static var currentTimestamp: Date { - Date().startOfHour - } -} diff --git a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift index 5ca4548e5..7745354a6 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift @@ -24,7 +24,7 @@ import Persistence final class PrivacyStatsUtils { static func fetchCurrentPackStats(in context: NSManagedObjectContext) -> PrivacyStatsPack { - let timestamp = Date().startOfHour + let timestamp = Date().privacyStatsPackTimestamp let request = DailyBlockedTrackersEntity.fetchRequest() request.predicate = NSPredicate(format: "%K == %@", #keyPath(DailyBlockedTrackersEntity.timestamp), timestamp as NSDate) request.returnsObjectsAsFaults = false @@ -40,11 +40,7 @@ final class PrivacyStatsUtils { } static func fetchOrInsertCurrentPacks(for companyNames: Set, in context: NSManagedObjectContext) -> [DailyBlockedTrackersEntity] { - fetchOrInsertPacks(for: Date(), companyNames: companyNames, in: context) - } - - static func fetchOrInsertPacks(for date: Date, companyNames: Set, in context: NSManagedObjectContext) -> [DailyBlockedTrackersEntity] { - let timestamp = date.startOfHour + let timestamp = Date().privacyStatsPackTimestamp let request = DailyBlockedTrackersEntity.fetchRequest() request.predicate = NSPredicate(format: "%K == %@ AND %K in %@", @@ -56,23 +52,18 @@ final class PrivacyStatsUtils { let missingCompanyNames = companyNames.subtracting(statsObjects.map(\.companyName)) for companyName in missingCompanyNames { - statsObjects.append(DailyBlockedTrackersEntity.make(timestamp: date, companyName: companyName, context: context)) + statsObjects.append(DailyBlockedTrackersEntity.make(timestamp: timestamp, companyName: companyName, context: context)) } return statsObjects } - static func load7DayStats(until date: Date = Date(), in context: NSManagedObjectContext) -> [String: Int64] { - let lastTimestamp = date.startOfHour - let firstTimestamp = lastTimestamp.daysAgo(6) - - return loadStats(since: firstTimestamp, in: context) - } + static func load7DayStats(in context: NSManagedObjectContext) -> [String: Int64] { + let startDate = Date().privacyStatsOldestPackTimestamp - static func loadStats(since date: Date, in context: NSManagedObjectContext) -> [String: Int64] { let request = NSFetchRequest(entityName: "DailyBlockedTrackersEntity") // Predicate to filter by date range - request.predicate = NSPredicate(format: "%K >= %@", #keyPath(DailyBlockedTrackersEntity.timestamp), date as NSDate) + request.predicate = NSPredicate(format: "%K >= %@", #keyPath(DailyBlockedTrackersEntity.timestamp), startDate as NSDate) // Expression description for the sum of count let countExpression = NSExpression(forKeyPath: #keyPath(DailyBlockedTrackersEntity.count)) @@ -101,7 +92,7 @@ final class PrivacyStatsUtils { } static func deleteOutdatedPacks(olderThan date: Date = Date(), in context: NSManagedObjectContext) { - let thisHour = date.startOfHour + let thisHour = date.privacyStatsPackTimestamp let oldestValidTimestamp = thisHour.daysAgo(7) let request = DailyBlockedTrackersEntity.fetchRequest() From 0006a084034f7c54b31f6cc7b5cc6813bedf613b Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 16:40:22 +0100 Subject: [PATCH 15/48] Rename --- Sources/PrivacyStats/PrivacyStats.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 36457acc3..2fb210e7a 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -77,7 +77,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { refreshTopCompanies() Task { - await loadCurrentPacks() + await loadCurrentPack() await currentStatsActor.commitChangesPublisher .sink { [weak self] pack in Task { @@ -162,7 +162,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume() } } - await loadCurrentPacks() + await loadCurrentPack() } private func deleteOldEntries() async { @@ -205,7 +205,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { topCompanies = Set(topTrackersArray) } - private func loadCurrentPacks() async { + private func loadCurrentPack() async { let currentPack = await withCheckedContinuation { (continuation: CheckedContinuation) in context.perform { [weak self] in guard let self else { From 58932bb1ecfbb345a86c9ec04eaf9c8bfa9fadeb Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 17:40:04 +0100 Subject: [PATCH 16/48] More renaming --- Sources/PrivacyStats/PrivacyStats.swift | 28 ++++++++++++------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 2fb210e7a..ebdf76471 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -51,24 +51,22 @@ public final class PrivacyStats: PrivacyStatsCollecting { public let trackerDataProvider: TrackerDataProviding public private(set) var topCompanies: Set = [] - private var cancellables: Set = [] + public let statsUpdatePublisher: AnyPublisher + private let db: CoreDataDatabase private let context: NSManagedObjectContext - - public let statsUpdatePublisher: AnyPublisher private let statsUpdateSubject = PassthroughSubject() - - private var currentStatsActor: CurrentPack - - private var commitTimer: Timer? + private var currentPack: CurrentPack + private var cancellables: Set = [] public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { + self.trackerDataProvider = trackerDataProvider self.db = databaseProvider.initializeDatabase() self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") - self.currentStatsActor = CurrentPack() + + currentPack = CurrentPack() statsUpdatePublisher = statsUpdateSubject.eraseToAnyPublisher() - self.trackerDataProvider = trackerDataProvider trackerDataProvider.trackerDataUpdatesPublisher .sink { [weak self] in self?.refreshTopCompanies() @@ -78,7 +76,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { refreshTopCompanies() Task { await loadCurrentPack() - await currentStatsActor.commitChangesPublisher + await currentPack.commitChangesPublisher .sink { [weak self] pack in Task { await self?.commitChanges(pack) @@ -97,7 +95,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { } public func recordBlockedTracker(_ name: String) async { - await currentStatsActor.recordBlockedTracker(name) + await currentPack.recordBlockedTracker(name) } private func commitChanges(_ pack: PrivacyStatsPack) async { @@ -206,7 +204,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { } private func loadCurrentPack() async { - let currentPack = await withCheckedContinuation { (continuation: CheckedContinuation) in + let pack = await withCheckedContinuation { (continuation: CheckedContinuation) in context.perform { [weak self] in guard let self else { continuation.resume(returning: nil) @@ -217,15 +215,15 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume(returning: currentPack) } } - if let currentPack { - await currentStatsActor.updatePack(currentPack) + if let pack { + await currentPack.updatePack(pack) } } @objc private func applicationWillTerminate(_: Notification) { let condition = RunLoop.ResumeCondition() Task { - await commitChanges(currentStatsActor.pack) + await commitChanges(currentPack.pack) condition.resolve() } // Run the loop until changes are saved From c48e48bcc88121734111620f0098224a6258cef4 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 17:42:47 +0100 Subject: [PATCH 17/48] Fix Swiftlint violations --- Sources/PrivacyStats/internal/PrivacyStatsPack.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift index bfe0b4baf..9c810df38 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift @@ -1,6 +1,5 @@ // // PrivacyStatsPack.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. // From 213e873a7766511e36236a12133928fbcf723530 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 17:48:18 +0100 Subject: [PATCH 18/48] Move code around --- Sources/PrivacyStats/PrivacyStats.swift | 60 +++++++++++++------------ 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index ebdf76471..cc5bf64ed 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -98,69 +98,71 @@ public final class PrivacyStats: PrivacyStatsCollecting { await currentPack.recordBlockedTracker(name) } - private func commitChanges(_ pack: PrivacyStatsPack) async { - await withCheckedContinuation { continuation in + public func fetchPrivacyStats() async -> [String: Int64] { + return await withCheckedContinuation { continuation in context.perform { [weak self] in guard let self else { - continuation.resume() + continuation.resume(returning: [:]) return } + let stats = PrivacyStatsUtils.load7DayStats(in: context) + continuation.resume(returning: stats) + } + } + } - let statsObjects = PrivacyStatsUtils.fetchOrInsertCurrentPacks(for: Set(pack.trackers.keys), in: context) - statsObjects.forEach { stats in - if let count = pack.trackers[stats.companyName] { - stats.count = count - } - } - - guard context.hasChanges else { + public func clearPrivacyStats() async { + await withCheckedContinuation { continuation in + context.perform { [weak self] in + guard let self else { continuation.resume() return } - + PrivacyStatsUtils.deleteAllStats(in: context) do { try context.save() - Logger.privacyStats.debug("Saved stats \(pack.timestamp) \(pack.trackers)") - statsUpdateSubject.send() + Logger.privacyStats.debug("Deleted outdated entries") } catch { Logger.privacyStats.error("Save error: \(error)") } continuation.resume() } } + await loadCurrentPack() } - public func fetchPrivacyStats() async -> [String: Int64] { - return await withCheckedContinuation { continuation in + // MARK: - Private + + private func commitChanges(_ pack: PrivacyStatsPack) async { + await withCheckedContinuation { continuation in context.perform { [weak self] in guard let self else { - continuation.resume(returning: [:]) + continuation.resume() return } - let stats = PrivacyStatsUtils.load7DayStats(in: context) - continuation.resume(returning: stats) - } - } - } - public func clearPrivacyStats() async { - await withCheckedContinuation { continuation in - context.perform { [weak self] in - guard let self else { + let statsObjects = PrivacyStatsUtils.fetchOrInsertCurrentPacks(for: Set(pack.trackers.keys), in: context) + statsObjects.forEach { stats in + if let count = pack.trackers[stats.companyName] { + stats.count = count + } + } + + guard context.hasChanges else { continuation.resume() return } - PrivacyStatsUtils.deleteAllStats(in: context) + do { try context.save() - Logger.privacyStats.debug("Deleted outdated entries") + Logger.privacyStats.debug("Saved stats \(pack.timestamp) \(pack.trackers)") + statsUpdateSubject.send() } catch { Logger.privacyStats.error("Save error: \(error)") } continuation.resume() } } - await loadCurrentPack() } private func deleteOldEntries() async { From 3c501dc840103e514e43a13b524e6e3c4494467a Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 17:54:34 +0100 Subject: [PATCH 19/48] Mark commitChangesSubject/Publisher nonisolated --- Sources/PrivacyStats/PrivacyStats.swift | 16 +++++++++------- .../PrivacyStats/internal/PrivacyStatsPack.swift | 4 ++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index cc5bf64ed..2ff9909c8 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -73,16 +73,18 @@ public final class PrivacyStats: PrivacyStatsCollecting { } .store(in: &cancellables) + currentPack.commitChangesPublisher + .sink { [weak self] pack in + Task { + await self?.commitChanges(pack) + } + } + .store(in: &cancellables) + refreshTopCompanies() + Task { await loadCurrentPack() - await currentPack.commitChangesPublisher - .sink { [weak self] pack in - Task { - await self?.commitChanges(pack) - } - } - .store(in: &cancellables) } #if os(iOS) diff --git a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift index 9c810df38..360e47761 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift @@ -32,9 +32,9 @@ struct PrivacyStatsPack: Sendable { actor CurrentPack { var pack: PrivacyStatsPack - private(set) lazy var commitChangesPublisher: AnyPublisher = commitChangesSubject.eraseToAnyPublisher() + nonisolated private(set) lazy var commitChangesPublisher: AnyPublisher = commitChangesSubject.eraseToAnyPublisher() - private let commitChangesSubject = PassthroughSubject() + nonisolated private let commitChangesSubject = PassthroughSubject() private var commitTask: Task? init() { From 9e32661421e018fb0f4ce0821a62ebd0e30a1b7e Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 17:59:11 +0100 Subject: [PATCH 20/48] Add PrivacyStatsTests module --- .../BrowserServicesKit-Package.xcscheme | 10 ++++++++ Package.swift | 7 ++++++ .../PrivacyStatsTests/PrivacyStatsTests.swift | 24 +++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 Tests/PrivacyStatsTests/PrivacyStatsTests.swift diff --git a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme index e7803535f..b465ab3b9 100644 --- a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme +++ b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme @@ -832,6 +832,16 @@ ReferencedContainer = "container:"> + + + + Date: Wed, 27 Nov 2024 18:00:20 +0100 Subject: [PATCH 21/48] Update C-S-S to 6.40.0 --- Package.resolved | 4 ++-- Package.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Package.resolved b/Package.resolved index 4461ccf24..7a4755e08 100644 --- a/Package.resolved +++ b/Package.resolved @@ -14,8 +14,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/content-scope-scripts", "state" : { - "revision" : "dfef00ef77f5181d1d8a4f7cc88f7b7c0514dd34", - "version" : "6.39.0" + "revision" : "e51efbca45d2178c43dd3b0fe4d18d19b34c54e3", + "version" : "6.40.0" } }, { diff --git a/Package.swift b/Package.swift index e4cbb86e7..4cd60a919 100644 --- a/Package.swift +++ b/Package.swift @@ -54,7 +54,7 @@ let package = Package( .package(url: "https://github.com/duckduckgo/TrackerRadarKit", exact: "3.0.0"), .package(url: "https://github.com/duckduckgo/sync_crypto", exact: "0.3.0"), .package(url: "https://github.com/gumob/PunycodeSwift.git", exact: "3.0.0"), - .package(url: "https://github.com/duckduckgo/content-scope-scripts", exact: "6.39.0"), + .package(url: "https://github.com/duckduckgo/content-scope-scripts", exact: "6.40.0"), .package(url: "https://github.com/duckduckgo/privacy-dashboard", exact: "7.2.0"), .package(url: "https://github.com/httpswift/swifter.git", exact: "1.5.0"), .package(url: "https://github.com/duckduckgo/bloom_cpp.git", exact: "3.0.0"), From e1d67965fd1467a90c71d37bac82ba0e6b844d64 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 27 Nov 2024 23:04:24 +0100 Subject: [PATCH 22/48] Move topCompanies to the client, make stats initializer sync and add some tests --- Sources/PrivacyStats/PrivacyStats.swift | 67 +++------ .../internal/DailyBlockedTrackers.swift | 9 +- .../internal/PrivacyStatsPack.swift | 10 +- .../PrivacyStatsTests/PrivacyStatsTests.swift | 133 +++++++++++++++++- 4 files changed, 165 insertions(+), 54 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 2ff9909c8..3832f9a79 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -33,47 +33,32 @@ public protocol PrivacyStatsDatabaseProviding { public protocol PrivacyStatsCollecting { func recordBlockedTracker(_ name: String) async - var topCompanies: Set { get } var statsUpdatePublisher: AnyPublisher { get } func fetchPrivacyStats() async -> [String: Int64] func clearPrivacyStats() async } -public protocol TrackerDataProviding { - var trackerData: TrackerData { get } - var trackerDataUpdatesPublisher: AnyPublisher { get } -} - public final class PrivacyStats: PrivacyStatsCollecting { public static let bundle = Bundle.module - public let trackerDataProvider: TrackerDataProviding - public private(set) var topCompanies: Set = [] public let statsUpdatePublisher: AnyPublisher private let db: CoreDataDatabase private let context: NSManagedObjectContext + private var currentPack: CurrentPack? private let statsUpdateSubject = PassthroughSubject() - private var currentPack: CurrentPack private var cancellables: Set = [] - public init(databaseProvider: PrivacyStatsDatabaseProviding, trackerDataProvider: TrackerDataProviding) { - self.trackerDataProvider = trackerDataProvider + public init(databaseProvider: PrivacyStatsDatabaseProviding) { self.db = databaseProvider.initializeDatabase() self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") - currentPack = CurrentPack() statsUpdatePublisher = statsUpdateSubject.eraseToAnyPublisher() + currentPack = .init(pack: initializeCurrentPack()) - trackerDataProvider.trackerDataUpdatesPublisher - .sink { [weak self] in - self?.refreshTopCompanies() - } - .store(in: &cancellables) - - currentPack.commitChangesPublisher + currentPack?.commitChangesPublisher .sink { [weak self] pack in Task { await self?.commitChanges(pack) @@ -81,23 +66,17 @@ public final class PrivacyStats: PrivacyStatsCollecting { } .store(in: &cancellables) - refreshTopCompanies() - - Task { - await loadCurrentPack() - } - #if os(iOS) let notificationName = UIApplication.willTerminateNotification #elseif os(macOS) let notificationName = NSApplication.willTerminateNotification #endif - NotificationCenter.default.addObserver(self, selector: #selector(applicationWillTerminate(_:)), name: notificationName, object: nil) + } public func recordBlockedTracker(_ name: String) async { - await currentPack.recordBlockedTracker(name) + await currentPack?.recordBlockedTracker(name) } public func fetchPrivacyStats() async -> [String: Int64] { @@ -189,24 +168,6 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } - private func refreshTopCompanies() { - struct TrackerWithPrevalence { - let name: String - let prevalence: Double - } - - let trackers: [TrackerWithPrevalence] = trackerDataProvider.trackerData.entities.values.compactMap { entity in - guard let displayName = entity.displayName, let prevalence = entity.prevalence else { - return nil - } - return TrackerWithPrevalence(name: displayName, prevalence: prevalence) - } - - let topTrackersArray = trackers.sorted(by: { $0.prevalence > $1.prevalence }).prefix(100).map(\.name) - Logger.privacyStats.debug("top tracker companies: \(topTrackersArray)") - topCompanies = Set(topTrackersArray) - } - private func loadCurrentPack() async { let pack = await withCheckedContinuation { (continuation: CheckedContinuation) in context.perform { [weak self] in @@ -220,14 +181,26 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } if let pack { - await currentPack.updatePack(pack) + await currentPack?.updatePack(pack) } } + private func initializeCurrentPack() -> PrivacyStatsPack { + var pack: PrivacyStatsPack? + context.performAndWait { + let currentPack = PrivacyStatsUtils.fetchCurrentPackStats(in: context) + Logger.privacyStats.debug("Loaded stats \(currentPack.timestamp) \(currentPack.trackers)") + pack = currentPack + } + return pack ?? PrivacyStatsPack(timestamp: Date().privacyStatsPackTimestamp) + } + @objc private func applicationWillTerminate(_: Notification) { let condition = RunLoop.ResumeCondition() Task { - await commitChanges(currentPack.pack) + if let pack = await currentPack?.pack { + await commitChanges(pack) + } condition.resolve() } // Run the loop until changes are saved diff --git a/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift b/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift index 6a388915d..46da05018 100644 --- a/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift +++ b/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift @@ -34,14 +34,19 @@ final class DailyBlockedTrackersEntity: NSManagedObject { @NSManaged var count: Int64 @NSManaged var timestamp: Date - convenience init(context moc: NSManagedObjectContext) { + private override init(entity: NSEntityDescription, insertInto context: NSManagedObjectContext?) { + super.init(entity: entity, insertInto: context) + } + + private convenience init(context moc: NSManagedObjectContext) { self.init(entity: DailyBlockedTrackersEntity.entity(in: moc), insertInto: moc) } - static func make(timestamp: Date = Date(), companyName: String, context: NSManagedObjectContext) -> DailyBlockedTrackersEntity { + static func make(timestamp: Date = Date(), companyName: String, count: Int64 = 0, context: NSManagedObjectContext) -> DailyBlockedTrackersEntity { let object = DailyBlockedTrackersEntity(context: context) object.timestamp = timestamp.privacyStatsPackTimestamp object.companyName = companyName + object.count = count return object } } diff --git a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift index 360e47761..e09d58798 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift @@ -32,13 +32,15 @@ struct PrivacyStatsPack: Sendable { actor CurrentPack { var pack: PrivacyStatsPack - nonisolated private(set) lazy var commitChangesPublisher: AnyPublisher = commitChangesSubject.eraseToAnyPublisher() + nonisolated private(set) lazy var commitChangesPublisher: AnyPublisher = commitChangesSubject.eraseToAnyPublisher() nonisolated private let commitChangesSubject = PassthroughSubject() + private var commitTask: Task? - init() { - pack = .init(timestamp: Date().privacyStatsPackTimestamp, trackers: [:]) + init(pack: PrivacyStatsPack) { + self.pack = pack +// pack = .init(timestamp: Date().privacyStatsPackTimestamp, trackers: [:]) } func updatePack(_ pack: PrivacyStatsPack) { @@ -59,7 +61,7 @@ actor CurrentPack { commitTask?.cancel() commitTask = Task { do { - try await Task.sleep(nanoseconds: 1000000000) + try await Task.sleep(nanoseconds: 1_000_000_000) Logger.privacyStats.debug("Storing trackers state") commitChangesSubject.send(pack) diff --git a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift index 914fe352b..ff332bc80 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift @@ -16,9 +16,140 @@ // limitations under the License. // +import Combine +import Persistence +import TrackerRadarKit import XCTest @testable import PrivacyStats -final class PrivacyStatsTests: XCTest { +final class TestPrivacyStatsDatabaseProvider: PrivacyStatsDatabaseProviding { + let databaseName: String + var database: CoreDataDatabase! + var location: URL! + init(databaseName: String) { + self.databaseName = databaseName + } + + func initializeDatabase() -> CoreDataDatabase? { + location = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + + let bundle = PrivacyStats.bundle + guard let model = CoreDataDatabase.loadModel(from: bundle, named: "PrivacyStats") else { + XCTFail("Failed to load model") + return nil + } + database = CoreDataDatabase(name: databaseName, containerLocation: location, model: model) + database.loadStore() + return database + } + + func tearDownDatabase() { + try? database.tearDown(deleteStores: true) + database = nil + try? FileManager.default.removeItem(at: location) + } +} + +final class MockTrackerDataProvider: TrackerDataProviding { + var trackerData: TrackerData = .init(trackers: [:], entities: [:], domains: [:], cnames: [:]) + + lazy var trackerDataUpdatesPublisher: AnyPublisher = trackerDataUpdatesSubject.eraseToAnyPublisher() + var trackerDataUpdatesSubject = PassthroughSubject() + +} + +final class PrivacyStatsTests: XCTestCase { + var databaseProvider: TestPrivacyStatsDatabaseProvider! + var trackerDataProvider: MockTrackerDataProvider! + var privacyStats: PrivacyStats! + + override func setUp() async throws { + databaseProvider = TestPrivacyStatsDatabaseProvider(databaseName: type(of: self).description()) + trackerDataProvider = MockTrackerDataProvider() + privacyStats = PrivacyStats(databaseProvider: databaseProvider, trackerDataProvider: trackerDataProvider) + } + + // MARK: - fetchPrivacyStats + + func testThatPrivacyStatsAreFetched() async throws { + let stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, [:]) + } + + func testThatFetchPrivacyStatsReturnsAllCompanies() async throws { + try addObjects { context in + [ + DailyBlockedTrackersEntity.make(companyName: "A", count: 10, context: context), + DailyBlockedTrackersEntity.make(companyName: "B", count: 5, context: context), + DailyBlockedTrackersEntity.make(companyName: "C", count: 13, context: context), + DailyBlockedTrackersEntity.make(companyName: "D", count: 42, context: context) + ] + } + + let stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 10, "B": 5, "C": 13, "D": 42]) + } + + func testThatFetchPrivacyStatsReturnsSumOfCompanyEntriesForPast7Days() async throws { + try addObjects { context in + let date = Date() + + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "A", count: 1, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(1), companyName: "A", count: 2, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(2), companyName: "A", count: 3, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(3), companyName: "A", count: 4, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(4), companyName: "A", count: 5, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(5), companyName: "A", count: 6, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(6), companyName: "A", count: 7, context: context) + ] + } + + let stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 28]) + } + + func testThatFetchPrivacyStatsDiscardsEntriesOlderThan7Days() async throws { + try addObjects { context in + let date = Date() + + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "A", count: 1, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(1), companyName: "A", count: 2, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(7), companyName: "A", count: 10, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(10), companyName: "A", count: 10, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(20), companyName: "A", count: 10, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(500), companyName: "A", count: 10, context: context), + ] + } + + let stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 3]) + } + + // MARK: - recordBlockedTracker + + func testRecordBlockedTrackerCausesDatabaseSave() async throws { + await privacyStats.recordBlockedTracker("A") + + try await Task.sleep(nanoseconds: 1_100_000_000) + + let stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 1]) + } + + // MARK: - Helpers + + func addObjects(_ objects: (NSManagedObjectContext) -> [DailyBlockedTrackersEntity], file: StaticString = #file, line: UInt = #line) throws { + let context = databaseProvider.database.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + _ = objects(context) + do { + try context.save() + } catch { + XCTFail("save failed: \(error)", file: file, line: line) + } + } + } } From 3ac66a5e2cd9020ab7678c0d0d71a5533b5d0f34 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 09:26:34 +0100 Subject: [PATCH 23/48] Add unit tests for PrivacyStats --- Sources/PrivacyStats/PrivacyStats.swift | 4 +- .../internal/PrivacyStatsPack.swift | 1 - .../internal/PrivacyStatsUtils.swift | 2 +- .../PrivacyStatsTests/PrivacyStatsTests.swift | 113 ++++++++++++++---- 4 files changed, 96 insertions(+), 24 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 3832f9a79..17fbf3ec9 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -175,7 +175,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume(returning: nil) return } - let currentPack = PrivacyStatsUtils.fetchCurrentPackStats(in: context) + let currentPack = PrivacyStatsUtils.fetchCurrentStatsPack(in: context) Logger.privacyStats.debug("Loaded stats \(currentPack.timestamp) \(currentPack.trackers)") continuation.resume(returning: currentPack) } @@ -188,7 +188,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { private func initializeCurrentPack() -> PrivacyStatsPack { var pack: PrivacyStatsPack? context.performAndWait { - let currentPack = PrivacyStatsUtils.fetchCurrentPackStats(in: context) + let currentPack = PrivacyStatsUtils.fetchCurrentStatsPack(in: context) Logger.privacyStats.debug("Loaded stats \(currentPack.timestamp) \(currentPack.trackers)") pack = currentPack } diff --git a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift index e09d58798..daed51340 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift @@ -40,7 +40,6 @@ actor CurrentPack { init(pack: PrivacyStatsPack) { self.pack = pack -// pack = .init(timestamp: Date().privacyStatsPackTimestamp, trackers: [:]) } func updatePack(_ pack: PrivacyStatsPack) { diff --git a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift index 7745354a6..4f289dcc5 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift @@ -23,7 +23,7 @@ import Persistence final class PrivacyStatsUtils { - static func fetchCurrentPackStats(in context: NSManagedObjectContext) -> PrivacyStatsPack { + static func fetchCurrentStatsPack(in context: NSManagedObjectContext) -> PrivacyStatsPack { let timestamp = Date().privacyStatsPackTimestamp let request = DailyBlockedTrackersEntity.fetchRequest() request.predicate = NSPredicate(format: "%K == %@", #keyPath(DailyBlockedTrackersEntity.timestamp), timestamp as NSDate) diff --git a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift index ff332bc80..94a90644f 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift @@ -31,14 +31,9 @@ final class TestPrivacyStatsDatabaseProvider: PrivacyStatsDatabaseProviding { self.databaseName = databaseName } - func initializeDatabase() -> CoreDataDatabase? { + func initializeDatabase() -> CoreDataDatabase { location = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) - - let bundle = PrivacyStats.bundle - guard let model = CoreDataDatabase.loadModel(from: bundle, named: "PrivacyStats") else { - XCTFail("Failed to load model") - return nil - } + let model = CoreDataDatabase.loadModel(from: PrivacyStats.bundle, named: "PrivacyStats")! database = CoreDataDatabase(name: databaseName, containerLocation: location, model: model) database.loadStore() return database @@ -51,23 +46,13 @@ final class TestPrivacyStatsDatabaseProvider: PrivacyStatsDatabaseProviding { } } -final class MockTrackerDataProvider: TrackerDataProviding { - var trackerData: TrackerData = .init(trackers: [:], entities: [:], domains: [:], cnames: [:]) - - lazy var trackerDataUpdatesPublisher: AnyPublisher = trackerDataUpdatesSubject.eraseToAnyPublisher() - var trackerDataUpdatesSubject = PassthroughSubject() - -} - final class PrivacyStatsTests: XCTestCase { var databaseProvider: TestPrivacyStatsDatabaseProvider! - var trackerDataProvider: MockTrackerDataProvider! var privacyStats: PrivacyStats! override func setUp() async throws { databaseProvider = TestPrivacyStatsDatabaseProvider(databaseName: type(of: self).description()) - trackerDataProvider = MockTrackerDataProvider() - privacyStats = PrivacyStats(databaseProvider: databaseProvider, trackerDataProvider: trackerDataProvider) + privacyStats = PrivacyStats(databaseProvider: databaseProvider) } // MARK: - fetchPrivacyStats @@ -130,17 +115,105 @@ final class PrivacyStatsTests: XCTestCase { // MARK: - recordBlockedTracker - func testRecordBlockedTrackerCausesDatabaseSave() async throws { + func testThatCallingRecordBlockedTrackerCausesDatabaseSaveAfterDelay() async throws { await privacyStats.recordBlockedTracker("A") + var stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, [:]) + try await Task.sleep(nanoseconds: 1_100_000_000) - let stats = await privacyStats.fetchPrivacyStats() + stats = await privacyStats.fetchPrivacyStats() XCTAssertEqual(stats, ["A": 1]) } + func testThatStatsUpdatePublisherIsCalledAfterDatabaseSave() async throws { + await privacyStats.recordBlockedTracker("A") + + await waitForStatsUpdateEvent() + + var stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 1]) + + await privacyStats.recordBlockedTracker("B") + + await waitForStatsUpdateEvent() + + stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 1, "B": 1]) + } + + func testWhenMultipleTrackersAreReportedInQuickSuccessionThenOnlyOneStatsUpdateEventIsReported() async throws { + await withTaskGroup(of: Void.self) { group in + (0..<5).forEach { _ in + group.addTask { + await self.privacyStats.recordBlockedTracker("A") + } + } + (0..<10).forEach { _ in + group.addTask { + await self.privacyStats.recordBlockedTracker("B") + } + } + (0..<3).forEach { _ in + group.addTask { + await self.privacyStats.recordBlockedTracker("C") + } + } + } + + // We have limited testing possibilities here, so let's just await the first stats update event + // and verify that all trackers are reported by privacy stats. + await waitForStatsUpdateEvent() + + let stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 5, "B": 10, "C": 3]) + } + + // MARK: - clearPrivacyStats + + func testWhenClearPrivacyStatsIsCalledThenFetchPrivacyStatsIsEmpty() async throws { + try addObjects { context in + let date = Date() + + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "A", count: 1, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(1), companyName: "A", count: 2, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(7), companyName: "A", count: 10, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(10), companyName: "A", count: 10, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(20), companyName: "A", count: 10, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(500), companyName: "A", count: 10, context: context), + ] + } + + var stats = await privacyStats.fetchPrivacyStats() + XCTAssertFalse(stats.isEmpty) + + await privacyStats.clearPrivacyStats() + + stats = await privacyStats.fetchPrivacyStats() + XCTAssertTrue(stats.isEmpty) + + let context = databaseProvider.database.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + do { + let allObjects = try context.fetch(DailyBlockedTrackersEntity.fetchRequest()) + XCTAssertTrue(allObjects.isEmpty) + } catch { + XCTFail("fetch failed: \(error)") + } + } + } + // MARK: - Helpers + func waitForStatsUpdateEvent(file: StaticString = #file, line: UInt = #line) async { + let expectation = self.expectation(description: "statsUpdate") + let cancellable = privacyStats.statsUpdatePublisher.sink { expectation.fulfill() } + await fulfillment(of: [expectation]) + cancellable.cancel() + } + func addObjects(_ objects: (NSManagedObjectContext) -> [DailyBlockedTrackersEntity], file: StaticString = #file, line: UInt = #line) throws { let context = databaseProvider.database.makeContext(concurrencyType: .privateQueueConcurrencyType) context.performAndWait { From 09dd899e81a7ee8a53f3ac3fcf6bba206e7b824c Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 09:53:43 +0100 Subject: [PATCH 24/48] Switch to daily packs, not hourly --- Sources/Common/Extensions/DateExtension.swift | 5 ----- Sources/PrivacyStats/internal/Date+PrivacyStats.swift | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/Sources/Common/Extensions/DateExtension.swift b/Sources/Common/Extensions/DateExtension.swift index a0268835f..50003675e 100644 --- a/Sources/Common/Extensions/DateExtension.swift +++ b/Sources/Common/Extensions/DateExtension.swift @@ -71,11 +71,6 @@ public extension Date { return Calendar.current.startOfDay(for: self) } - var startOfHour: Date { - let dateComponents = Calendar.current.dateComponents([.year, .month, .day, .hour, .timeZone], from: self) - return Calendar.current.date(from: dateComponents)! - } - func daysAgo(_ days: Int) -> Date { return Calendar.current.date(byAdding: .day, value: -days, to: self)! } diff --git a/Sources/PrivacyStats/internal/Date+PrivacyStats.swift b/Sources/PrivacyStats/internal/Date+PrivacyStats.swift index 6b848a60f..55c8b11ad 100644 --- a/Sources/PrivacyStats/internal/Date+PrivacyStats.swift +++ b/Sources/PrivacyStats/internal/Date+PrivacyStats.swift @@ -22,7 +22,7 @@ import Foundation extension Date { var privacyStatsPackTimestamp: Date { - startOfHour + startOfDay } var privacyStatsOldestPackTimestamp: Date { From c6cd8447eca9bc61c830c8275a434c65873776eb Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 11:59:29 +0100 Subject: [PATCH 25/48] Add placeholder for PrivacyStatsUtilsTests --- Sources/PrivacyStats/PrivacyStats.swift | 16 ++--- .../internal/PrivacyStatsUtils.swift | 58 +++++++++++-------- .../PrivacyStatsUtilsTests.swift | 58 +++++++++++++++++++ 3 files changed, 102 insertions(+), 30 deletions(-) create mode 100644 Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 17fbf3ec9..b301ce08f 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -122,7 +122,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { return } - let statsObjects = PrivacyStatsUtils.fetchOrInsertCurrentPacks(for: Set(pack.trackers.keys), in: context) + let statsObjects = PrivacyStatsUtils.fetchOrInsertCurrentStats(for: Set(pack.trackers.keys), in: context) statsObjects.forEach { stats in if let count = pack.trackers[stats.companyName] { stats.count = count @@ -175,9 +175,10 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume(returning: nil) return } - let currentPack = PrivacyStatsUtils.fetchCurrentStatsPack(in: context) - Logger.privacyStats.debug("Loaded stats \(currentPack.timestamp) \(currentPack.trackers)") - continuation.resume(returning: currentPack) + let timestamp = Date().privacyStatsPackTimestamp + let currentDayStats = PrivacyStatsUtils.loadCurrentDayStats(in: context) + Logger.privacyStats.debug("Loaded stats \(timestamp) \(currentDayStats)") + continuation.resume(returning: PrivacyStatsPack(timestamp: timestamp, trackers: currentDayStats)) } } if let pack { @@ -188,9 +189,10 @@ public final class PrivacyStats: PrivacyStatsCollecting { private func initializeCurrentPack() -> PrivacyStatsPack { var pack: PrivacyStatsPack? context.performAndWait { - let currentPack = PrivacyStatsUtils.fetchCurrentStatsPack(in: context) - Logger.privacyStats.debug("Loaded stats \(currentPack.timestamp) \(currentPack.trackers)") - pack = currentPack + let timestamp = Date().privacyStatsPackTimestamp + let currentDayStats = PrivacyStatsUtils.loadCurrentDayStats(in: context) + Logger.privacyStats.debug("Loaded stats \(timestamp) \(currentDayStats)") + pack = PrivacyStatsPack(timestamp: timestamp, trackers: currentDayStats) } return pack ?? PrivacyStatsPack(timestamp: Date().privacyStatsPackTimestamp) } diff --git a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift index 4f289dcc5..683c3c25c 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift @@ -23,23 +23,18 @@ import Persistence final class PrivacyStatsUtils { - static func fetchCurrentStatsPack(in context: NSManagedObjectContext) -> PrivacyStatsPack { - let timestamp = Date().privacyStatsPackTimestamp - let request = DailyBlockedTrackersEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K == %@", #keyPath(DailyBlockedTrackersEntity.timestamp), timestamp as NSDate) - request.returnsObjectsAsFaults = false - - let statsObjects = (try? context.fetch(request)) ?? [] - - var pack = PrivacyStatsPack(timestamp: timestamp, trackers: [:]) - statsObjects.forEach { object in - pack.trackers[object.companyName] = object.count - } - - return pack - } - - static func fetchOrInsertCurrentPacks(for companyNames: Set, in context: NSManagedObjectContext) -> [DailyBlockedTrackersEntity] { + /** + * Returns objects corresponding to current stats for companies specified by `companyNames`. + * + * If an object doesn't exist (no trackers for a given company were reported on a given day) + * then a new object for that company is inserted into the context and returned. + * If a user opens the app for the first time on a given day, the database will not contain + * any records for that day and this function will only insert new objects. + * + * > Note: `current stats` refer to stats objects that are active on a given day, i.e. their + * timestamp's day matches current day. + */ + static func fetchOrInsertCurrentStats(for companyNames: Set, in context: NSManagedObjectContext) -> [DailyBlockedTrackersEntity] { let timestamp = Date().privacyStatsPackTimestamp let request = DailyBlockedTrackersEntity.fetchRequest() @@ -57,12 +52,24 @@ final class PrivacyStatsUtils { return statsObjects } + /** + * Returns a dictionary representation of blocked trackers counts grouped by company name for the current day. + */ + static func loadCurrentDayStats(in context: NSManagedObjectContext) -> [String: Int64] { + let startDate = Date().privacyStatsPackTimestamp + return loadBlockedTrackerStats(since: startDate, in: context) + } + + /** + * Returns a dictionary representation of blocked trackers counts grouped by company name for past 7 days. + */ static func load7DayStats(in context: NSManagedObjectContext) -> [String: Int64] { let startDate = Date().privacyStatsOldestPackTimestamp + return loadBlockedTrackerStats(since: startDate, in: context) + } + private static func loadBlockedTrackerStats(since startDate: Date, in context: NSManagedObjectContext) -> [String: Int64] { let request = NSFetchRequest(entityName: "DailyBlockedTrackersEntity") - - // Predicate to filter by date range request.predicate = NSPredicate(format: "%K >= %@", #keyPath(DailyBlockedTrackersEntity.timestamp), startDate as NSDate) // Expression description for the sum of count @@ -91,15 +98,20 @@ final class PrivacyStatsUtils { return groupedResults } - static func deleteOutdatedPacks(olderThan date: Date = Date(), in context: NSManagedObjectContext) { - let thisHour = date.privacyStatsPackTimestamp - let oldestValidTimestamp = thisHour.daysAgo(7) + /** + * Deletes stats older than 7 days for all companies. + */ + static func deleteOutdatedPacks(in context: NSManagedObjectContext) { + let oldestValidTimestamp = Date().privacyStatsOldestPackTimestamp let request = DailyBlockedTrackersEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K <= %@", #keyPath(DailyBlockedTrackersEntity.timestamp), oldestValidTimestamp as NSDate) + request.predicate = NSPredicate(format: "%K < %@", #keyPath(DailyBlockedTrackersEntity.timestamp), oldestValidTimestamp as NSDate) context.deleteAll(matching: request) } + /** + * Deletes all stats entries in the database. + */ static func deleteAllStats(in context: NSManagedObjectContext) { context.deleteAll(matching: DailyBlockedTrackersEntity.fetchRequest()) } diff --git a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift new file mode 100644 index 000000000..ea5390e42 --- /dev/null +++ b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift @@ -0,0 +1,58 @@ +// +// PrivacyStatsUtilsTests.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import CoreData +import XCTest +@testable import PrivacyStats + +final class PrivacyStatsUtilsTests: XCTestCase { + + /** + * Returns objects corresponding to current stats for companies specified by `companyNames`. + * + * If an object doesn't exist (no trackers for a given company were reported on a given day) + * then a new object for that company is inserted into the context and returned. + * If a user opens the app for the first time on a given day, the database will not contain + * any records for that day and this function will only insert new objects. + * + * > Note: `current stats` refer to stats objects that are active on a given day, i.e. their + * timestamp's day matches current day. + */ + // static func fetchOrInsertCurrentStats(for companyNames: Set, in context: NSManagedObjectContext) -> [DailyBlockedTrackersEntity] + + /** + * Returns a dictionary representation of blocked trackers counts grouped by company name for the current day. + */ + // static func loadCurrentDayStats(in context: NSManagedObjectContext) -> [String: Int64] + + /** + * Returns a dictionary representation of blocked trackers counts grouped by company name for past 7 days. + */ + // static func load7DayStats(in context: NSManagedObjectContext) -> [String: Int64] + + /** + * Deletes stats older than 7 days for all companies. + */ + // static func deleteOutdatedPacks(in context: NSManagedObjectContext) + + /** + * Deletes all stats entries in the database. + */ + // static func deleteAllStats(in context: NSManagedObjectContext) +} From 68d34b5d4d6f79ce37a9fe3c5a6d5336ae870054 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 12:25:54 +0100 Subject: [PATCH 26/48] Add EventMapping to PrivacyStats --- Sources/PrivacyStats/PrivacyStats.swift | 96 +++++++++++++++---- .../internal/PrivacyStatsUtils.swift | 16 ++-- 2 files changed, 85 insertions(+), 27 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index b301ce08f..1cdb353f5 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -17,6 +17,7 @@ // import Combine +import Common import Foundation import os.log import Persistence @@ -39,6 +40,42 @@ public protocol PrivacyStatsCollecting { func clearPrivacyStats() async } +/** + * Errors that may be reported by `PrivacyStats`. + */ +public enum PrivacyStatsError: CustomNSError { + case failedToFetchPrivacyStatsSummary(Error) + case failedToStorePrivacyStats(Error) + case failedToClearPrivacyStats(Error) + case failedToLoadCurrentPrivacyStats(Error) + + public static let errorDomain: String = "PrivacyStatsError" + + public var errorCode: Int { + switch self { + case .failedToFetchPrivacyStatsSummary: + return 1 + case .failedToStorePrivacyStats: + return 2 + case .failedToClearPrivacyStats: + return 3 + case .failedToLoadCurrentPrivacyStats: + return 4 + } + } + + public var underlyingError: Error { + switch self { + case .failedToFetchPrivacyStatsSummary(let error), + .failedToStorePrivacyStats(let error), + .failedToClearPrivacyStats(let error), + .failedToLoadCurrentPrivacyStats(let error): + return error + } + } +} + + public final class PrivacyStats: PrivacyStatsCollecting { public static let bundle = Bundle.module @@ -51,9 +88,12 @@ public final class PrivacyStats: PrivacyStatsCollecting { private let statsUpdateSubject = PassthroughSubject() private var cancellables: Set = [] - public init(databaseProvider: PrivacyStatsDatabaseProviding) { + private let errorEvents: EventMapping? + + public init(databaseProvider: PrivacyStatsDatabaseProviding, errorEvents: EventMapping? = nil) { self.db = databaseProvider.initializeDatabase() self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") + self.errorEvents = errorEvents statsUpdatePublisher = statsUpdateSubject.eraseToAnyPublisher() currentPack = .init(pack: initializeCurrentPack()) @@ -86,8 +126,13 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume(returning: [:]) return } - let stats = PrivacyStatsUtils.load7DayStats(in: context) - continuation.resume(returning: stats) + do { + let stats = try PrivacyStatsUtils.load7DayStats(in: context) + continuation.resume(returning: stats) + } catch { + errorEvents?.fire(.failedToFetchPrivacyStatsSummary(error)) + continuation.resume(returning: [:]) + } } } } @@ -105,6 +150,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { Logger.privacyStats.debug("Deleted outdated entries") } catch { Logger.privacyStats.error("Save error: \(error)") + errorEvents?.fire(.failedToFetchPrivacyStatsSummary(error)) } continuation.resume() } @@ -122,24 +168,25 @@ public final class PrivacyStats: PrivacyStatsCollecting { return } - let statsObjects = PrivacyStatsUtils.fetchOrInsertCurrentStats(for: Set(pack.trackers.keys), in: context) - statsObjects.forEach { stats in - if let count = pack.trackers[stats.companyName] { - stats.count = count + do { + let statsObjects = try PrivacyStatsUtils.fetchOrInsertCurrentStats(for: Set(pack.trackers.keys), in: context) + statsObjects.forEach { stats in + if let count = pack.trackers[stats.companyName] { + stats.count = count + } } - } - guard context.hasChanges else { - continuation.resume() - return - } + guard context.hasChanges else { + continuation.resume() + return + } - do { try context.save() Logger.privacyStats.debug("Saved stats \(pack.timestamp) \(pack.trackers)") statsUpdateSubject.send() } catch { Logger.privacyStats.error("Save error: \(error)") + errorEvents?.fire(.failedToStorePrivacyStats(error)) } continuation.resume() } @@ -161,6 +208,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { Logger.privacyStats.debug("Deleted outdated entries") } catch { Logger.privacyStats.error("Save error: \(error)") + errorEvents?.fire(.failedToClearPrivacyStats(error)) } } continuation.resume() @@ -176,9 +224,14 @@ public final class PrivacyStats: PrivacyStatsCollecting { return } let timestamp = Date().privacyStatsPackTimestamp - let currentDayStats = PrivacyStatsUtils.loadCurrentDayStats(in: context) - Logger.privacyStats.debug("Loaded stats \(timestamp) \(currentDayStats)") - continuation.resume(returning: PrivacyStatsPack(timestamp: timestamp, trackers: currentDayStats)) + do { + let currentDayStats = try PrivacyStatsUtils.loadCurrentDayStats(in: context) + Logger.privacyStats.debug("Loaded stats \(timestamp) \(currentDayStats)") + continuation.resume(returning: PrivacyStatsPack(timestamp: timestamp, trackers: currentDayStats)) + } catch { + Logger.privacyStats.error("Faild to load current stats: \(error)") + errorEvents?.fire(.failedToLoadCurrentPrivacyStats(error)) + } } } if let pack { @@ -190,9 +243,14 @@ public final class PrivacyStats: PrivacyStatsCollecting { var pack: PrivacyStatsPack? context.performAndWait { let timestamp = Date().privacyStatsPackTimestamp - let currentDayStats = PrivacyStatsUtils.loadCurrentDayStats(in: context) - Logger.privacyStats.debug("Loaded stats \(timestamp) \(currentDayStats)") - pack = PrivacyStatsPack(timestamp: timestamp, trackers: currentDayStats) + do { + let currentDayStats = try PrivacyStatsUtils.loadCurrentDayStats(in: context) + Logger.privacyStats.debug("Loaded stats \(timestamp) \(currentDayStats)") + pack = PrivacyStatsPack(timestamp: timestamp, trackers: currentDayStats) + } catch { + Logger.privacyStats.error("Faild to load current stats: \(error)") + errorEvents?.fire(.failedToLoadCurrentPrivacyStats(error)) + } } return pack ?? PrivacyStatsPack(timestamp: Date().privacyStatsPackTimestamp) } diff --git a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift index 683c3c25c..56dd28ffd 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift @@ -34,7 +34,7 @@ final class PrivacyStatsUtils { * > Note: `current stats` refer to stats objects that are active on a given day, i.e. their * timestamp's day matches current day. */ - static func fetchOrInsertCurrentStats(for companyNames: Set, in context: NSManagedObjectContext) -> [DailyBlockedTrackersEntity] { + static func fetchOrInsertCurrentStats(for companyNames: Set, in context: NSManagedObjectContext) throws -> [DailyBlockedTrackersEntity] { let timestamp = Date().privacyStatsPackTimestamp let request = DailyBlockedTrackersEntity.fetchRequest() @@ -43,7 +43,7 @@ final class PrivacyStatsUtils { #keyPath(DailyBlockedTrackersEntity.companyName), companyNames) request.returnsObjectsAsFaults = false - var statsObjects = (try? context.fetch(request)) ?? [] + var statsObjects = try context.fetch(request) let missingCompanyNames = companyNames.subtracting(statsObjects.map(\.companyName)) for companyName in missingCompanyNames { @@ -55,20 +55,20 @@ final class PrivacyStatsUtils { /** * Returns a dictionary representation of blocked trackers counts grouped by company name for the current day. */ - static func loadCurrentDayStats(in context: NSManagedObjectContext) -> [String: Int64] { + static func loadCurrentDayStats(in context: NSManagedObjectContext) throws -> [String: Int64] { let startDate = Date().privacyStatsPackTimestamp - return loadBlockedTrackerStats(since: startDate, in: context) + return try loadBlockedTrackersStats(since: startDate, in: context) } /** * Returns a dictionary representation of blocked trackers counts grouped by company name for past 7 days. */ - static func load7DayStats(in context: NSManagedObjectContext) -> [String: Int64] { + static func load7DayStats(in context: NSManagedObjectContext) throws -> [String: Int64] { let startDate = Date().privacyStatsOldestPackTimestamp - return loadBlockedTrackerStats(since: startDate, in: context) + return try loadBlockedTrackersStats(since: startDate, in: context) } - private static func loadBlockedTrackerStats(since startDate: Date, in context: NSManagedObjectContext) -> [String: Int64] { + private static func loadBlockedTrackersStats(since startDate: Date, in context: NSManagedObjectContext) throws -> [String: Int64] { let request = NSFetchRequest(entityName: "DailyBlockedTrackersEntity") request.predicate = NSPredicate(format: "%K >= %@", #keyPath(DailyBlockedTrackersEntity.timestamp), startDate as NSDate) @@ -86,7 +86,7 @@ final class PrivacyStatsUtils { request.propertiesToFetch = [#keyPath(DailyBlockedTrackersEntity.companyName), sumExpressionDescription] request.resultType = .dictionaryResultType - let results = ((try? context.fetch(request)) as? [[String: Any]]) ?? [] + let results = (try context.fetch(request) as? [[String: Any]]) ?? [] let groupedResults = results.reduce(into: [String: Int64]()) { partialResult, result in if let companyName = result[#keyPath(DailyBlockedTrackersEntity.companyName)] as? String, From d1549cdd6d24737d244b522d684275a2c23111d5 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 14:22:47 +0100 Subject: [PATCH 27/48] Add tests for PrivacyStatsUtils --- .../internal/PrivacyStatsUtils.swift | 10 +- .../PrivacyStatsTests/PrivacyStatsTests.swift | 48 +-- .../PrivacyStatsUtilsTests.swift | 356 ++++++++++++++++-- .../TestPrivacyStatsDatabaseProvider.swift | 59 +++ 4 files changed, 395 insertions(+), 78 deletions(-) create mode 100644 Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift diff --git a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift index 56dd28ffd..efa515b3d 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift @@ -72,6 +72,8 @@ final class PrivacyStatsUtils { let request = NSFetchRequest(entityName: "DailyBlockedTrackersEntity") request.predicate = NSPredicate(format: "%K >= %@", #keyPath(DailyBlockedTrackersEntity.timestamp), startDate as NSDate) + let companyNameKey = #keyPath(DailyBlockedTrackersEntity.companyName) + // Expression description for the sum of count let countExpression = NSExpression(forKeyPath: #keyPath(DailyBlockedTrackersEntity.count)) let sumExpression = NSExpression(forFunction: "sum:", arguments: [countExpression]) @@ -81,16 +83,14 @@ final class PrivacyStatsUtils { sumExpressionDescription.expression = sumExpression sumExpressionDescription.expressionResultType = .integer64AttributeType - // Configure the fetch request for aggregation - request.propertiesToGroupBy = [#keyPath(DailyBlockedTrackersEntity.companyName)] - request.propertiesToFetch = [#keyPath(DailyBlockedTrackersEntity.companyName), sumExpressionDescription] + request.propertiesToGroupBy = [companyNameKey] + request.propertiesToFetch = [companyNameKey, sumExpressionDescription] request.resultType = .dictionaryResultType let results = (try context.fetch(request) as? [[String: Any]]) ?? [] let groupedResults = results.reduce(into: [String: Int64]()) { partialResult, result in - if let companyName = result[#keyPath(DailyBlockedTrackersEntity.companyName)] as? String, - let totalCount = result["totalCount"] as? Int64 { + if let companyName = result[companyNameKey] as? String, let totalCount = result["totalCount"] as? Int64, totalCount > 0 { partialResult[companyName] = totalCount } } diff --git a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift index 94a90644f..f1ea9dce4 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift @@ -22,30 +22,6 @@ import TrackerRadarKit import XCTest @testable import PrivacyStats -final class TestPrivacyStatsDatabaseProvider: PrivacyStatsDatabaseProviding { - let databaseName: String - var database: CoreDataDatabase! - var location: URL! - - init(databaseName: String) { - self.databaseName = databaseName - } - - func initializeDatabase() -> CoreDataDatabase { - location = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) - let model = CoreDataDatabase.loadModel(from: PrivacyStats.bundle, named: "PrivacyStats")! - database = CoreDataDatabase(name: databaseName, containerLocation: location, model: model) - database.loadStore() - return database - } - - func tearDownDatabase() { - try? database.tearDown(deleteStores: true) - database = nil - try? FileManager.default.removeItem(at: location) - } -} - final class PrivacyStatsTests: XCTestCase { var databaseProvider: TestPrivacyStatsDatabaseProvider! var privacyStats: PrivacyStats! @@ -55,6 +31,10 @@ final class PrivacyStatsTests: XCTestCase { privacyStats = PrivacyStats(databaseProvider: databaseProvider) } + override func tearDown() async throws { + databaseProvider.tearDownDatabase() + } + // MARK: - fetchPrivacyStats func testThatPrivacyStatsAreFetched() async throws { @@ -63,7 +43,7 @@ final class PrivacyStatsTests: XCTestCase { } func testThatFetchPrivacyStatsReturnsAllCompanies() async throws { - try addObjects { context in + try databaseProvider.addObjects { context in [ DailyBlockedTrackersEntity.make(companyName: "A", count: 10, context: context), DailyBlockedTrackersEntity.make(companyName: "B", count: 5, context: context), @@ -77,7 +57,7 @@ final class PrivacyStatsTests: XCTestCase { } func testThatFetchPrivacyStatsReturnsSumOfCompanyEntriesForPast7Days() async throws { - try addObjects { context in + try databaseProvider.addObjects { context in let date = Date() return [ @@ -96,7 +76,7 @@ final class PrivacyStatsTests: XCTestCase { } func testThatFetchPrivacyStatsDiscardsEntriesOlderThan7Days() async throws { - try addObjects { context in + try databaseProvider.addObjects { context in let date = Date() return [ @@ -173,7 +153,7 @@ final class PrivacyStatsTests: XCTestCase { // MARK: - clearPrivacyStats func testWhenClearPrivacyStatsIsCalledThenFetchPrivacyStatsIsEmpty() async throws { - try addObjects { context in + try databaseProvider.addObjects { context in let date = Date() return [ @@ -213,16 +193,4 @@ final class PrivacyStatsTests: XCTestCase { await fulfillment(of: [expectation]) cancellable.cancel() } - - func addObjects(_ objects: (NSManagedObjectContext) -> [DailyBlockedTrackersEntity], file: StaticString = #file, line: UInt = #line) throws { - let context = databaseProvider.database.makeContext(concurrencyType: .privateQueueConcurrencyType) - context.performAndWait { - _ = objects(context) - do { - try context.save() - } catch { - XCTFail("save failed: \(error)", file: file, line: line) - } - } - } } diff --git a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift index ea5390e42..80bcd2cb1 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift @@ -17,42 +17,332 @@ // limitations under the License. // -import CoreData +import Persistence import XCTest @testable import PrivacyStats final class PrivacyStatsUtilsTests: XCTestCase { + var databaseProvider: TestPrivacyStatsDatabaseProvider! + var database: CoreDataDatabase! - /** - * Returns objects corresponding to current stats for companies specified by `companyNames`. - * - * If an object doesn't exist (no trackers for a given company were reported on a given day) - * then a new object for that company is inserted into the context and returned. - * If a user opens the app for the first time on a given day, the database will not contain - * any records for that day and this function will only insert new objects. - * - * > Note: `current stats` refer to stats objects that are active on a given day, i.e. their - * timestamp's day matches current day. - */ - // static func fetchOrInsertCurrentStats(for companyNames: Set, in context: NSManagedObjectContext) -> [DailyBlockedTrackersEntity] - - /** - * Returns a dictionary representation of blocked trackers counts grouped by company name for the current day. - */ - // static func loadCurrentDayStats(in context: NSManagedObjectContext) -> [String: Int64] - - /** - * Returns a dictionary representation of blocked trackers counts grouped by company name for past 7 days. - */ - // static func load7DayStats(in context: NSManagedObjectContext) -> [String: Int64] - - /** - * Deletes stats older than 7 days for all companies. - */ - // static func deleteOutdatedPacks(in context: NSManagedObjectContext) - - /** - * Deletes all stats entries in the database. - */ - // static func deleteAllStats(in context: NSManagedObjectContext) + override func setUp() async throws { + databaseProvider = TestPrivacyStatsDatabaseProvider(databaseName: type(of: self).description()) + databaseProvider.initializeDatabase() + database = databaseProvider.database + } + + override func tearDown() async throws { + databaseProvider.tearDownDatabase() + } + + // MARK: - fetchOrInsertCurrentStats + + func testWhenThereAreNoObjectsForCompaniesThenFetchOrInsertCurrentStatsInsertsNewObjects() { + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + let currentPackTimestamp = Date().privacyStatsPackTimestamp + let companyNames: Set = ["A", "B", "C", "D"] + + var returnedEntities: [DailyBlockedTrackersEntity] = [] + do { + returnedEntities = try PrivacyStatsUtils.fetchOrInsertCurrentStats(for: companyNames, in: context) + } catch { + XCTFail("Should not throw") + } + + let insertedEntities = context.insertedObjects.compactMap { $0 as? DailyBlockedTrackersEntity } + + XCTAssertEqual(returnedEntities.count, 4) + XCTAssertEqual(insertedEntities.count, 4) + XCTAssertEqual(Set(insertedEntities.map(\.companyName)), companyNames) + XCTAssertEqual(Set(insertedEntities.map(\.companyName)), Set(returnedEntities.map(\.companyName))) + + // All inserted entries have the same timestamp + XCTAssertEqual(Set(insertedEntities.map(\.timestamp)), [currentPackTimestamp]) + + // All inserted entries have the count of 0 + XCTAssertEqual(Set(insertedEntities.map(\.count)), [0]) + } + } + + func testWhenThereAreExistingObjectsForCompaniesThenFetchOrInsertCurrentStatsReturnsThem() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "A", count: 123, context: context), + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "B", count: 4567, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + let companyNames: Set = ["A", "B", "C", "D"] + + var returnedEntities: [DailyBlockedTrackersEntity] = [] + do { + returnedEntities = try PrivacyStatsUtils.fetchOrInsertCurrentStats(for: companyNames, in: context) + } catch { + XCTFail("Should not throw") + } + + let insertedEntities = context.insertedObjects.compactMap { $0 as? DailyBlockedTrackersEntity } + + XCTAssertEqual(returnedEntities.count, 4) + XCTAssertEqual(insertedEntities.count, 2) + XCTAssertEqual(Set(returnedEntities.map(\.companyName)), companyNames) + XCTAssertEqual(Set(insertedEntities.map(\.companyName)), ["C", "D"]) + + do { + let companyA = try XCTUnwrap(returnedEntities.first { $0.companyName == "A" }) + let companyB = try XCTUnwrap(returnedEntities.first { $0.companyName == "B" }) + + XCTAssertEqual(companyA.count, 123) + XCTAssertEqual(companyB.count, 4567) + } catch { + XCTFail("Should find companies A and B") + } + } + } + + // MARK: - loadCurrentDayStats + + func testWhenThereAreNoObjectsInDatabaseThenLoadCurrentDayStatsIsEmpty() throws { + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + do { + let currentDayStats = try PrivacyStatsUtils.loadCurrentDayStats(in: context) + XCTAssertTrue(currentDayStats.isEmpty) + } catch { + XCTFail("Should not throw") + } + } + } + + func testWhenThereAreObjectsInDatabaseForPreviousDaysThenLoadCurrentDayStatsIsEmpty() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(1), companyName: "A", count: 123, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(2), companyName: "B", count: 4567, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(5), companyName: "C", count: 890, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + do { + let currentDayStats = try PrivacyStatsUtils.loadCurrentDayStats(in: context) + XCTAssertTrue(currentDayStats.isEmpty) + } catch { + XCTFail("Should not throw") + } + } + } + + func testThatObjectsWithZeroCountAreNotReportedByLoadCurrentDayStats() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "A", count: 0, context: context), + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "B", count: 0, context: context), + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "C", count: 0, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + do { + let currentDayStats = try PrivacyStatsUtils.loadCurrentDayStats(in: context) + XCTAssertTrue(currentDayStats.isEmpty) + } catch { + XCTFail("Should not throw") + } + } + } + + func testThatObjectsWithNonZeroCountAreReportedByLoadCurrentDayStats() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "A", count: 150, context: context), + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "B", count: 400, context: context), + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "C", count: 84, context: context), + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "D", count: 5, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + do { + let currentDayStats = try PrivacyStatsUtils.loadCurrentDayStats(in: context) + XCTAssertEqual(currentDayStats, ["A": 150, "B": 400, "C": 84, "D": 5]) + } catch { + XCTFail("Should not throw") + } + } + } + + // MARK: - load7DayStats + + func testWhenThereAreNoObjectsInDatabaseThenLoad7DayStatsIsEmpty() throws { + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + do { + let stats = try PrivacyStatsUtils.load7DayStats(in: context) + XCTAssertTrue(stats.isEmpty) + } catch { + XCTFail("Should not throw") + } + } + } + + func testWhenThereAreObjectsInDatabaseFrom7DaysAgoOrMoreThenLoad7DayStatsIsEmpty() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(10), companyName: "A", count: 123, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(20), companyName: "B", count: 4567, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(7), companyName: "C", count: 890, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + do { + let stats = try PrivacyStatsUtils.load7DayStats(in: context) + XCTAssertTrue(stats.isEmpty) + } catch { + XCTFail("Should not throw") + } + } + } + + func testThatObjectsWithZeroCountAreNotReportedByLoad7DayStats() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "A", count: 0, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(4), companyName: "B", count: 0, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(6), companyName: "C", count: 0, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + do { + let stats = try PrivacyStatsUtils.load7DayStats(in: context) + XCTAssertTrue(stats.isEmpty) + } catch { + XCTFail("Should not throw") + } + } + } + + func testThatObjectsWithNonZeroCountAreReportedByLoad7DayStats() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "A", count: 150, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(1), companyName: "B", count: 400, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(2), companyName: "C", count: 84, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(6), companyName: "D", count: 5, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + do { + let stats = try PrivacyStatsUtils.load7DayStats(in: context) + XCTAssertEqual(stats, ["A": 150, "B": 400, "C": 84, "D": 5]) + } catch { + XCTFail("Should not throw") + } + } + } + + // MARK: - deleteOutdatedPacks + + func testWhenDeleteOutdatedPacksIsCalledThenObjectsFrom7DaysAgoOrMoreAreDeleted() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "C", count: 1, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(4), companyName: "C", count: 2, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(6), companyName: "C", count: 3, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(7), companyName: "C", count: 4, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(8), companyName: "C", count: 5, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(100), companyName: "C", count: 6, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + PrivacyStatsUtils.deleteOutdatedPacks(in: context) + + let deletedObjects = context.deletedObjects.compactMap { $0 as? DailyBlockedTrackersEntity } + + XCTAssertEqual(deletedObjects.count, 3) + XCTAssertEqual(Set(deletedObjects.map(\.count)), [4, 5, 6]) + } + } + + func testWhenObjectsFrom7DaysAgoOrMoreAreNotPresentThenDeleteOutdatedPacksHasNoEffect() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "C", count: 1, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(4), companyName: "C", count: 2, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(6), companyName: "C", count: 3, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + PrivacyStatsUtils.deleteOutdatedPacks(in: context) + XCTAssertFalse(context.hasChanges) + } + } + + // MARK: - deleteAllStats + + func testThatDeleteAllStatsRemovesAllDatabaseObjects() throws { + let date = Date() + + try databaseProvider.addObjects { context in + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "C", count: 1, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(4), companyName: "C", count: 2, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(6), companyName: "C", count: 3, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(60), companyName: "C", count: 3, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(600), companyName: "C", count: 3, context: context) + ] + } + + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + PrivacyStatsUtils.deleteAllStats(in: context) + + XCTAssertEqual(context.deletedObjects.count, 5) + } + } } diff --git a/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift b/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift new file mode 100644 index 000000000..0d4ee8c06 --- /dev/null +++ b/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift @@ -0,0 +1,59 @@ +// +// TestPrivacyStatsDatabaseProvider.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Persistence +import XCTest +@testable import PrivacyStats + +final class TestPrivacyStatsDatabaseProvider: PrivacyStatsDatabaseProviding { + let databaseName: String + var database: CoreDataDatabase! + var location: URL! + + init(databaseName: String) { + self.databaseName = databaseName + } + + @discardableResult + func initializeDatabase() -> CoreDataDatabase { + location = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + let model = CoreDataDatabase.loadModel(from: PrivacyStats.bundle, named: "PrivacyStats")! + database = CoreDataDatabase(name: databaseName, containerLocation: location, model: model) + database.loadStore() + return database + } + + func tearDownDatabase() { + try? database.tearDown(deleteStores: true) + database = nil + try? FileManager.default.removeItem(at: location) + } + + func addObjects(_ objects: (NSManagedObjectContext) -> [DailyBlockedTrackersEntity], file: StaticString = #file, line: UInt = #line) throws { + let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + _ = objects(context) + do { + try context.save() + } catch { + XCTFail("save failed: \(error)", file: file, line: line) + } + } + } +} From 113d575ac993bd57e5b00109026fb7ea1561cc54 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 15:36:29 +0100 Subject: [PATCH 28/48] Add tests for CurrentPack --- Sources/PrivacyStats/PrivacyStats.swift | 48 ++---- .../PrivacyStats/internal/CurrentPack.swift | 101 +++++++++++++ ...swift => DailyBlockedTrackersEntity.swift} | 2 +- .../internal/Date+PrivacyStats.swift | 11 ++ .../internal/PrivacyStatsPack.swift | 52 +------ .../PrivacyStatsTests/CurrentPackTests.swift | 139 ++++++++++++++++++ .../PrivacyStatsTests/PrivacyStatsTests.swift | 2 +- .../PrivacyStatsUtilsTests.swift | 1 - 8 files changed, 269 insertions(+), 87 deletions(-) create mode 100644 Sources/PrivacyStats/internal/CurrentPack.swift rename Sources/PrivacyStats/internal/{DailyBlockedTrackers.swift => DailyBlockedTrackersEntity.swift} (98%) create mode 100644 Tests/PrivacyStatsTests/CurrentPackTests.swift diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 1cdb353f5..bd55e0925 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -28,18 +28,6 @@ import AppKit import UIKit #endif -public protocol PrivacyStatsDatabaseProviding { - func initializeDatabase() -> CoreDataDatabase -} - -public protocol PrivacyStatsCollecting { - func recordBlockedTracker(_ name: String) async - - var statsUpdatePublisher: AnyPublisher { get } - func fetchPrivacyStats() async -> [String: Int64] - func clearPrivacyStats() async -} - /** * Errors that may be reported by `PrivacyStats`. */ @@ -75,6 +63,17 @@ public enum PrivacyStatsError: CustomNSError { } } +public protocol PrivacyStatsDatabaseProviding { + func initializeDatabase() -> CoreDataDatabase +} + +public protocol PrivacyStatsCollecting { + func recordBlockedTracker(_ name: String) async + + var statsUpdatePublisher: AnyPublisher { get } + func fetchPrivacyStats() async -> [String: Int64] + func clearPrivacyStats() async +} public final class PrivacyStats: PrivacyStatsCollecting { @@ -155,7 +154,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume() } } - await loadCurrentPack() + await currentPack?.resetPack() } // MARK: - Private @@ -216,29 +215,6 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } - private func loadCurrentPack() async { - let pack = await withCheckedContinuation { (continuation: CheckedContinuation) in - context.perform { [weak self] in - guard let self else { - continuation.resume(returning: nil) - return - } - let timestamp = Date().privacyStatsPackTimestamp - do { - let currentDayStats = try PrivacyStatsUtils.loadCurrentDayStats(in: context) - Logger.privacyStats.debug("Loaded stats \(timestamp) \(currentDayStats)") - continuation.resume(returning: PrivacyStatsPack(timestamp: timestamp, trackers: currentDayStats)) - } catch { - Logger.privacyStats.error("Faild to load current stats: \(error)") - errorEvents?.fire(.failedToLoadCurrentPrivacyStats(error)) - } - } - } - if let pack { - await currentPack?.updatePack(pack) - } - } - private func initializeCurrentPack() -> PrivacyStatsPack { var pack: PrivacyStatsPack? context.performAndWait { diff --git a/Sources/PrivacyStats/internal/CurrentPack.swift b/Sources/PrivacyStats/internal/CurrentPack.swift new file mode 100644 index 000000000..f7bfe5bfe --- /dev/null +++ b/Sources/PrivacyStats/internal/CurrentPack.swift @@ -0,0 +1,101 @@ +// +// CurrentPack.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Combine +import Foundation +import os.log + +/** + * This actor provides thread-safe access to an instance of `PrivacyStatsPack`. + * + * It's used by `PrivacyStats` class to record blocked trackers that can possibly + * come from multiple open tabs (web views) at the same time. + */ +actor CurrentPack { + /** + * Current stats pack. + */ + private(set) var pack: PrivacyStatsPack + + /** + * Publisher that fires events whenever tracker stats are ready to be persisted to disk. + * + * This happens after recording new blocked tracker, when no new tracker has been recorded for 1s. + */ + nonisolated private(set) lazy var commitChangesPublisher: AnyPublisher = commitChangesSubject.eraseToAnyPublisher() + + nonisolated private let commitChangesSubject = PassthroughSubject() + private var commitTask: Task? + private var commitDebounce: UInt64 + + /// The `commitDebounce` parameter should only be modified in unit tests. + init(pack: PrivacyStatsPack, commitDebounce: UInt64 = 1_000_000_000) { + self.pack = pack + self.commitDebounce = commitDebounce + } + + /** + * This function is used when clearing app data, to clear any stats cached in memory. + * + * It sets a new empty pack with the current timestamp. + */ + func resetPack() { + resetStats(andSet: Date().privacyStatsPackTimestamp) + } + + /** + * This function increments trackers count for a given company name. + * + * Updates are kept in memory and scheduled for saving to persistent storage with 1s debounce. + * This function also detects when the current pack becomes outdated (which happens + * when current timestamp's day becomes greater than pack's timestamp's day), in which + * case current pack is scheduled for persisting on disk and a new empty pack is + * created for the new timestamp. + */ + func recordBlockedTracker(_ companyName: String) { + + let currentTimestamp = Date().privacyStatsPackTimestamp + if currentTimestamp != pack.timestamp { + Logger.privacyStats.debug("New timestamp detected, storing trackers state and creating new pack") + commitChangesSubject.send(pack) + resetStats(andSet: currentTimestamp) + } + + let count = pack.trackers[companyName] ?? 0 + pack.trackers[companyName] = count + 1 + + commitTask?.cancel() + commitTask = Task { + do { + // Note that this doesn't always sleep for the full debounce time, but the sleep is interrupted + // as soon as the task gets cancelled. + try await Task.sleep(nanoseconds: commitDebounce) + + Logger.privacyStats.debug("Storing trackers state") + commitChangesSubject.send(pack) + } catch { + // Commit task got cancelled + } + } + } + + private func resetStats(andSet newTimestamp: Date) { + pack = PrivacyStatsPack(timestamp: newTimestamp, trackers: [:]) + } +} diff --git a/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift b/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift similarity index 98% rename from Sources/PrivacyStats/internal/DailyBlockedTrackers.swift rename to Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift index 46da05018..d785d295e 100644 --- a/Sources/PrivacyStats/internal/DailyBlockedTrackers.swift +++ b/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift @@ -1,5 +1,5 @@ // -// DailyBlockedTrackers.swift +// DailyBlockedTrackersEntity.swift // // Copyright © 2024 DuckDuckGo. All rights reserved. // diff --git a/Sources/PrivacyStats/internal/Date+PrivacyStats.swift b/Sources/PrivacyStats/internal/Date+PrivacyStats.swift index 55c8b11ad..017ef2d94 100644 --- a/Sources/PrivacyStats/internal/Date+PrivacyStats.swift +++ b/Sources/PrivacyStats/internal/Date+PrivacyStats.swift @@ -21,10 +21,21 @@ import Foundation extension Date { + /** + * Returns a valid timestamp for `DailyBlockedTrackersEntity` instance matching the sender. + * + * Blocked trackers are packed by day so the timestap of the pack must be the exact start of a day. + */ var privacyStatsPackTimestamp: Date { startOfDay } + /** + * Returns the oldest valid timestamp for `DailyBlockedTrackersEntity` instance matching the sender. + * + * Privacy Stats only keeps track of 7 days worth of tracking history, so the oldest timestamp is + * beginning of the day 6 days ago. + */ var privacyStatsOldestPackTimestamp: Date { privacyStatsPackTimestamp.daysAgo(6) } diff --git a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift index daed51340..3c4c8a04b 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsPack.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsPack.swift @@ -16,11 +16,12 @@ // limitations under the License. // -import Combine import Foundation -import os.log -struct PrivacyStatsPack: Sendable { +/** + * This struct keeps track of the summary of blocked trackers for a single unit of time (1 day). + */ +struct PrivacyStatsPack: Equatable { let timestamp: Date var trackers: [String: Int64] @@ -29,48 +30,3 @@ struct PrivacyStatsPack: Sendable { self.trackers = trackers } } - -actor CurrentPack { - var pack: PrivacyStatsPack - - nonisolated private(set) lazy var commitChangesPublisher: AnyPublisher = commitChangesSubject.eraseToAnyPublisher() - nonisolated private let commitChangesSubject = PassthroughSubject() - - private var commitTask: Task? - - init(pack: PrivacyStatsPack) { - self.pack = pack - } - - func updatePack(_ pack: PrivacyStatsPack) { - self.pack = pack - } - - func recordBlockedTracker(_ name: String) { - - let currentTimestamp = Date().privacyStatsPackTimestamp - if currentTimestamp != pack.timestamp { - commitChangesSubject.send(pack) - resetStats(andSet: currentTimestamp) - } - - let count = pack.trackers[name] ?? 0 - pack.trackers[name] = count + 1 - - commitTask?.cancel() - commitTask = Task { - do { - try await Task.sleep(nanoseconds: 1_000_000_000) - - Logger.privacyStats.debug("Storing trackers state") - commitChangesSubject.send(pack) - } catch { - // commit task got cancelled - } - } - } - - private func resetStats(andSet newTimestamp: Date) { - pack = PrivacyStatsPack(timestamp: newTimestamp, trackers: [:]) - } -} diff --git a/Tests/PrivacyStatsTests/CurrentPackTests.swift b/Tests/PrivacyStatsTests/CurrentPackTests.swift new file mode 100644 index 000000000..864fc456f --- /dev/null +++ b/Tests/PrivacyStatsTests/CurrentPackTests.swift @@ -0,0 +1,139 @@ +// +// CurrentPackTests.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Combine +import XCTest +@testable import PrivacyStats + +final class CurrentPackTests: XCTestCase { + var currentPack: CurrentPack! + + override func setUp() async throws { + currentPack = CurrentPack(pack: .init(timestamp: Date().privacyStatsPackTimestamp), commitDebounce: 10_000_000) + } + + func testThatRecordBlockedTrackerUpdatesThePack() async { + await currentPack.recordBlockedTracker("A") + let companyA = await currentPack.pack.trackers["A"] + XCTAssertEqual(companyA, 1) + } + + func testThatRecordBlockedTrackerTriggersCommitChangesEvent() async throws { + let packs = try await waitForCommitChangesEvents(for: 100_000_000) { + await currentPack.recordBlockedTracker("A") + } + + let companyA = await currentPack.pack.trackers["A"] + XCTAssertEqual(companyA, 1) + XCTAssertEqual(packs.first?.trackers["A"], 1) + } + + func testThatMultipleCallsToRecordBlockedTrackerOnlyTriggerOneCommitChangesEvent() async throws { + let packs = try await waitForCommitChangesEvents(for: 100_000_000) { + await currentPack.recordBlockedTracker("A") + await currentPack.recordBlockedTracker("A") + await currentPack.recordBlockedTracker("A") + await currentPack.recordBlockedTracker("A") + await currentPack.recordBlockedTracker("A") + } + + XCTAssertEqual(packs.count, 1) + XCTAssertEqual(packs.first?.trackers["A"], 5) + } + + func testThatRecordBlockedTrackerCalledConcurrentlyForTheSameCompanyStoresAllCalls() async { + await withTaskGroup(of: Void.self) { group in + (0..<1000).forEach { _ in + group.addTask { + await self.currentPack.recordBlockedTracker("A") + } + } + } + let companyA = await currentPack.pack.trackers["A"] + XCTAssertEqual(companyA, 1000) + } + + func testWhenCurrentPackIsOldThenRecordBlockedTrackerSendsCommitEventAndCreatesNewPack() async throws { + let oldTimestamp = Date().privacyStatsPackTimestamp.daysAgo(1) + let pack = PrivacyStatsPack( + timestamp: oldTimestamp, + trackers: ["A": 100, "B": 50, "C": 400] + ) + currentPack = CurrentPack(pack: pack, commitDebounce: 10_000_000) + + + let packs = try await waitForCommitChangesEvents(for: 100_000_000) { + await currentPack.recordBlockedTracker("A") + } + + XCTAssertEqual(packs.count, 2) + let oldPack = try XCTUnwrap(packs.first) + XCTAssertEqual(oldPack, pack) + let newPack = try XCTUnwrap(packs.last) + XCTAssertEqual(newPack, PrivacyStatsPack(timestamp: Date().privacyStatsPackTimestamp, trackers: ["A": 1])) + } + + func testThatResetPackClearsAllRecordedTrackersAndSetsCurrentTimestamp() async { + let oldTimestamp = Date().privacyStatsPackTimestamp.daysAgo(1) + let pack = PrivacyStatsPack( + timestamp: oldTimestamp, + trackers: ["A": 100, "B": 50, "C": 400] + ) + currentPack = CurrentPack(pack: pack, commitDebounce: 10_000_000) + + await currentPack.resetPack() + + let packAfterReset = await currentPack.pack + XCTAssertEqual(packAfterReset, PrivacyStatsPack(timestamp: Date().privacyStatsPackTimestamp, trackers: [:])) + } + + // MARK: - Helpers + + func waitForCommitChangesEvent() async -> PrivacyStatsPack? { + let expectation = self.expectation(description: "commitChanges") + var pack: PrivacyStatsPack? + let cancellable = currentPack.commitChangesPublisher.sink { value in + pack = value + expectation.fulfill() + } + await fulfillment(of: [expectation], timeout: 1) + cancellable.cancel() + return pack + } + + /** + * Sets up Combine subscription, then calls the provided block and then waits + * for the specific time before cancelling the subscription. + * Returns an array of values passed in the published events. + */ + func waitForCommitChangesEvents(for nanoseconds: UInt64, _ block: () async -> Void) async throws -> [PrivacyStatsPack] { + var packs: [PrivacyStatsPack] = [] + let cancellable = currentPack.commitChangesPublisher + .sink { value in + packs.append(value) + print("APPEND \(value.timestamp)") + } + + await block() + + try await Task.sleep(nanoseconds: nanoseconds) + cancellable.cancel() + return packs + } +} diff --git a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift index f1ea9dce4..13eacd01c 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift @@ -190,7 +190,7 @@ final class PrivacyStatsTests: XCTestCase { func waitForStatsUpdateEvent(file: StaticString = #file, line: UInt = #line) async { let expectation = self.expectation(description: "statsUpdate") let cancellable = privacyStats.statsUpdatePublisher.sink { expectation.fulfill() } - await fulfillment(of: [expectation]) + await fulfillment(of: [expectation], timeout: 2) cancellable.cancel() } } diff --git a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift index 80bcd2cb1..d2f0d1510 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift @@ -341,7 +341,6 @@ final class PrivacyStatsUtilsTests: XCTestCase { context.performAndWait { PrivacyStatsUtils.deleteAllStats(in: context) - XCTAssertEqual(context.deletedObjects.count, 5) } } From e21233a9137b408c8b841cbbf1a576ebf2c3ace1 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 16:20:55 +0100 Subject: [PATCH 29/48] Add unit tests for deleting outdated objects --- Sources/PrivacyStats/PrivacyStats.swift | 42 ++++-------- .../PrivacyStats/internal/CurrentPack.swift | 4 +- .../internal/Date+PrivacyStats.swift | 9 +++ .../internal/PrivacyStatsUtils.swift | 4 +- .../PrivacyStatsTests/CurrentPackTests.swift | 10 +-- .../PrivacyStatsTests/PrivacyStatsTests.swift | 64 +++++++++++++++++++ .../PrivacyStatsUtilsTests.swift | 2 +- .../TestPrivacyStatsDatabaseProvider.swift | 9 ++- 8 files changed, 103 insertions(+), 41 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index bd55e0925..1163a923c 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -34,7 +34,6 @@ import UIKit public enum PrivacyStatsError: CustomNSError { case failedToFetchPrivacyStatsSummary(Error) case failedToStorePrivacyStats(Error) - case failedToClearPrivacyStats(Error) case failedToLoadCurrentPrivacyStats(Error) public static let errorDomain: String = "PrivacyStatsError" @@ -45,10 +44,8 @@ public enum PrivacyStatsError: CustomNSError { return 1 case .failedToStorePrivacyStats: return 2 - case .failedToClearPrivacyStats: - return 3 case .failedToLoadCurrentPrivacyStats: - return 4 + return 3 } } @@ -56,7 +53,6 @@ public enum PrivacyStatsError: CustomNSError { switch self { case .failedToFetchPrivacyStatsSummary(let error), .failedToStorePrivacyStats(let error), - .failedToClearPrivacyStats(let error), .failedToLoadCurrentPrivacyStats(let error): return error } @@ -175,6 +171,12 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } + // Delete outdated packs if the pack we're storing is from a previous day. + // This means that it's a new day and we may have outdated packs. + if pack.timestamp < Date.currentPrivacyStatsPackTimestamp { + PrivacyStatsUtils.deleteOutdatedPacks(in: context) + } + guard context.hasChanges else { continuation.resume() return @@ -192,43 +194,23 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } - private func deleteOldEntries() async { - await withCheckedContinuation { continuation in - context.perform { [weak self] in - guard let self else { - continuation.resume() - return - } - - PrivacyStatsUtils.deleteOutdatedPacks(in: context) - if context.hasChanges { - do { - try context.save() - Logger.privacyStats.debug("Deleted outdated entries") - } catch { - Logger.privacyStats.error("Save error: \(error)") - errorEvents?.fire(.failedToClearPrivacyStats(error)) - } - } - continuation.resume() - } - } - } - private func initializeCurrentPack() -> PrivacyStatsPack { var pack: PrivacyStatsPack? context.performAndWait { - let timestamp = Date().privacyStatsPackTimestamp + let timestamp = Date.currentPrivacyStatsPackTimestamp do { let currentDayStats = try PrivacyStatsUtils.loadCurrentDayStats(in: context) Logger.privacyStats.debug("Loaded stats \(timestamp) \(currentDayStats)") pack = PrivacyStatsPack(timestamp: timestamp, trackers: currentDayStats) + + PrivacyStatsUtils.deleteOutdatedPacks(in: context) + try context.save() } catch { Logger.privacyStats.error("Faild to load current stats: \(error)") errorEvents?.fire(.failedToLoadCurrentPrivacyStats(error)) } } - return pack ?? PrivacyStatsPack(timestamp: Date().privacyStatsPackTimestamp) + return pack ?? PrivacyStatsPack(timestamp: Date.currentPrivacyStatsPackTimestamp) } @objc private func applicationWillTerminate(_: Notification) { diff --git a/Sources/PrivacyStats/internal/CurrentPack.swift b/Sources/PrivacyStats/internal/CurrentPack.swift index f7bfe5bfe..3fa0b3e95 100644 --- a/Sources/PrivacyStats/internal/CurrentPack.swift +++ b/Sources/PrivacyStats/internal/CurrentPack.swift @@ -56,7 +56,7 @@ actor CurrentPack { * It sets a new empty pack with the current timestamp. */ func resetPack() { - resetStats(andSet: Date().privacyStatsPackTimestamp) + resetStats(andSet: Date.currentPrivacyStatsPackTimestamp) } /** @@ -70,7 +70,7 @@ actor CurrentPack { */ func recordBlockedTracker(_ companyName: String) { - let currentTimestamp = Date().privacyStatsPackTimestamp + let currentTimestamp = Date.currentPrivacyStatsPackTimestamp if currentTimestamp != pack.timestamp { Logger.privacyStats.debug("New timestamp detected, storing trackers state and creating new pack") commitChangesSubject.send(pack) diff --git a/Sources/PrivacyStats/internal/Date+PrivacyStats.swift b/Sources/PrivacyStats/internal/Date+PrivacyStats.swift index 017ef2d94..f56072d75 100644 --- a/Sources/PrivacyStats/internal/Date+PrivacyStats.swift +++ b/Sources/PrivacyStats/internal/Date+PrivacyStats.swift @@ -21,6 +21,15 @@ import Foundation extension Date { + /** + * Returns privacy stats pack timestamp for the current date. + * + * See `privacyStatsPackTimestamp`. + */ + static var currentPrivacyStatsPackTimestamp: Date { + Date().privacyStatsPackTimestamp + } + /** * Returns a valid timestamp for `DailyBlockedTrackersEntity` instance matching the sender. * diff --git a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift index efa515b3d..b191c625d 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift @@ -35,7 +35,7 @@ final class PrivacyStatsUtils { * timestamp's day matches current day. */ static func fetchOrInsertCurrentStats(for companyNames: Set, in context: NSManagedObjectContext) throws -> [DailyBlockedTrackersEntity] { - let timestamp = Date().privacyStatsPackTimestamp + let timestamp = Date.currentPrivacyStatsPackTimestamp let request = DailyBlockedTrackersEntity.fetchRequest() request.predicate = NSPredicate(format: "%K == %@ AND %K in %@", @@ -56,7 +56,7 @@ final class PrivacyStatsUtils { * Returns a dictionary representation of blocked trackers counts grouped by company name for the current day. */ static func loadCurrentDayStats(in context: NSManagedObjectContext) throws -> [String: Int64] { - let startDate = Date().privacyStatsPackTimestamp + let startDate = Date.currentPrivacyStatsPackTimestamp return try loadBlockedTrackersStats(since: startDate, in: context) } diff --git a/Tests/PrivacyStatsTests/CurrentPackTests.swift b/Tests/PrivacyStatsTests/CurrentPackTests.swift index 864fc456f..63e0d57d8 100644 --- a/Tests/PrivacyStatsTests/CurrentPackTests.swift +++ b/Tests/PrivacyStatsTests/CurrentPackTests.swift @@ -25,7 +25,7 @@ final class CurrentPackTests: XCTestCase { var currentPack: CurrentPack! override func setUp() async throws { - currentPack = CurrentPack(pack: .init(timestamp: Date().privacyStatsPackTimestamp), commitDebounce: 10_000_000) + currentPack = CurrentPack(pack: .init(timestamp: Date.currentPrivacyStatsPackTimestamp), commitDebounce: 10_000_000) } func testThatRecordBlockedTrackerUpdatesThePack() async { @@ -70,7 +70,7 @@ final class CurrentPackTests: XCTestCase { } func testWhenCurrentPackIsOldThenRecordBlockedTrackerSendsCommitEventAndCreatesNewPack() async throws { - let oldTimestamp = Date().privacyStatsPackTimestamp.daysAgo(1) + let oldTimestamp = Date.currentPrivacyStatsPackTimestamp.daysAgo(1) let pack = PrivacyStatsPack( timestamp: oldTimestamp, trackers: ["A": 100, "B": 50, "C": 400] @@ -86,11 +86,11 @@ final class CurrentPackTests: XCTestCase { let oldPack = try XCTUnwrap(packs.first) XCTAssertEqual(oldPack, pack) let newPack = try XCTUnwrap(packs.last) - XCTAssertEqual(newPack, PrivacyStatsPack(timestamp: Date().privacyStatsPackTimestamp, trackers: ["A": 1])) + XCTAssertEqual(newPack, PrivacyStatsPack(timestamp: Date.currentPrivacyStatsPackTimestamp, trackers: ["A": 1])) } func testThatResetPackClearsAllRecordedTrackersAndSetsCurrentTimestamp() async { - let oldTimestamp = Date().privacyStatsPackTimestamp.daysAgo(1) + let oldTimestamp = Date.currentPrivacyStatsPackTimestamp.daysAgo(1) let pack = PrivacyStatsPack( timestamp: oldTimestamp, trackers: ["A": 100, "B": 50, "C": 400] @@ -100,7 +100,7 @@ final class CurrentPackTests: XCTestCase { await currentPack.resetPack() let packAfterReset = await currentPack.pack - XCTAssertEqual(packAfterReset, PrivacyStatsPack(timestamp: Date().privacyStatsPackTimestamp, trackers: [:])) + XCTAssertEqual(packAfterReset, PrivacyStatsPack(timestamp: Date.currentPrivacyStatsPackTimestamp, trackers: [:])) } // MARK: - Helpers diff --git a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift index 13eacd01c..85e201889 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift @@ -35,6 +35,39 @@ final class PrivacyStatsTests: XCTestCase { databaseProvider.tearDownDatabase() } + // MARK: - initializer + + func testThatOutdatedTrackerStatsAreDeletedUponInitialization() async throws { + try databaseProvider.addObjects { context in + let date = Date() + + return [ + DailyBlockedTrackersEntity.make(timestamp: date, companyName: "A", count: 1, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(1), companyName: "A", count: 2, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(6), companyName: "A", count: 7, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(7), companyName: "A", count: 100, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(8), companyName: "A", count: 100, context: context) + ] + } + + // recreate database provider with existing location so that the existing database is persisted in the initializer + databaseProvider = TestPrivacyStatsDatabaseProvider(databaseName: type(of: self).description(), location: databaseProvider.location) + privacyStats = PrivacyStats(databaseProvider: databaseProvider) + + let stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 10]) + + let context = databaseProvider.database.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + do { + let allObjects = try context.fetch(DailyBlockedTrackersEntity.fetchRequest()) + XCTAssertEqual(Set(allObjects.map(\.count)), [1, 2, 7]) + } catch { + XCTFail("Context fetch should not fail") + } + } + } + // MARK: - fetchPrivacyStats func testThatPrivacyStatsAreFetched() async throws { @@ -150,6 +183,37 @@ final class PrivacyStatsTests: XCTestCase { XCTAssertEqual(stats, ["A": 5, "B": 10, "C": 3]) } + func testThatCallingRecordBlockedTrackerWithNextDayTimestampCausesDeletingOldEntriesFromDatabase() async throws { + try databaseProvider.addObjects { context in + let date = Date() + return [ + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(1), companyName: "A", count: 2, context: context), + DailyBlockedTrackersEntity.make(timestamp: date.daysAgo(7), companyName: "A", count: 100, context: context), + ] + } + + // recreate database provider with existing location so that the existing database is persisted in the initializer + databaseProvider = TestPrivacyStatsDatabaseProvider(databaseName: type(of: self).description(), location: databaseProvider.location) + privacyStats = PrivacyStats(databaseProvider: databaseProvider) + + await privacyStats.recordBlockedTracker("A") + + try await Task.sleep(nanoseconds: 1_100_000_000) + + let stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 3]) // 2 from yesterday and 1 from today + + let context = databaseProvider.database.makeContext(concurrencyType: .privateQueueConcurrencyType) + context.performAndWait { + do { + let allObjects = try context.fetch(DailyBlockedTrackersEntity.fetchRequest()) + XCTAssertEqual(Set(allObjects.map(\.count)), [1, 2]) // 7 day ago entry got deleted + } catch { + XCTFail("Context fetch should not fail") + } + } + } + // MARK: - clearPrivacyStats func testWhenClearPrivacyStatsIsCalledThenFetchPrivacyStatsIsEmpty() async throws { diff --git a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift index d2f0d1510..4f554e7c2 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift @@ -40,7 +40,7 @@ final class PrivacyStatsUtilsTests: XCTestCase { func testWhenThereAreNoObjectsForCompaniesThenFetchOrInsertCurrentStatsInsertsNewObjects() { let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) context.performAndWait { - let currentPackTimestamp = Date().privacyStatsPackTimestamp + let currentPackTimestamp = Date.currentPrivacyStatsPackTimestamp let companyNames: Set = ["A", "B", "C", "D"] var returnedEntities: [DailyBlockedTrackersEntity] = [] diff --git a/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift b/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift index 0d4ee8c06..c5d093c33 100644 --- a/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift +++ b/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift @@ -30,9 +30,16 @@ final class TestPrivacyStatsDatabaseProvider: PrivacyStatsDatabaseProviding { self.databaseName = databaseName } + init(databaseName: String, location: URL) { + self.databaseName = databaseName + self.location = location + } + @discardableResult func initializeDatabase() -> CoreDataDatabase { - location = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + if location == nil { + location = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + } let model = CoreDataDatabase.loadModel(from: PrivacyStats.bundle, named: "PrivacyStats")! database = CoreDataDatabase(name: databaseName, containerLocation: location, model: model) database.loadStore() From 02e4e2ecadaad159ad9b2ac25ce40e4c64058cc2 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 16:48:07 +0100 Subject: [PATCH 30/48] Add more docs and fix SwiftLint violations --- Sources/PrivacyStats/PrivacyStats.swift | 52 +++++++++++++++---- .../PrivacyStats/internal/CurrentPack.swift | 1 - .../PrivacyStatsTests/CurrentPackTests.swift | 2 - .../PrivacyStatsUtilsTests.swift | 1 - .../TestPrivacyStatsDatabaseProvider.swift | 1 - 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 1163a923c..b6301db4b 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -59,15 +59,41 @@ public enum PrivacyStatsError: CustomNSError { } } +/** + * This protocol describes database provider consumed by `PrivacyStats`. + */ public protocol PrivacyStatsDatabaseProviding { func initializeDatabase() -> CoreDataDatabase } +/** + * This protocol describes `PrivacyStats` interface. + */ public protocol PrivacyStatsCollecting { + + /** + * Record a tracker for a given `companyName`. + * + * `PrivacyStats` implementation calls the actor under the hood, + * and as such it can safely be called on multiple threads concurrently. + */ func recordBlockedTracker(_ name: String) async + /** + * Publisher emitting values whenever updated privacy stats were persisted to disk. + */ var statsUpdatePublisher: AnyPublisher { get } + + /** + * This function fetches privacy stats in a dictionary format + * with keys being company names and values being total number + * of tracking attempts blocked in past 7 days. + */ func fetchPrivacyStats() async -> [String: Int64] + + /** + * This function clears all blocked tracker stats from the database. + */ func clearPrivacyStats() async } @@ -101,17 +127,11 @@ public final class PrivacyStats: PrivacyStatsCollecting { } .store(in: &cancellables) -#if os(iOS) - let notificationName = UIApplication.willTerminateNotification -#elseif os(macOS) - let notificationName = NSApplication.willTerminateNotification -#endif - NotificationCenter.default.addObserver(self, selector: #selector(applicationWillTerminate(_:)), name: notificationName, object: nil) - + subscribeToAppTermination() } - public func recordBlockedTracker(_ name: String) async { - await currentPack?.recordBlockedTracker(name) + public func recordBlockedTracker(_ companyName: String) async { + await currentPack?.recordBlockedTracker(companyName) } public func fetchPrivacyStats() async -> [String: Int64] { @@ -194,6 +214,11 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } + /** + * This function is only called in the initializer. It performs a blocking call to the database + * to spare us the hassle of declaring the initializer async or spawning tasks from within the + * initializer without being able to await them, thus making testing trickier. + */ private func initializeCurrentPack() -> PrivacyStatsPack { var pack: PrivacyStatsPack? context.performAndWait { @@ -213,6 +238,15 @@ public final class PrivacyStats: PrivacyStatsCollecting { return pack ?? PrivacyStatsPack(timestamp: Date.currentPrivacyStatsPackTimestamp) } + private func subscribeToAppTermination() { +#if os(iOS) + let notificationName = UIApplication.willTerminateNotification +#elseif os(macOS) + let notificationName = NSApplication.willTerminateNotification +#endif + NotificationCenter.default.addObserver(self, selector: #selector(applicationWillTerminate(_:)), name: notificationName, object: nil) + } + @objc private func applicationWillTerminate(_: Notification) { let condition = RunLoop.ResumeCondition() Task { diff --git a/Sources/PrivacyStats/internal/CurrentPack.swift b/Sources/PrivacyStats/internal/CurrentPack.swift index 3fa0b3e95..f506a0ea4 100644 --- a/Sources/PrivacyStats/internal/CurrentPack.swift +++ b/Sources/PrivacyStats/internal/CurrentPack.swift @@ -1,6 +1,5 @@ // // CurrentPack.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. // diff --git a/Tests/PrivacyStatsTests/CurrentPackTests.swift b/Tests/PrivacyStatsTests/CurrentPackTests.swift index 63e0d57d8..20578b192 100644 --- a/Tests/PrivacyStatsTests/CurrentPackTests.swift +++ b/Tests/PrivacyStatsTests/CurrentPackTests.swift @@ -1,6 +1,5 @@ // // CurrentPackTests.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. // @@ -77,7 +76,6 @@ final class CurrentPackTests: XCTestCase { ) currentPack = CurrentPack(pack: pack, commitDebounce: 10_000_000) - let packs = try await waitForCommitChangesEvents(for: 100_000_000) { await currentPack.recordBlockedTracker("A") } diff --git a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift index 4f554e7c2..db2c37f31 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift @@ -1,6 +1,5 @@ // // PrivacyStatsUtilsTests.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. // diff --git a/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift b/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift index c5d093c33..2cb210f0b 100644 --- a/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift +++ b/Tests/PrivacyStatsTests/TestPrivacyStatsDatabaseProvider.swift @@ -1,6 +1,5 @@ // // TestPrivacyStatsDatabaseProvider.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. // From a10132accf5668b4e2ba4b5a0795fed79ee6bdfc Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 16:59:10 +0100 Subject: [PATCH 31/48] Fix flaky test --- Tests/PrivacyStatsTests/PrivacyStatsTests.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift index 85e201889..0a2fa0fec 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift @@ -134,7 +134,7 @@ final class PrivacyStatsTests: XCTestCase { var stats = await privacyStats.fetchPrivacyStats() XCTAssertEqual(stats, [:]) - try await Task.sleep(nanoseconds: 1_100_000_000) + try await Task.sleep(nanoseconds: 1_500_000_000) stats = await privacyStats.fetchPrivacyStats() XCTAssertEqual(stats, ["A": 1]) @@ -198,16 +198,17 @@ final class PrivacyStatsTests: XCTestCase { await privacyStats.recordBlockedTracker("A") - try await Task.sleep(nanoseconds: 1_100_000_000) + // No Task.sleep because the commit event will be sent immediately from the actor when pack's timestamp changes. + // We aren't testing the debounced commit in this test case. let stats = await privacyStats.fetchPrivacyStats() - XCTAssertEqual(stats, ["A": 3]) // 2 from yesterday and 1 from today + XCTAssertEqual(stats, ["A": 2]) let context = databaseProvider.database.makeContext(concurrencyType: .privateQueueConcurrencyType) context.performAndWait { do { let allObjects = try context.fetch(DailyBlockedTrackersEntity.fetchRequest()) - XCTAssertEqual(Set(allObjects.map(\.count)), [1, 2]) // 7 day ago entry got deleted + XCTAssertEqual(Set(allObjects.map(\.count)), [2]) } catch { XCTFail("Context fetch should not fail") } From c19d9d45a83164d0c564b3ddf73cda956ba5de8b Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 17:11:50 +0100 Subject: [PATCH 32/48] Cancel commit task on CurrentPack deinit --- Sources/PrivacyStats/internal/CurrentPack.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/PrivacyStats/internal/CurrentPack.swift b/Sources/PrivacyStats/internal/CurrentPack.swift index f506a0ea4..0d879248c 100644 --- a/Sources/PrivacyStats/internal/CurrentPack.swift +++ b/Sources/PrivacyStats/internal/CurrentPack.swift @@ -49,6 +49,10 @@ actor CurrentPack { self.commitDebounce = commitDebounce } + deinit { + commitTask?.cancel() + } + /** * This function is used when clearing app data, to clear any stats cached in memory. * From f1cd8fc4fcd9cfcb47bebb75dc64644682ad33b4 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 17:18:58 +0100 Subject: [PATCH 33/48] Another fix --- Sources/PrivacyStats/PrivacyStats.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index b6301db4b..6945e9def 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -178,7 +178,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { private func commitChanges(_ pack: PrivacyStatsPack) async { await withCheckedContinuation { continuation in context.perform { [weak self] in - guard let self else { + guard let self, context.persistentStoreCoordinator?.persistentStores.isEmpty == false else { continuation.resume() return } From 0ab96176a72405e637aae7fe192f8dd36b64833f Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 17:57:27 +0100 Subject: [PATCH 34/48] Don't notify about change when saving current day pack --- Sources/PrivacyStats/PrivacyStats.swift | 17 +++++++--- .../PrivacyStats/internal/CurrentPack.swift | 34 +++++++++++++------ .../PrivacyStatsTests/PrivacyStatsTests.swift | 8 +++-- 3 files changed, 41 insertions(+), 18 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 6945e9def..c14318bf9 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -178,11 +178,14 @@ public final class PrivacyStats: PrivacyStatsCollecting { private func commitChanges(_ pack: PrivacyStatsPack) async { await withCheckedContinuation { continuation in context.perform { [weak self] in - guard let self, context.persistentStoreCoordinator?.persistentStores.isEmpty == false else { + guard let self else { continuation.resume() return } + // Check if the pack we're currently storing is from a previous day. + let isCurrentDayPack = pack.timestamp == Date.currentPrivacyStatsPackTimestamp + do { let statsObjects = try PrivacyStatsUtils.fetchOrInsertCurrentStats(for: Set(pack.trackers.keys), in: context) statsObjects.forEach { stats in @@ -191,9 +194,8 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } - // Delete outdated packs if the pack we're storing is from a previous day. - // This means that it's a new day and we may have outdated packs. - if pack.timestamp < Date.currentPrivacyStatsPackTimestamp { + // When storing a pack from a previous day, we may have outdated packs, so delete them as needed. + if !isCurrentDayPack { PrivacyStatsUtils.deleteOutdatedPacks(in: context) } @@ -204,7 +206,12 @@ public final class PrivacyStats: PrivacyStatsCollecting { try context.save() Logger.privacyStats.debug("Saved stats \(pack.timestamp) \(pack.trackers)") - statsUpdateSubject.send() + + // Only emit update event when saving current-day pack. For previous-day pack, + // a follow-up commit event will come and we'll emit the update then. + if isCurrentDayPack { + statsUpdateSubject.send() + } } catch { Logger.privacyStats.error("Save error: \(error)") errorEvents?.fire(.failedToStorePrivacyStats(error)) diff --git a/Sources/PrivacyStats/internal/CurrentPack.swift b/Sources/PrivacyStats/internal/CurrentPack.swift index 0d879248c..fffc6b090 100644 --- a/Sources/PrivacyStats/internal/CurrentPack.swift +++ b/Sources/PrivacyStats/internal/CurrentPack.swift @@ -76,24 +76,36 @@ actor CurrentPack { let currentTimestamp = Date.currentPrivacyStatsPackTimestamp if currentTimestamp != pack.timestamp { Logger.privacyStats.debug("New timestamp detected, storing trackers state and creating new pack") - commitChangesSubject.send(pack) + notifyChanges(for: pack, immediately: true) resetStats(andSet: currentTimestamp) } let count = pack.trackers[companyName] ?? 0 pack.trackers[companyName] = count + 1 + notifyChanges(for: pack, immediately: false) + } + + private func notifyChanges(for pack: PrivacyStatsPack, immediately shouldPublishImmediately: Bool) { commitTask?.cancel() - commitTask = Task { - do { - // Note that this doesn't always sleep for the full debounce time, but the sleep is interrupted - // as soon as the task gets cancelled. - try await Task.sleep(nanoseconds: commitDebounce) - - Logger.privacyStats.debug("Storing trackers state") - commitChangesSubject.send(pack) - } catch { - // Commit task got cancelled + + if shouldPublishImmediately { + + commitChangesSubject.send(pack) + + } else { + + commitTask = Task { + do { + // Note that this doesn't always sleep for the full debounce time, but the sleep is interrupted + // as soon as the task gets cancelled. + try await Task.sleep(nanoseconds: commitDebounce) + + Logger.privacyStats.debug("Storing trackers state") + commitChangesSubject.send(pack) + } catch { + // Commit task got cancelled + } } } } diff --git a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift index 0a2fa0fec..44b78e233 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift @@ -198,10 +198,10 @@ final class PrivacyStatsTests: XCTestCase { await privacyStats.recordBlockedTracker("A") - // No Task.sleep because the commit event will be sent immediately from the actor when pack's timestamp changes. + // No waiting here because the first commit event will be sent immediately from the actor when pack's timestamp changes. // We aren't testing the debounced commit in this test case. - let stats = await privacyStats.fetchPrivacyStats() + var stats = await privacyStats.fetchPrivacyStats() XCTAssertEqual(stats, ["A": 2]) let context = databaseProvider.database.makeContext(concurrencyType: .privateQueueConcurrencyType) @@ -213,6 +213,10 @@ final class PrivacyStatsTests: XCTestCase { XCTFail("Context fetch should not fail") } } + + await waitForStatsUpdateEvent() + stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 3]) } // MARK: - clearPrivacyStats From 25bc3e1e670cbdf5082524a8a87c721d680ecb98 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 28 Nov 2024 20:43:54 +0100 Subject: [PATCH 35/48] Remove unneeded code from CurrentPackTests --- Tests/PrivacyStatsTests/CurrentPackTests.swift | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/Tests/PrivacyStatsTests/CurrentPackTests.swift b/Tests/PrivacyStatsTests/CurrentPackTests.swift index 20578b192..07bc634e2 100644 --- a/Tests/PrivacyStatsTests/CurrentPackTests.swift +++ b/Tests/PrivacyStatsTests/CurrentPackTests.swift @@ -103,18 +103,6 @@ final class CurrentPackTests: XCTestCase { // MARK: - Helpers - func waitForCommitChangesEvent() async -> PrivacyStatsPack? { - let expectation = self.expectation(description: "commitChanges") - var pack: PrivacyStatsPack? - let cancellable = currentPack.commitChangesPublisher.sink { value in - pack = value - expectation.fulfill() - } - await fulfillment(of: [expectation], timeout: 1) - cancellable.cancel() - return pack - } - /** * Sets up Combine subscription, then calls the provided block and then waits * for the specific time before cancelling the subscription. @@ -122,11 +110,7 @@ final class CurrentPackTests: XCTestCase { */ func waitForCommitChangesEvents(for nanoseconds: UInt64, _ block: () async -> Void) async throws -> [PrivacyStatsPack] { var packs: [PrivacyStatsPack] = [] - let cancellable = currentPack.commitChangesPublisher - .sink { value in - packs.append(value) - print("APPEND \(value.timestamp)") - } + let cancellable = currentPack.commitChangesPublisher.sink { packs.append($0) } await block() From b51633ff01fd824bed0b6e3500d5b9384fde419a Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 10:06:08 +0100 Subject: [PATCH 36/48] Update C-S-S to 6.41.0 --- Package.resolved | 4 ++-- Package.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Package.resolved b/Package.resolved index 7a4755e08..5a1dbee07 100644 --- a/Package.resolved +++ b/Package.resolved @@ -14,8 +14,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/content-scope-scripts", "state" : { - "revision" : "e51efbca45d2178c43dd3b0fe4d18d19b34c54e3", - "version" : "6.40.0" + "revision" : "c4bb146afdf0c7a93fb9a7d95b1cb255708a470d", + "version" : "6.41.0" } }, { diff --git a/Package.swift b/Package.swift index 4cd60a919..ac361e7db 100644 --- a/Package.swift +++ b/Package.swift @@ -54,7 +54,7 @@ let package = Package( .package(url: "https://github.com/duckduckgo/TrackerRadarKit", exact: "3.0.0"), .package(url: "https://github.com/duckduckgo/sync_crypto", exact: "0.3.0"), .package(url: "https://github.com/gumob/PunycodeSwift.git", exact: "3.0.0"), - .package(url: "https://github.com/duckduckgo/content-scope-scripts", exact: "6.40.0"), + .package(url: "https://github.com/duckduckgo/content-scope-scripts", exact: "6.41.0"), .package(url: "https://github.com/duckduckgo/privacy-dashboard", exact: "7.2.0"), .package(url: "https://github.com/httpswift/swifter.git", exact: "1.5.0"), .package(url: "https://github.com/duckduckgo/bloom_cpp.git", exact: "3.0.0"), From d50db7e0e3fcac25036c80d325cfaf5efc6ab1b8 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 12:21:40 +0100 Subject: [PATCH 37/48] Update Sources/Common/Extensions/DateExtension.swift Co-authored-by: Pete Smith <5278441+aataraxiaa@users.noreply.github.com> --- Sources/Common/Extensions/DateExtension.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Common/Extensions/DateExtension.swift b/Sources/Common/Extensions/DateExtension.swift index 50003675e..aff76e959 100644 --- a/Sources/Common/Extensions/DateExtension.swift +++ b/Sources/Common/Extensions/DateExtension.swift @@ -72,7 +72,7 @@ public extension Date { } func daysAgo(_ days: Int) -> Date { - return Calendar.current.date(byAdding: .day, value: -days, to: self)! + Calendar.current.date(byAdding: .day, value: -days, to: self)! } static var startOfMinuteNow: Date { From 9041b8a1483e88b16033e76beddda52891f57d24 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 12:22:25 +0100 Subject: [PATCH 38/48] Remove Date.isSameHour --- Sources/Common/Extensions/DateExtension.swift | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Sources/Common/Extensions/DateExtension.swift b/Sources/Common/Extensions/DateExtension.swift index aff76e959..2a4ca74ea 100644 --- a/Sources/Common/Extensions/DateExtension.swift +++ b/Sources/Common/Extensions/DateExtension.swift @@ -54,10 +54,6 @@ public extension Date { return Calendar.current.isDate(date1, inSameDayAs: date2) } - static func isSameHour(_ date1: Date, _ date2: Date) -> Bool { - return Calendar.current.isDate(date1, equalTo: date2, toGranularity: .hour) - } - static var startOfDayTomorrow: Date { let tomorrow = Calendar.current.date(byAdding: .day, value: 1, to: Date())! return Calendar.current.startOfDay(for: tomorrow) From 46897e6d49c815cc696ab7bdc8fc2623c2811af8 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 12:32:53 +0100 Subject: [PATCH 39/48] Extract entity name into a constant --- .../PrivacyStats/internal/DailyBlockedTrackersEntity.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift b/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift index d785d295e..3a78d51c3 100644 --- a/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift +++ b/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift @@ -21,13 +21,16 @@ import CoreData @objc(DailyBlockedTrackersEntity) final class DailyBlockedTrackersEntity: NSManagedObject { + enum Const { + static let entityName = "DailyBlockedTrackersEntity" + } @nonobjc class func fetchRequest() -> NSFetchRequest { - return NSFetchRequest(entityName: "DailyBlockedTrackersEntity") + return NSFetchRequest(entityName: Const.entityName) } class func entity(in context: NSManagedObjectContext) -> NSEntityDescription { - return NSEntityDescription.entity(forEntityName: "DailyBlockedTrackersEntity", in: context)! + return NSEntityDescription.entity(forEntityName: Const.entityName, in: context)! } @NSManaged var companyName: String From 184595e7dce172f890389f57550abbafab46cb62 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 12:33:19 +0100 Subject: [PATCH 40/48] Remove unnecessary return statements --- .../PrivacyStats/internal/DailyBlockedTrackersEntity.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift b/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift index 3a78d51c3..728a5943c 100644 --- a/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift +++ b/Sources/PrivacyStats/internal/DailyBlockedTrackersEntity.swift @@ -26,11 +26,11 @@ final class DailyBlockedTrackersEntity: NSManagedObject { } @nonobjc class func fetchRequest() -> NSFetchRequest { - return NSFetchRequest(entityName: Const.entityName) + NSFetchRequest(entityName: Const.entityName) } class func entity(in context: NSManagedObjectContext) -> NSEntityDescription { - return NSEntityDescription.entity(forEntityName: Const.entityName, in: context)! + NSEntityDescription.entity(forEntityName: Const.entityName, in: context)! } @NSManaged var companyName: String From 6f605ec9a7bfca654931f39af67c33caca46bbb2 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 12:44:59 +0100 Subject: [PATCH 41/48] Use NSBatchDeleteRequest for deleting objects --- Sources/PrivacyStats/PrivacyStats.swift | 8 ++--- .../internal/PrivacyStatsUtils.swift | 17 ++++++---- .../PrivacyStatsUtilsTests.swift | 32 +++++++++++++------ 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index c14318bf9..233fd74ce 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -159,9 +159,8 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume() return } - PrivacyStatsUtils.deleteAllStats(in: context) do { - try context.save() + try PrivacyStatsUtils.deleteAllStats(in: context) Logger.privacyStats.debug("Deleted outdated entries") } catch { Logger.privacyStats.error("Save error: \(error)") @@ -196,7 +195,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { // When storing a pack from a previous day, we may have outdated packs, so delete them as needed. if !isCurrentDayPack { - PrivacyStatsUtils.deleteOutdatedPacks(in: context) + try PrivacyStatsUtils.deleteOutdatedPacks(in: context) } guard context.hasChanges else { @@ -235,8 +234,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { Logger.privacyStats.debug("Loaded stats \(timestamp) \(currentDayStats)") pack = PrivacyStatsPack(timestamp: timestamp, trackers: currentDayStats) - PrivacyStatsUtils.deleteOutdatedPacks(in: context) - try context.save() + try PrivacyStatsUtils.deleteOutdatedPacks(in: context) } catch { Logger.privacyStats.error("Faild to load current stats: \(error)") errorEvents?.fire(.failedToLoadCurrentPrivacyStats(error)) diff --git a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift index b191c625d..d6ceb3032 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift @@ -69,7 +69,7 @@ final class PrivacyStatsUtils { } private static func loadBlockedTrackersStats(since startDate: Date, in context: NSManagedObjectContext) throws -> [String: Int64] { - let request = NSFetchRequest(entityName: "DailyBlockedTrackersEntity") + let request = NSFetchRequest(entityName: DailyBlockedTrackersEntity.Const.entityName) request.predicate = NSPredicate(format: "%K >= %@", #keyPath(DailyBlockedTrackersEntity.timestamp), startDate as NSDate) let companyNameKey = #keyPath(DailyBlockedTrackersEntity.companyName) @@ -101,18 +101,21 @@ final class PrivacyStatsUtils { /** * Deletes stats older than 7 days for all companies. */ - static func deleteOutdatedPacks(in context: NSManagedObjectContext) { + static func deleteOutdatedPacks(in context: NSManagedObjectContext) throws { let oldestValidTimestamp = Date().privacyStatsOldestPackTimestamp - let request = DailyBlockedTrackersEntity.fetchRequest() - request.predicate = NSPredicate(format: "%K < %@", #keyPath(DailyBlockedTrackersEntity.timestamp), oldestValidTimestamp as NSDate) - context.deleteAll(matching: request) + let fetchRequest = NSFetchRequest(entityName: DailyBlockedTrackersEntity.Const.entityName) + fetchRequest.predicate = NSPredicate(format: "%K < %@", #keyPath(DailyBlockedTrackersEntity.timestamp), oldestValidTimestamp as NSDate) + let deleteRequest = NSBatchDeleteRequest(fetchRequest: fetchRequest) + + try context.execute(deleteRequest) } /** * Deletes all stats entries in the database. */ - static func deleteAllStats(in context: NSManagedObjectContext) { - context.deleteAll(matching: DailyBlockedTrackersEntity.fetchRequest()) + static func deleteAllStats(in context: NSManagedObjectContext) throws { + let deleteRequest = NSBatchDeleteRequest(fetchRequest: DailyBlockedTrackersEntity.fetchRequest()) + try context.execute(deleteRequest) } } diff --git a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift index db2c37f31..ec0e7e606 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsUtilsTests.swift @@ -293,12 +293,14 @@ final class PrivacyStatsUtilsTests: XCTestCase { let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) context.performAndWait { - PrivacyStatsUtils.deleteOutdatedPacks(in: context) - - let deletedObjects = context.deletedObjects.compactMap { $0 as? DailyBlockedTrackersEntity } + do { + try PrivacyStatsUtils.deleteOutdatedPacks(in: context) - XCTAssertEqual(deletedObjects.count, 3) - XCTAssertEqual(Set(deletedObjects.map(\.count)), [4, 5, 6]) + let allObjects = try context.fetch(DailyBlockedTrackersEntity.fetchRequest()) + XCTAssertEqual(Set(allObjects.map(\.count)), [1, 2, 3]) + } catch { + XCTFail("Should not throw") + } } } @@ -316,8 +318,14 @@ final class PrivacyStatsUtilsTests: XCTestCase { let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) context.performAndWait { - PrivacyStatsUtils.deleteOutdatedPacks(in: context) - XCTAssertFalse(context.hasChanges) + do { + try PrivacyStatsUtils.deleteOutdatedPacks(in: context) + + let allObjects = try context.fetch(DailyBlockedTrackersEntity.fetchRequest()) + XCTAssertEqual(allObjects.count, 3) + } catch { + XCTFail("Should not throw") + } } } @@ -339,8 +347,14 @@ final class PrivacyStatsUtilsTests: XCTestCase { let context = database.makeContext(concurrencyType: .privateQueueConcurrencyType) context.performAndWait { - PrivacyStatsUtils.deleteAllStats(in: context) - XCTAssertEqual(context.deletedObjects.count, 5) + do { + try PrivacyStatsUtils.deleteAllStats(in: context) + + let allObjects = try context.fetch(DailyBlockedTrackersEntity.fetchRequest()) + XCTAssertTrue(allObjects.isEmpty) + } catch { + XCTFail("Should not throw") + } } } } From 661849c7653290660ab33aa62e54c4b6521c5bfc Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 12:55:15 +0100 Subject: [PATCH 42/48] Replace handling app termination with a dedicated function to be used by clients --- Sources/PrivacyStats/PrivacyStats.swift | 43 +++++++------------ .../PrivacyStatsTests/PrivacyStatsTests.swift | 30 +++++++++++++ 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 233fd74ce..7f401b404 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -18,15 +18,11 @@ import Combine import Common +import CoreData import Foundation import os.log import Persistence import TrackerRadarKit -#if os(macOS) -import AppKit -#elseif os(iOS) -import UIKit -#endif /** * Errors that may be reported by `PrivacyStats`. @@ -95,6 +91,14 @@ public protocol PrivacyStatsCollecting { * This function clears all blocked tracker stats from the database. */ func clearPrivacyStats() async + + /** + * This function saves all pending changes to the persistent storage. + * + * It should only be used in response to app termination because otherwise + * the `PrivacyStats` object schedules persisting internally. + */ + func handleAppTermination() async } public final class PrivacyStats: PrivacyStatsCollecting { @@ -126,8 +130,6 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } .store(in: &cancellables) - - subscribeToAppTermination() } public func recordBlockedTracker(_ companyName: String) async { @@ -172,6 +174,12 @@ public final class PrivacyStats: PrivacyStatsCollecting { await currentPack?.resetPack() } + public func handleAppTermination() async { + if let pack = await currentPack?.pack { + await commitChanges(pack) + } + } + // MARK: - Private private func commitChanges(_ pack: PrivacyStatsPack) async { @@ -242,25 +250,4 @@ public final class PrivacyStats: PrivacyStatsCollecting { } return pack ?? PrivacyStatsPack(timestamp: Date.currentPrivacyStatsPackTimestamp) } - - private func subscribeToAppTermination() { -#if os(iOS) - let notificationName = UIApplication.willTerminateNotification -#elseif os(macOS) - let notificationName = NSApplication.willTerminateNotification -#endif - NotificationCenter.default.addObserver(self, selector: #selector(applicationWillTerminate(_:)), name: notificationName, object: nil) - } - - @objc private func applicationWillTerminate(_: Notification) { - let condition = RunLoop.ResumeCondition() - Task { - if let pack = await currentPack?.pack { - await commitChanges(pack) - } - condition.resolve() - } - // Run the loop until changes are saved - RunLoop.current.run(until: condition) - } } diff --git a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift index 44b78e233..92775cf46 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift @@ -254,6 +254,36 @@ final class PrivacyStatsTests: XCTestCase { } } + // MARK: - handleAppTermination + + func testThatHandleAppTerminationSavesCurrentPack() async throws { + let context = databaseProvider.database.makeContext(concurrencyType: .privateQueueConcurrencyType) + + context.performAndWait { + do { + let allObjects = try context.fetch(DailyBlockedTrackersEntity.fetchRequest()) + XCTAssertTrue(allObjects.isEmpty) + } catch { + XCTFail("fetch failed: \(error)") + } + } + await privacyStats.recordBlockedTracker("A") + await privacyStats.handleAppTermination() + + context.performAndWait { + do { + let allObjects = try context.fetch(DailyBlockedTrackersEntity.fetchRequest()) + XCTAssertEqual(allObjects.count, 1) + } catch { + XCTFail("fetch failed: \(error)") + } + } + + await waitForStatsUpdateEvent() + let stats = await privacyStats.fetchPrivacyStats() + XCTAssertEqual(stats, ["A": 1]) + } + // MARK: - Helpers func waitForStatsUpdateEvent(file: StaticString = #file, line: UInt = #line) async { From f291fab7b5b1a8995ae9451f6a23aa1009627443 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 13:40:21 +0100 Subject: [PATCH 43/48] Update Sources/PrivacyStats/PrivacyStats.swift Co-authored-by: Pete Smith <5278441+aataraxiaa@users.noreply.github.com> --- Sources/PrivacyStats/PrivacyStats.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 7f401b404..325f77cb2 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -70,7 +70,7 @@ public protocol PrivacyStatsCollecting { /** * Record a tracker for a given `companyName`. * - * `PrivacyStats` implementation calls the actor under the hood, + * `PrivacyStats` implementation calls the `CurrentPack` actor under the hood, * and as such it can safely be called on multiple threads concurrently. */ func recordBlockedTracker(_ name: String) async From b85e69959b890844a9f855e3e4130f5a2275e891 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 14:14:30 +0100 Subject: [PATCH 44/48] Reset context after executing NSBatchDeleteRequest --- Sources/PrivacyStats/PrivacyStats.swift | 12 +++++------- .../PrivacyStats/internal/PrivacyStatsUtils.swift | 2 ++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 325f77cb2..9f9944398 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -201,11 +201,6 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } - // When storing a pack from a previous day, we may have outdated packs, so delete them as needed. - if !isCurrentDayPack { - try PrivacyStatsUtils.deleteOutdatedPacks(in: context) - } - guard context.hasChanges else { continuation.resume() return @@ -214,10 +209,13 @@ public final class PrivacyStats: PrivacyStatsCollecting { try context.save() Logger.privacyStats.debug("Saved stats \(pack.timestamp) \(pack.trackers)") - // Only emit update event when saving current-day pack. For previous-day pack, - // a follow-up commit event will come and we'll emit the update then. if isCurrentDayPack { + // Only emit update event when saving current-day pack. For previous-day pack, + // a follow-up commit event will come and we'll emit the update then. statsUpdateSubject.send() + } else { + // When storing a pack from a previous day, we may have outdated packs, so delete them as needed. + try PrivacyStatsUtils.deleteOutdatedPacks(in: context) } } catch { Logger.privacyStats.error("Save error: \(error)") diff --git a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift index d6ceb3032..33b93d869 100644 --- a/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift +++ b/Sources/PrivacyStats/internal/PrivacyStatsUtils.swift @@ -109,6 +109,7 @@ final class PrivacyStatsUtils { let deleteRequest = NSBatchDeleteRequest(fetchRequest: fetchRequest) try context.execute(deleteRequest) + context.reset() } /** @@ -117,5 +118,6 @@ final class PrivacyStatsUtils { static func deleteAllStats(in context: NSManagedObjectContext) throws { let deleteRequest = NSBatchDeleteRequest(fetchRequest: DailyBlockedTrackersEntity.fetchRequest()) try context.execute(deleteRequest) + context.reset() } } From e0d3d33efb7e2b4a111bd0fcd303e453a851c3fe Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 14:36:58 +0100 Subject: [PATCH 45/48] Make currentPack non-optional lazy var --- Sources/PrivacyStats/PrivacyStats.swift | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 9f9944398..2e483893e 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -109,7 +109,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { private let db: CoreDataDatabase private let context: NSManagedObjectContext - private var currentPack: CurrentPack? + private lazy var currentPack: CurrentPack = .init(pack: initializeCurrentPack()) private let statsUpdateSubject = PassthroughSubject() private var cancellables: Set = [] @@ -121,9 +121,8 @@ public final class PrivacyStats: PrivacyStatsCollecting { self.errorEvents = errorEvents statsUpdatePublisher = statsUpdateSubject.eraseToAnyPublisher() - currentPack = .init(pack: initializeCurrentPack()) - currentPack?.commitChangesPublisher + currentPack.commitChangesPublisher .sink { [weak self] pack in Task { await self?.commitChanges(pack) @@ -133,7 +132,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { } public func recordBlockedTracker(_ companyName: String) async { - await currentPack?.recordBlockedTracker(companyName) + await currentPack.recordBlockedTracker(companyName) } public func fetchPrivacyStats() async -> [String: Int64] { @@ -171,13 +170,11 @@ public final class PrivacyStats: PrivacyStatsCollecting { continuation.resume() } } - await currentPack?.resetPack() + await currentPack.resetPack() } public func handleAppTermination() async { - if let pack = await currentPack?.pack { - await commitChanges(pack) - } + await commitChanges(currentPack.pack) } // MARK: - Private @@ -242,7 +239,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { try PrivacyStatsUtils.deleteOutdatedPacks(in: context) } catch { - Logger.privacyStats.error("Faild to load current stats: \(error)") + Logger.privacyStats.error("Failed to load current stats: \(error)") errorEvents?.fire(.failedToLoadCurrentPrivacyStats(error)) } } From 3dd3b560d7937d45ec5d4ef7f8d9d0a0adabea4e Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 14:52:50 +0100 Subject: [PATCH 46/48] Send a stats update event after resetting data --- Sources/PrivacyStats/PrivacyStats.swift | 1 + .../PrivacyStatsTests/PrivacyStatsTests.swift | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index 2e483893e..fdca97d7f 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -171,6 +171,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { } } await currentPack.resetPack() + statsUpdateSubject.send() } public func handleAppTermination() async { diff --git a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift index 92775cf46..fa05d8178 100644 --- a/Tests/PrivacyStatsTests/PrivacyStatsTests.swift +++ b/Tests/PrivacyStatsTests/PrivacyStatsTests.swift @@ -221,6 +221,12 @@ final class PrivacyStatsTests: XCTestCase { // MARK: - clearPrivacyStats + func testThatClearPrivacyStatsTriggersUpdatesPublisher() async throws { + try await waitForStatsUpdateEvents(for: 1, count: 1) { + await privacyStats.clearPrivacyStats() + } + } + func testWhenClearPrivacyStatsIsCalledThenFetchPrivacyStatsIsEmpty() async throws { try databaseProvider.addObjects { context in let date = Date() @@ -292,4 +298,20 @@ final class PrivacyStatsTests: XCTestCase { await fulfillment(of: [expectation], timeout: 2) cancellable.cancel() } + + /** + * Sets up an expectation with the fulfillment count specified by `count` parameter, + * then sets up Combine subscription, then calls the provided block and waits + * for time specified by `duration` before cancelling the subscription. + */ + func waitForStatsUpdateEvents(for duration: TimeInterval, count: Int, _ block: () async -> Void) async throws { + let expectation = self.expectation(description: "statsUpdate") + expectation.expectedFulfillmentCount = count + let cancellable = privacyStats.statsUpdatePublisher.sink { expectation.fulfill() } + + await block() + + await fulfillment(of: [expectation], timeout: duration) + cancellable.cancel() + } } From c0b708c48e3038694670f383f879143fd6242097 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 15:06:51 +0100 Subject: [PATCH 47/48] Increase timeout in testThatMultipleCallsToRecordBlockedTrackerOnlyTriggerOneCommitChangesEvent --- Tests/PrivacyStatsTests/CurrentPackTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/PrivacyStatsTests/CurrentPackTests.swift b/Tests/PrivacyStatsTests/CurrentPackTests.swift index 07bc634e2..f22ed322d 100644 --- a/Tests/PrivacyStatsTests/CurrentPackTests.swift +++ b/Tests/PrivacyStatsTests/CurrentPackTests.swift @@ -44,7 +44,7 @@ final class CurrentPackTests: XCTestCase { } func testThatMultipleCallsToRecordBlockedTrackerOnlyTriggerOneCommitChangesEvent() async throws { - let packs = try await waitForCommitChangesEvents(for: 100_000_000) { + let packs = try await waitForCommitChangesEvents(for: 1000_000_000) { await currentPack.recordBlockedTracker("A") await currentPack.recordBlockedTracker("A") await currentPack.recordBlockedTracker("A") From e7db29260c0acebacca6c4b1f1d19c2287c01bd0 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 29 Nov 2024 15:20:48 +0100 Subject: [PATCH 48/48] Make initializeCurrentPack a static function and currenPack a let --- Sources/PrivacyStats/PrivacyStats.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/PrivacyStats/PrivacyStats.swift b/Sources/PrivacyStats/PrivacyStats.swift index fdca97d7f..c298f60fc 100644 --- a/Sources/PrivacyStats/PrivacyStats.swift +++ b/Sources/PrivacyStats/PrivacyStats.swift @@ -109,7 +109,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { private let db: CoreDataDatabase private let context: NSManagedObjectContext - private lazy var currentPack: CurrentPack = .init(pack: initializeCurrentPack()) + private let currentPack: CurrentPack private let statsUpdateSubject = PassthroughSubject() private var cancellables: Set = [] @@ -119,7 +119,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { self.db = databaseProvider.initializeDatabase() self.context = db.makeContext(concurrencyType: .privateQueueConcurrencyType, name: "PrivacyStats") self.errorEvents = errorEvents - + self.currentPack = .init(pack: Self.initializeCurrentPack(in: context, errorEvents: errorEvents)) statsUpdatePublisher = statsUpdateSubject.eraseToAnyPublisher() currentPack.commitChangesPublisher @@ -229,7 +229,7 @@ public final class PrivacyStats: PrivacyStatsCollecting { * to spare us the hassle of declaring the initializer async or spawning tasks from within the * initializer without being able to await them, thus making testing trickier. */ - private func initializeCurrentPack() -> PrivacyStatsPack { + private static func initializeCurrentPack(in context: NSManagedObjectContext, errorEvents: EventMapping?) -> PrivacyStatsPack { var pack: PrivacyStatsPack? context.performAndWait { let timestamp = Date.currentPrivacyStatsPackTimestamp