Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Implement Queue Manager for State & Interruption Management #2757

Merged
merged 21 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions DuckDuckGo/DBP/DataBrokerProtectionDebugMenu.swift
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ final class DataBrokerProtectionDebugMenu: NSMenu {
NSMenuItem(title: "Operations") {
NSMenuItem(title: "Hidden WebView") {
menuItem(withTitle: "Run queued operations",
action: #selector(DataBrokerProtectionDebugMenu.runQueuedOperations(_:)),
action: #selector(DataBrokerProtectionDebugMenu.startScheduledOperations(_:)),
representedObject: false)

menuItem(withTitle: "Run scan operations",
Expand All @@ -119,7 +119,7 @@ final class DataBrokerProtectionDebugMenu: NSMenu {

NSMenuItem(title: "Visible WebView") {
menuItem(withTitle: "Run queued operations",
action: #selector(DataBrokerProtectionDebugMenu.runQueuedOperations(_:)),
action: #selector(DataBrokerProtectionDebugMenu.startScheduledOperations(_:)),
representedObject: true)

menuItem(withTitle: "Run scan operations",
Expand Down Expand Up @@ -204,18 +204,18 @@ final class DataBrokerProtectionDebugMenu: NSMenu {
}
}

@objc private func runQueuedOperations(_ sender: NSMenuItem) {
@objc private func startScheduledOperations(_ sender: NSMenuItem) {
os_log("Running queued operations...", log: .dataBrokerProtection)
let showWebView = sender.representedObject as? Bool ?? false

DataBrokerProtectionManager.shared.loginItemInterface.runQueuedOperations(showWebView: showWebView)
DataBrokerProtectionManager.shared.loginItemInterface.startScheduledOperations(showWebView: showWebView)
}

@objc private func runScanOperations(_ sender: NSMenuItem) {
os_log("Running scan operations...", log: .dataBrokerProtection)
let showWebView = sender.representedObject as? Bool ?? false

DataBrokerProtectionManager.shared.loginItemInterface.startManualScan(showWebView: showWebView)
DataBrokerProtectionManager.shared.loginItemInterface.startImmediateOperations(showWebView: showWebView)
}

@objc private func runOptoutOperations(_ sender: NSMenuItem) {
Expand Down Expand Up @@ -295,7 +295,7 @@ final class DataBrokerProtectionDebugMenu: NSMenu {
}

@objc private func forceBrokerJSONFilesUpdate() {
if let updater = DataBrokerProtectionBrokerUpdater.provide() {
if let updater = DefaultDataBrokerProtectionBrokerUpdater.provideForDebug() {
updater.updateBrokers()
}
}
Expand Down
8 changes: 4 additions & 4 deletions DuckDuckGo/DBP/DataBrokerProtectionLoginItemInterface.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,20 @@

func profileSaved() {
enableLoginItem()
ipcClient.profileSaved { error in

Check failure on line 61 in DuckDuckGo/DBP/DataBrokerProtectionLoginItemInterface.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Unused parameter in a closure should be replaced with _ (unused_closure_parameter)
// TODO

Check failure on line 62 in DuckDuckGo/DBP/DataBrokerProtectionLoginItemInterface.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

TODOs should be resolved (todo)
}
}

func dataDeleted() {
ipcClient.dataDeleted { error in

Check failure on line 67 in DuckDuckGo/DBP/DataBrokerProtectionLoginItemInterface.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Unused parameter in a closure should be replaced with _ (unused_closure_parameter)
// TODO

Check failure on line 68 in DuckDuckGo/DBP/DataBrokerProtectionLoginItemInterface.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

TODOs should be resolved (todo)
}
}

func appLaunched() {
ipcClient.appLaunched { error in

Check failure on line 73 in DuckDuckGo/DBP/DataBrokerProtectionLoginItemInterface.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Unused parameter in a closure should be replaced with _ (unused_closure_parameter)
// TODO

Check failure on line 74 in DuckDuckGo/DBP/DataBrokerProtectionLoginItemInterface.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

TODOs should be resolved (todo)
}
}

Expand All @@ -81,12 +81,12 @@
ipcClient.openBrowser(domain: domain)
}

func startManualScan(showWebView: Bool) {
ipcClient.startManualScan(showWebView: showWebView)
func startImmediateOperations(showWebView: Bool) {
ipcClient.startImmediateOperations(showWebView: showWebView)
}

func runQueuedOperations(showWebView: Bool) {
ipcClient.runQueuedOperations(showWebView: showWebView)
func startScheduledOperations(showWebView: Bool) {
ipcClient.startScheduledOperations(showWebView: showWebView)
}

func runAllOptOuts(showWebView: Bool) {
Expand Down
6 changes: 3 additions & 3 deletions DuckDuckGo/DBP/DataBrokerProtectionPixelsHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class DataBrokerProtectionPixelsHandler: EventMapping<DataBrokerProtectio
.ipcServerScanAllBrokersCompletedOnAgentWithError(error: let error),
.ipcServerScanAllBrokersCompletionCalledOnAppWithError(error: let error),
.ipcServerOptOutAllBrokersCompletion(error: let error),
.ipcServerRunQueuedOperationsCompletion(error: let error):
.ipcServerStartScheduledOperationsCompletion(error: let error):
PixelKit.fire(DebugEvent(event, error: error), frequency: .dailyAndCount, includeAppVersionParameter: true)
case .ipcServerStartSchedulerCalledByApp,
.ipcServerStartSchedulerReceivedByAgent,
Expand Down Expand Up @@ -72,10 +72,10 @@ public class DataBrokerProtectionPixelsHandler: EventMapping<DataBrokerProtectio
.backgroundAgentStarted,
.backgroundAgentRunOperationsAndStartSchedulerIfPossible,
.backgroundAgentRunOperationsAndStartSchedulerIfPossibleNoSavedProfile,
.backgroundAgentRunOperationsAndStartSchedulerIfPossibleRunQueuedOperationsCallbackStartScheduler,
.backgroundAgentRunOperationsAndStartSchedulerIfPossibleStartScheduledOperationsCallbackStartScheduler,
.backgroundAgentStartedStoppingDueToAnotherInstanceRunning,
.ipcServerOptOutAllBrokers,
.ipcServerRunQueuedOperations,
.ipcServerStartScheduledOperations,
.ipcServerRunAllOperations,
.scanSuccess,
.scanFailed,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ public final class DataBrokerProtectionAgentManager {
return
}

scheduler.runQueuedOperations(showWebView: false) { [weak self] _ in
self?.pixelHandler.fire(.backgroundAgentRunOperationsAndStartSchedulerIfPossibleRunQueuedOperationsCallbackStartScheduler)
scheduler.startScheduledOperations(showWebView: false) { [weak self] _ in
self?.pixelHandler.fire(.backgroundAgentRunOperationsAndStartSchedulerIfPossibleStartScheduledOperationsCallbackStartScheduler)
self?.scheduler.startScheduler()
}
}
Expand All @@ -105,7 +105,7 @@ public final class DataBrokerProtectionAgentManager {
extension DataBrokerProtectionAgentManager: DataBrokerProtectionAgentAppEvents {

public func profileSaved() {
scheduler.startManualScan(startTime: Date()) { _ in
scheduler.startImmediateOperations(startTime: Date()) { _ in

}
}
Expand All @@ -115,7 +115,7 @@ extension DataBrokerProtectionAgentManager: DataBrokerProtectionAgentAppEvents {
}

public func appLaunched() {
scheduler.runQueuedOperations()
scheduler.startScheduledOperations()
}
}

Expand All @@ -126,14 +126,14 @@ extension DataBrokerProtectionAgentManager: DataBrokerProtectionAgentDebugComman
}
}

public func startManualScan(showWebView: Bool) {
scheduler.startManualScan(startTime: Date()) { _ in
public func startImmediateOperations(showWebView: Bool) {
scheduler.startImmediateOperations(startTime: Date()) { _ in

}
}

public func runQueuedOperations(showWebView: Bool) {
scheduler.runQueuedOperations(showWebView: showWebView)
public func startScheduledOperations(showWebView: Bool) {
scheduler.startScheduledOperations(showWebView: showWebView)
}

public func runAllOptOuts(showWebView: Bool) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ final class DataBrokerRunCustomJSONViewModel: ObservableObject {

final class FakeStageDurationCalculator: StageDurationCalculator {
var attemptId: UUID = UUID()
var isManualScan: Bool = false
var isImmediateOperation: Bool = false

func durationSinceLastStage() -> Double {
0.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ public protocol DataBrokerProtectionAgentAppEvents {

public protocol DataBrokerProtectionAgentDebugCommands {
func openBrowser(domain: String)
func startManualScan(showWebView: Bool)
func runQueuedOperations(showWebView: Bool)
func startImmediateOperations(showWebView: Bool)
func startScheduledOperations(showWebView: Bool)
func runAllOptOuts(showWebView: Bool)
func getDebugMetadata() async -> DBPBackgroundAgentMetadata?
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,19 +133,19 @@ extension DataBrokerProtectionIPCClient: IPCServerInterface {
})
}

public func startManualScan(showWebView: Bool) {
public func startImmediateOperations(showWebView: Bool) {
xpc.execute(call: { server in
server.startManualScan(showWebView: showWebView)
server.startImmediateOperations(showWebView: showWebView)
}, xpcReplyErrorHandler: { error in
os_log("Error \(error.localizedDescription)")
// Intentional no-op as there's no completion block
// If you add a completion block, please remember to call it here too!
})
}

public func runQueuedOperations(showWebView: Bool) {
public func startScheduledOperations(showWebView: Bool) {
xpc.execute(call: { server in
server.runQueuedOperations(showWebView: showWebView)
server.startScheduledOperations(showWebView: showWebView)
}, xpcReplyErrorHandler: { error in
os_log("Error \(error.localizedDescription)")
// Intentional no-op as there's no completion block
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@
///
func openBrowser(domain: String)

func startManualScan(showWebView: Bool)
func runQueuedOperations(showWebView: Bool)
func startImmediateOperations(showWebView: Bool)
func startScheduledOperations(showWebView: Bool)
func runAllOptOuts(showWebView: Bool)
func getDebugMetadata(completion: @escaping (DBPBackgroundAgentMetadata?) -> Void)
}
Expand Down Expand Up @@ -154,7 +154,7 @@
extension DataBrokerProtectionIPCServer: XPCServerInterface {

func register() {

Check failure on line 157 in LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/IPC/DataBrokerProtectionIPCServer.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Lines should not have trailing whitespace (trailing_whitespace)
}

// MARK: - DataBrokerProtectionAgentAppEvents
Expand All @@ -180,12 +180,12 @@
serverDelegate?.openBrowser(domain: domain)
}

func startManualScan(showWebView: Bool) {
serverDelegate?.startManualScan(showWebView: showWebView)
func startImmediateOperations(showWebView: Bool) {
serverDelegate?.startImmediateOperations(showWebView: showWebView)
}

func runQueuedOperations(showWebView: Bool) {
serverDelegate?.runQueuedOperations(showWebView: showWebView)
func startScheduledOperations(showWebView: Bool) {
serverDelegate?.startScheduledOperations(showWebView: showWebView)
}

func runAllOptOuts(showWebView: Bool) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,15 @@ extension DataBrokerJob {
}

private func fireSiteLoadingPixel(startTime: Date, hasError: Bool) {
if stageCalculator.isManualScan {
if stageCalculator.isImmediateOperation {
let dataBrokerURL = self.query.dataBroker.url
let durationInMs = (Date().timeIntervalSince(startTime) * 1000).rounded(.towardZero)
pixelHandler.fire(.initialScanSiteLoadDuration(duration: durationInMs, hasError: hasError, brokerURL: dataBrokerURL))
}
}

func firePostLoadingDurationPixel(hasError: Bool) {
if stageCalculator.isManualScan, let postLoadingSiteStartTime = self.postLoadingSiteStartTime {
if stageCalculator.isImmediateOperation, let postLoadingSiteStartTime = self.postLoadingSiteStartTime {
let dataBrokerURL = self.query.dataBroker.url
let durationInMs = (Date().timeIntervalSince(postLoadingSiteStartTime) * 1000).rounded(.towardZero)
pixelHandler.fire(.initialScanPostLoadingDuration(duration: durationInMs, hasError: hasError, brokerURL: dataBrokerURL))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,9 @@
import Foundation
import Common

enum OperationType {
case manualScan
case optOut
case all
}

protocol DataBrokerOperationDependencies {
var database: DataBrokerProtectionRepository { get }
var brokerTimeInterval: TimeInterval { get }
var config: DataBrokerProtectionProcessorConfiguration { get }
var runnerProvider: JobRunnerProvider { get }
var notificationCenter: NotificationCenter { get }
var pixelHandler: EventMapping<DataBrokerProtectionPixels> { get }
Expand All @@ -36,30 +30,36 @@ protocol DataBrokerOperationDependencies {

struct DefaultDataBrokerOperationDependencies: DataBrokerOperationDependencies {
let database: DataBrokerProtectionRepository
let brokerTimeInterval: TimeInterval
var config: DataBrokerProtectionProcessorConfiguration
let runnerProvider: JobRunnerProvider
let notificationCenter: NotificationCenter
let pixelHandler: EventMapping<DataBrokerProtectionPixels>
let userNotificationService: DataBrokerProtectionUserNotificationService
}

final class DataBrokerOperation: Operation {
enum OperationType {
case scan
case optOut
case all
}

protocol DataBrokerOperationErrorDelegate: AnyObject {
func dataBrokerOperationDidError(_ error: Error, withBrokerName brokerName: String?)
}

public var error: Error?
// swiftlint:disable explicit_non_final_class
class DataBrokerOperation: Operation {

private let dataBrokerID: Int64
private let database: DataBrokerProtectionRepository
private let operationType: OperationType
private let priorityDate: Date? // The date to filter and sort operations priorities
private let showWebView: Bool
private(set) weak var errorDelegate: DataBrokerOperationErrorDelegate? // Internal read-only to enable mocking
private let operationDependencies: DataBrokerOperationDependencies

private let id = UUID()
private var _isExecuting = false
private var _isFinished = false
private let brokerTimeInterval: TimeInterval? // The time in seconds to wait in-between operations
private let priorityDate: Date? // The date to filter and sort operations priorities
private let operationType: OperationType
private let notificationCenter: NotificationCenter
private let runner: WebJobRunner
private let pixelHandler: EventMapping<DataBrokerProtectionPixels>
private let showWebView: Bool
private let userNotificationService: DataBrokerProtectionUserNotificationService

deinit {
os_log("Deinit operation: %{public}@", log: .dataBrokerProtection, String(describing: id.uuidString))
Expand All @@ -69,18 +69,15 @@ final class DataBrokerOperation: Operation {
operationType: OperationType,
priorityDate: Date? = nil,
showWebView: Bool,
errorDelegate: DataBrokerOperationErrorDelegate,
operationDependencies: DataBrokerOperationDependencies) {

self.dataBrokerID = dataBrokerID
self.priorityDate = priorityDate
self.operationType = operationType
self.showWebView = showWebView
self.database = operationDependencies.database
self.brokerTimeInterval = operationDependencies.brokerTimeInterval
self.runner = operationDependencies.runnerProvider.getJobRunner()
self.notificationCenter = operationDependencies.notificationCenter
self.pixelHandler = operationDependencies.pixelHandler
self.userNotificationService = operationDependencies.userNotificationService
self.errorDelegate = errorDelegate
self.operationDependencies = operationDependencies
super.init()
}

Expand Down Expand Up @@ -122,7 +119,7 @@ final class DataBrokerOperation: Operation {
switch operationType {
case .optOut:
operationsData = brokerProfileQueriesData.flatMap { $0.optOutJobData }
case .manualScan:
case .scan:
operationsData = brokerProfileQueriesData.filter { $0.profileQuery.deprecated == false }.compactMap { $0.scanJobData }
case .all:
operationsData = brokerProfileQueriesData.flatMap { $0.operationsData }
Expand All @@ -141,12 +138,11 @@ final class DataBrokerOperation: Operation {
return filteredAndSortedOperationsData
}

// swiftlint:disable:next function_body_length
private func runOperation() async {
let allBrokerProfileQueryData: [BrokerProfileQueryData]

do {
allBrokerProfileQueryData = try database.fetchAllBrokerProfileQueryData()
allBrokerProfileQueryData = try operationDependencies.database.fetchAllBrokerProfileQueryData()
} catch {
os_log("DataBrokerOperationsCollection error: runOperation, error: %{public}@", log: .error, error.localizedDescription)
return
Expand Down Expand Up @@ -178,31 +174,26 @@ final class DataBrokerOperation: Operation {

try await DataBrokerProfileQueryOperationManager().runOperation(operationData: operationData,
brokerProfileQueryData: brokerProfileData,
database: database,
notificationCenter: notificationCenter,
runner: runner,
pixelHandler: pixelHandler,
database: operationDependencies.database,
notificationCenter: operationDependencies.notificationCenter,
runner: operationDependencies.runnerProvider.getJobRunner(),
pixelHandler: operationDependencies.pixelHandler,
showWebView: showWebView,
isManualScan: operationType == .manualScan,
userNotificationService: userNotificationService,
isImmediateOperation: operationType == .scan,
userNotificationService: operationDependencies.userNotificationService,
shouldRunNextStep: { [weak self] in
guard let self = self else { return false }
return !self.isCancelled
})

if let sleepInterval = brokerTimeInterval {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

brokerTimeInterval was optional in this type, but was always passed in as a parameter. Optionality was unnecessary.

os_log("Waiting...: %{public}f", log: .dataBrokerProtection, sleepInterval)
try await Task.sleep(nanoseconds: UInt64(sleepInterval) * 1_000_000_000)
}
let sleepInterval = operationDependencies.config.intervalBetweenSameBrokerOperations
os_log("Waiting...: %{public}f", log: .dataBrokerProtection, sleepInterval)
try await Task.sleep(nanoseconds: UInt64(sleepInterval) * 1_000_000_000)

} catch {
os_log("Error: %{public}@", log: .dataBrokerProtection, error.localizedDescription)
self.error = error

if let error = error as? DataBrokerProtectionError,
let dataBrokerName = brokerProfileQueriesData.first?.dataBroker.name {
pixelHandler.fire(.error(error: error, dataBroker: dataBrokerName))
}
errorDelegate?.dataBrokerOperationDidError(error, withBrokerName: brokerProfileQueriesData.first?.dataBroker.name)
}
}

Expand All @@ -222,3 +213,4 @@ final class DataBrokerOperation: Operation {
os_log("Finished operation: %{public}@", log: .dataBrokerProtection, String(describing: id.uuidString))
}
}
// swiftlint:enable explicit_non_final_class
Loading
Loading