Add Privacy Stats module for collecting stats about blocked trackers #1097
Conversation
ayoy
changed the title
There was a problem hiding this comment.
Left a few comments @ayoy. Overall looks good. Nice tests. 👍🏼
Co-authored-by: Pete Smith <[email protected]>
Co-authored-by: Pete Smith <[email protected]>
| fetchRequest.predicate = NSPredicate(format: "%K < %@", #keyPath(DailyBlockedTrackersEntity.timestamp), oldestValidTimestamp as NSDate) | ||
| let deleteRequest = NSBatchDeleteRequest(fetchRequest: fetchRequest) | ||
|
|
||
| try context.execute(deleteRequest) |
There was a problem hiding this comment.
As we are using batch delete request, I'd reset the context after this to invalidate any object that could be still cached in the object graph by the context.
|
|
||
| private let db: CoreDataDatabase | ||
| private let context: NSManagedObjectContext | ||
| private var currentPack: CurrentPack? |
There was a problem hiding this comment.
Can this be non optional let?
There was a problem hiding this comment.
It can, it was just initialized using self.initializeCurrentPack() which was causing the "self used before all properties are initialized". But if I make it lazy var then it works fine as non-optional :)
There was a problem hiding this comment.
You could make it static and pass context and errorEvents as params to avoid this warning.
There was a problem hiding this comment.
That was my initial idea actually. Agreed that making it a let has a benefit to the reader so let me update it 👍
| * It sets a new empty pack with the current timestamp. | ||
| */ | ||
| func resetPack() { | ||
| resetStats(andSet: Date.currentPrivacyStatsPackTimestamp) |
There was a problem hiding this comment.
Just curious - shouldn't that notifyChanges ?
There was a problem hiding this comment.
Great point - it actually should. It works because it's used by the Fire button which closes everything and opens a new window in which case new data is pulled, but overall this should trigger a notification. Thanks for spotting it!
…iggerOneCommitChangesEvent
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/72649045549333/1208246350498754/f
iOS PR: duckduckgo/iOS#3651
macOS PR: duckduckgo/macos-browser#3611
What kind of version bump will this require?: Major
Optional:
Tech Design URL: https://app.asana.com/0/481882893211075/1208848285586302
CC: @bwaresiak
Description:
This change adds PrivacyStats module that will be used in macOS HTML New Tab Page.
Privacy Stats uses Core Data for storage where it keeps blocked trackers counts per tracker
company name per day, for past 7 days.
PrivacyStats class is the main (and only) public interface. It provides simple API to fetch current
stats, clear them (for Fire button use) and record blocked trackers, as well as a publisher
that notifies about changes to stats.
Steps to test this PR:
TBD in the macOS PR
OS Testing:
Internal references:
Software Engineering Expectations
Technical Design Template