Skip to content

Commit

Permalink
Run opt outs when profile is saved (#3384)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1203581873609357/1208333317271878/f
Tech Design URL:
CC:

**Description**:

Opt outs are triggered as soon as initial scan finishes.

**Steps to test this PR**:
1. If you build from Xcode 16, check out this branch instead:
https://github.com/duckduckgo/macos-browser/tree/anh/always-show-webview
(context: https://app.asana.com/0/414235014887631/1208502720748038/f)
2. Go through the PIR flow, ideally with a test profile with at least 2
parent brokers
3. Check console logs while attaching to `DuckDuckGo Personal
Information Removal` to verify that the opt out jobs are submitted when
the initial scan finishes.
4. Open Debug > PIR > DB Browser and double check the OptOutJob table. 

<!--
Tagging instructions
If this PR isn't ready to be merged for whatever reason it should be
marked with the `DO NOT MERGE` label (particularly if it's a draft)
If it's pending Product Review/PFR, please add the `Pending Product
Review` label.

If at any point it isn't actively being worked on/ready for
review/otherwise moving forward (besides the above PR/PFR exception)
strongly consider closing it (or not opening it in the first place). If
you decide not to close it, make sure it's labelled to make it clear the
PRs state and comment with more information.
-->

**Definition of Done**:

* [ ] Does this PR satisfy our [Definition of
Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?

---
###### Internal references:
[Pull Request Review
Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f)
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
[Pull Request
Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
  • Loading branch information
quanganhdo authored Oct 11, 2024
1 parent 39ae68c commit 7ac4c4c
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 76 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public final class DataBrokerProtectionAgentManager {
activityScheduler.startScheduler()
didStartActivityScheduler = true
fireMonitoringPixels()
queueManager.startScheduledOperationsIfPermitted(showWebView: false, operationDependencies: operationDependencies, completion: nil)
queueManager.startScheduledOperationsIfPermitted(showWebView: false, operationDependencies: operationDependencies, errorHandler: nil, completion: nil)

/// Monitors entitlement changes every 60 minutes to optimize system performance and resource utilization by avoiding unnecessary operations when entitlement is invalid.
/// While keeping the agent active with invalid entitlement has no significant risk, setting the monitoring interval at 60 minutes is a good balance to minimize backend checks.
Expand Down Expand Up @@ -207,15 +207,19 @@ extension DataBrokerProtectionAgentManager {
extension DataBrokerProtectionAgentManager: DataBrokerProtectionBackgroundActivitySchedulerDelegate {

public func dataBrokerProtectionBackgroundActivitySchedulerDidTrigger(_ activityScheduler: DataBrokerProtection.DataBrokerProtectionBackgroundActivityScheduler, completion: (() -> Void)?) {
startScheduledOperations(completion: completion)
}

func startScheduledOperations(completion: (() -> Void)?) {
fireMonitoringPixels()
queueManager.startScheduledOperationsIfPermitted(showWebView: false, operationDependencies: operationDependencies) { _ in
completion?()
}
queueManager.startScheduledOperationsIfPermitted(showWebView: false,
operationDependencies: operationDependencies,
errorHandler: nil,
completion: completion)
}
}

extension DataBrokerProtectionAgentManager: DataBrokerProtectionAgentAppEvents {

public func profileSaved() {
let backgroundAgentInitialScanStartTime = Date()

Expand Down Expand Up @@ -245,21 +249,25 @@ extension DataBrokerProtectionAgentManager: DataBrokerProtectionAgentAppEvents {
self.pixelHandler.fire(.ipcServerImmediateScansFinishedWithoutError)
self.userNotificationService.sendFirstScanCompletedNotification()
}
} completion: { [weak self] in
guard let self else { return }

if let hasMatches = try? self.dataManager.hasMatches(),
hasMatches {
hasMatches {
self.userNotificationService.scheduleCheckInNotificationIfPossible()
}

fireImmediateScansCompletionPixel(startTime: backgroundAgentInitialScanStartTime)

self.startScheduledOperations(completion: nil)
}
}

public func appLaunched() {
fireMonitoringPixels()
queueManager.startScheduledOperationsIfPermitted(showWebView: false,
operationDependencies:
operationDependencies) { [weak self] errors in
operationDependencies: operationDependencies,
errorHandler: { [weak self] errors in
guard let self = self else { return }

if let errors = errors {
Expand All @@ -285,7 +293,7 @@ extension DataBrokerProtectionAgentManager: DataBrokerProtectionAgentAppEvents {
if errors?.oneTimeError == nil {
self.pixelHandler.fire(.ipcServerAppLaunchedScheduledScansFinishedWithoutError)
}
}
}, completion: nil)
}

private func fireImmediateScansCompletionPixel(startTime: Date) {
Expand All @@ -310,18 +318,21 @@ extension DataBrokerProtectionAgentManager: DataBrokerProtectionAgentDebugComman
public func startImmediateOperations(showWebView: Bool) {
queueManager.startImmediateOperationsIfPermitted(showWebView: showWebView,
operationDependencies: operationDependencies,
errorHandler: nil,
completion: nil)
}

public func startScheduledOperations(showWebView: Bool) {
queueManager.startScheduledOperationsIfPermitted(showWebView: showWebView,
operationDependencies: operationDependencies,
errorHandler: nil,
completion: nil)
}

public func runAllOptOuts(showWebView: Bool) {
queueManager.execute(.startOptOutOperations(showWebView: showWebView,
operationDependencies: operationDependencies,
errorHandler: nil,
completion: nil))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,15 @@ protocol DataBrokerProtectionOperationQueue {
var maxConcurrentOperationCount: Int { get set }
func cancelAllOperations()
func addOperation(_ op: Operation)
func addBarrierCompletionBlock(_ barrier: @escaping @Sendable () -> Void)
func addBarrierBlock(_ barrier: @escaping @Sendable () -> Void)
}

extension OperationQueue: DataBrokerProtectionOperationQueue {
/*
An unfortunute necessarity due to an issue with xcode 16 and building for Product Review Release
Originally we just used addBarrierBlock directly, but now it won't build as it says
the extension doesn't conform to the protocol
The docs however say the method signiture is unchanged, so we've decided to blame the build system
*/
func addBarrierCompletionBlock(_ barrier: @escaping @Sendable () -> Void) {
addBarrierBlock(barrier)
}
}
extension OperationQueue: DataBrokerProtectionOperationQueue {}

enum DataBrokerProtectionQueueMode {
case idle
case immediate(completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?)
case scheduled(completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?)
case immediate(errorHandler: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?, completion: (() -> Void)?)
case scheduled(errorHandler: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?, completion: (() -> Void)?)

var priorityDate: Date? {
switch self {
Expand Down Expand Up @@ -73,7 +63,8 @@ enum DataBrokerProtectionQueueError: Error {
enum DataBrokerProtectionQueueManagerDebugCommand {
case startOptOutOperations(showWebView: Bool,
operationDependencies: DataBrokerOperationDependencies,
completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?)
errorHandler: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?,
completion: (() -> Void)?)
}

protocol DataBrokerProtectionQueueManager {
Expand All @@ -86,10 +77,12 @@ protocol DataBrokerProtectionQueueManager {

func startImmediateOperationsIfPermitted(showWebView: Bool,
operationDependencies: DataBrokerOperationDependencies,
completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?)
errorHandler: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?,
completion: (() -> Void)?)
func startScheduledOperationsIfPermitted(showWebView: Bool,
operationDependencies: DataBrokerOperationDependencies,
completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?)
errorHandler: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?,
completion: (() -> Void)?)

func execute(_ command: DataBrokerProtectionQueueManagerDebugCommand)
var debugRunningStatusString: String { get }
Expand Down Expand Up @@ -131,37 +124,44 @@ final class DefaultDataBrokerProtectionQueueManager: DataBrokerProtectionQueueMa

func startImmediateOperationsIfPermitted(showWebView: Bool,
operationDependencies: DataBrokerOperationDependencies,
completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?) {
errorHandler: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?,
completion: (() -> Void)?) {

let newMode = DataBrokerProtectionQueueMode.immediate(completion: completion)
let newMode = DataBrokerProtectionQueueMode.immediate(errorHandler: errorHandler, completion: completion)
startOperationsIfPermitted(forNewMode: newMode,
type: .scan,
showWebView: showWebView,
operationDependencies: operationDependencies) { [weak self] errors in
completion?(errors)
self?.mismatchCalculator.calculateMismatches()
errorHandler?(errors)
} completion: {
completion?()
}
}

func startScheduledOperationsIfPermitted(showWebView: Bool,
operationDependencies: DataBrokerOperationDependencies,
completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?) {
let newMode = DataBrokerProtectionQueueMode.scheduled(completion: completion)
errorHandler: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?,
completion: (() -> Void)?) {
let newMode = DataBrokerProtectionQueueMode.scheduled(errorHandler: errorHandler, completion: completion)
startOperationsIfPermitted(forNewMode: newMode,
type: .all,
showWebView: showWebView,
operationDependencies: operationDependencies,
errorHandler: errorHandler,
completion: completion)
}

func execute(_ command: DataBrokerProtectionQueueManagerDebugCommand) {
guard case .startOptOutOperations(let showWebView,
let operationDependencies,
let errorHandler,
let completion) = command else { return }

addOperations(withType: .optOut,
showWebView: showWebView,
operationDependencies: operationDependencies,
errorHandler: errorHandler,
completion: completion)
}
}
Expand All @@ -172,12 +172,14 @@ private extension DefaultDataBrokerProtectionQueueManager {
type: OperationType,
showWebView: Bool,
operationDependencies: DataBrokerOperationDependencies,
completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?) {
errorHandler: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?,
completion: (() -> Void)?) {

guard mode.canBeInterruptedBy(newMode: newMode) else {
let error = DataBrokerProtectionQueueError.cannotInterrupt
let errorCollection = DataBrokerProtectionAgentErrorCollection(oneTimeError: error)
completion?(errorCollection)
errorHandler?(errorCollection)
completion?()
return
}

Expand All @@ -191,24 +193,29 @@ private extension DefaultDataBrokerProtectionQueueManager {
priorityDate: mode.priorityDate,
showWebView: showWebView,
operationDependencies: operationDependencies,
errorHandler: errorHandler,
completion: completion)
}

func cancelCurrentModeAndResetIfNeeded() {
switch mode {
case .immediate(let completion), .scheduled(let completion):
case .immediate(let errorHandler, let completion), .scheduled(let errorHandler, let completion):
operationQueue.cancelAllOperations()
let errorCollection = DataBrokerProtectionAgentErrorCollection(oneTimeError: DataBrokerProtectionQueueError.interrupted, operationErrors: operationErrorsForCurrentOperations())
completion?(errorCollection)
resetModeAndClearErrors()
errorHandler?(errorCollection)
resetMode(clearErrors: true)
completion?()
resetMode()
default:
break
}
}

func resetModeAndClearErrors() {
func resetMode(clearErrors: Bool = false) {
mode = .idle
operationErrors = []
if clearErrors {
operationErrors = []
}
}

func updateBrokerData() {
Expand All @@ -220,7 +227,8 @@ private extension DefaultDataBrokerProtectionQueueManager {
priorityDate: Date? = nil,
showWebView: Bool,
operationDependencies: DataBrokerOperationDependencies,
completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?) {
errorHandler: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?,
completion: (() -> Void)?) {

operationQueue.maxConcurrentOperationCount = operationDependencies.config.concurrentOperationsFor(type)

Expand All @@ -238,14 +246,17 @@ private extension DefaultDataBrokerProtectionQueueManager {
}
} catch {
Logger.dataBrokerProtection.error("DataBrokerProtectionProcessor error: addOperations, error: \(error.localizedDescription, privacy: .public)")
completion?(DataBrokerProtectionAgentErrorCollection(oneTimeError: error))
errorHandler?(DataBrokerProtectionAgentErrorCollection(oneTimeError: error))
completion?()
return
}

operationQueue.addBarrierCompletionBlock { [weak self] in
operationQueue.addBarrierBlock { [weak self] in
let errorCollection = DataBrokerProtectionAgentErrorCollection(oneTimeError: nil, operationErrors: self?.operationErrorsForCurrentOperations())
completion?(errorCollection)
self?.resetModeAndClearErrors()
errorHandler?(errorCollection)
self?.resetMode(clearErrors: true)
completion?()
self?.resetMode()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import os.log
public extension Logger {
fileprivate static let subsystem = "com.duckduckgo.macos.browser.databroker-protection"

static var dataBrokerProtection = { Logger(subsystem: subsystem, category: "") }()
static var dataBrokerProtection = { Logger(subsystem: subsystem, category: "Data Broker Protection") }()
static var action = { Logger(subsystem: subsystem, category: "Action") }()
static var service = { Logger(subsystem: subsystem, category: "Service") }()
static var backgroundAgent = { Logger(subsystem: subsystem, category: "Background Agent") }()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase {

let scanCalledExpectation = XCTestExpectation(description: "Scan called")
var startScheduledScansCalled = false
mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = { _ in
mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = {
startScheduledScansCalled = true
scanCalledExpectation.fulfill()
}
Expand Down Expand Up @@ -209,7 +209,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase {
mockDataManager.profileToReturn = mockProfile

var startScheduledScansCalled = false
mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = { _ in
mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = {
startScheduledScansCalled = true
}

Expand Down Expand Up @@ -237,7 +237,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase {
mockDataManager.profileToReturn = mockProfile

var startImmediateScansCalled = false
mockQueueManager.startImmediateOperationsIfPermittedCalledCompletion = { _ in
mockQueueManager.startImmediateOperationsIfPermittedCalledCompletion = {
startImmediateScansCalled = true
}

Expand Down Expand Up @@ -381,7 +381,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase {
privacyConfigurationManager: mockPrivacyConfigurationManager)

var startScheduledScansCalled = false
mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = { _ in
mockQueueManager.startScheduledOperationsIfPermittedCalledCompletion = {
startScheduledScansCalled = true
}

Expand Down
Loading

0 comments on commit 7ac4c4c

Please sign in to comment.