From fad1361c35d61087b42898e1d587be75aaa1154f Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Fri, 9 Aug 2024 17:16:54 +0100 Subject: [PATCH 01/10] Subscription + content blocker Logging refactored --- .swiftlint.yml | 7 --- .../BrowserServicesKit-Package.xcscheme | 14 +++++ .../ContentBlockerRulesManager.swift | 31 +++++------ .../UserContentController.swift | 34 ++++++------ Sources/Common/DecodableHelper.swift | 1 + .../Common/Extensions/NSObjectExtension.swift | 1 + Sources/Common/Logging.swift | 42 ++++++++------- Sources/PixelKit/PixelKit.swift | 2 +- Sources/Subscription/API/APIService.swift | 9 ++-- .../AppStoreAccountManagementFlow.swift | 8 +-- .../Flows/AppStore/AppStorePurchaseFlow.swift | 14 ++--- .../Flows/AppStore/AppStoreRestoreFlow.swift | 16 +++--- .../Flows/Stripe/StripePurchaseFlow.swift | 12 ++--- .../Subscription/Logger+Subscription.swift | 25 +++++++++ .../Managers/AccountManager.swift | 13 ++--- .../Managers/StorePurchaseManager.swift | 54 +++++++++---------- 16 files changed, 159 insertions(+), 124 deletions(-) create mode 100644 Sources/Subscription/Logger+Subscription.swift diff --git a/.swiftlint.yml b/.swiftlint.yml index ce6e4cc7a..1888792bb 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -31,13 +31,6 @@ custom_rules: - keyword message: "Classes should be `final` by default, use explicit `internal` or `public` for non-final classes." severity: error - enforce_os_log_wrapper: - included: ".*\\.swift" - name: "Use `import Common` for os_log instead of `import os.log`" - regex: "^(import (?:os\\.log|os|OSLog))$" - capture_group: 0 - message: "os_log wrapper ensures log args are @autoclosures (computed when needed) and to be able to use String Interpolation." - severity: error analyzer_rules: # Rules run by `swiftlint analyze` - explicit_self diff --git a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme index d79d5c6e1..36f46def7 100644 --- a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme +++ b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme @@ -441,6 +441,20 @@ ReferencedContainer = "container:"> + + + + Void)? private let errorReporting: EventMapping? - private let getLog: () -> OSLog - private var log: OSLog { - getLog() - } // Public only for tests public var sourceManagers = [String: ContentBlockerRulesSourceManager]() @@ -143,14 +140,12 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { exceptionsSource: ContentBlockerRulesExceptionsSource, lastCompiledRulesStore: LastCompiledRulesStore? = nil, cache: ContentBlockerRulesCaching? = nil, - errorReporting: EventMapping? = nil, - log: @escaping @autoclosure () -> OSLog = .disabled) { + errorReporting: EventMapping? = nil) { self.rulesSource = rulesSource self.exceptionsSource = exceptionsSource self.lastCompiledRulesStore = lastCompiledRulesStore self.cache = cache self.errorReporting = errorReporting - self.getLog = log workQueue.async { _ = self.updateCompilationState(token: "") @@ -201,7 +196,7 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { @discardableResult public func scheduleCompilation() -> CompletionToken { let token = UUID().uuidString - os_log("Scheduling compilation with %{public}s", log: log, type: .default, token) + Logger.contentBlocking.log("Scheduling compilation with \(token, privacy: .public)") workQueue.async { let shouldStartCompilation = self.updateCompilationState(token: token) if shouldStartCompilation { @@ -213,7 +208,7 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { /// Returns true if the compilation should be executed immediately private func updateCompilationState(token: CompletionToken) -> Bool { - os_log("Requesting compilation...", log: log, type: .default) + Logger.contentBlocking.log("Requesting compilation...") lock.lock() guard case .idle = state else { if case .recompiling(let tokens) = state { @@ -237,16 +232,16 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { Returns true if rules were found, false otherwise. */ private func lookupCompiledRules() -> Bool { - os_log("Lookup compiled rules", log: log, type: .debug) + Logger.contentBlocking.debug("Lookup compiled rules") prepareSourceManagers() let initialCompilationTask = LookupRulesTask(sourceManagers: Array(sourceManagers.values)) let mutex = DispatchSemaphore(value: 0) - Task { [log] in + Task { do { try await initialCompilationTask.lookupCachedRulesLists() } catch { - os_log("❌ Lookup failed: %{public}s", log: log, type: .debug, error.localizedDescription) + Logger.contentBlocking.debug("❌ Lookup failed: \(error.localizedDescription, privacy: .public)") } mutex.signal() } @@ -255,7 +250,7 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { if let result = initialCompilationTask.result { let rules = result.map(Rules.init(compilationResult:)) - os_log("🟩 Found %{public}d rules", log: log, type: .debug, rules.count) + Logger.contentBlocking.debug("🟩 Found \(rules.count, privacy: .public) rules") applyRules(rules) return true } @@ -267,7 +262,7 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { Returns true if rules were found, false otherwise. */ private func fetchLastCompiledRules(with lastCompiledRules: [LastCompiledRules]) { - os_log("Fetch last compiled rules: %{public}d", log: log, type: .debug, lastCompiledRules.count) + Logger.contentBlocking.debug("Fetch last compiled rules: \(lastCompiledRules.count, privacy: .public)") let initialCompilationTask = LastCompiledRulesLookupTask(sourceRules: rulesSource.contentBlockerRulesLists, lastCompiledRules: lastCompiledRules) @@ -299,19 +294,17 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { manager.rulesList = rulesList sourceManager = manager } else { - let log = self.log sourceManager = ContentBlockerRulesSourceManager(rulesList: rulesList, exceptionsSource: self.exceptionsSource, errorReporting: self.errorReporting, - onCriticalError: self.onCriticalError, - log: log) + onCriticalError: self.onCriticalError) self.sourceManagers[rulesList.name] = sourceManager } } } private func startCompilationProcess() { - os_log("Starting compilataion process", log: log, type: .debug) + Logger.contentBlocking.debug("Starting compilataion process") prepareSourceManagers() // Prepare compilation tasks based on the sources @@ -368,7 +361,7 @@ public class ContentBlockerRulesManager: CompiledRuleListsSource { let newRules: [Rules] = currentTasks.compactMap { task in guard let result = task.result else { - os_log("Failed to complete task %{public}s ", log: self.log, type: .error, task.rulesList.name) + Logger.contentBlocking.debug("Failed to complete task \(task.rulesList.name, privacy: .public)") return nil } let rules = Rules(compilationResult: result) diff --git a/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift b/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift index 5a8942efe..2aa3fca82 100644 --- a/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift +++ b/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift @@ -21,6 +21,7 @@ import Common import UserScript import WebKit import QuartzCore +import os.log public protocol UserContentControllerDelegate: AnyObject { @MainActor @@ -80,14 +81,15 @@ final public class UserContentController: WKUserContentController { self.removeAllUserScripts() if let contentBlockingAssets = newValue { - os_log(.debug, log: .contentBlocking, "\(self): πŸ“š installing \(contentBlockingAssets)") + Logger.contentBlocking.debug("πŸ“š installing \(contentBlockingAssets.debugDescription)") self.installGlobalContentRuleLists(contentBlockingAssets.globalRuleLists) os_log(.debug, log: .userScripts, "\(self): πŸ“œ installing user scripts") self.installUserScripts(contentBlockingAssets.wkUserScripts, handlers: contentBlockingAssets.userScripts.userScripts) - os_log(.debug, log: .contentBlocking, "\(self): βœ… installing content blocking assets done") + Logger.contentBlocking.debug("βœ… installing content blocking assets done") } } } + @MainActor private func installContentBlockingAssets(_ contentBlockingAssets: ContentBlockingAssets) { // donβ€˜t install ContentBlockingAssets (especially Message Handlers retaining `self`) after cleanUpBeforeClosing was called @@ -124,7 +126,7 @@ final public class UserContentController: WKUserContentController { installUserScripts([], handlers: earlyAccessHandlers) assetsPublisherCancellable = assetsPublisher.sink { [weak self, selfDescr=self.debugDescription] content in - os_log(.debug, log: .contentBlocking, "\(selfDescr): πŸ“š received content blocking assets") + Logger.contentBlocking.debug("\(selfDescr): πŸ“š received content blocking assets") Task.detached { [weak self] in let contentBlockingAssets = await ContentBlockingAssets(content: content) await self?.installContentBlockingAssets(contentBlockingAssets) @@ -147,12 +149,12 @@ final public class UserContentController: WKUserContentController { private func installGlobalContentRuleLists(_ globalContentRuleLists: [String: WKContentRuleList]) { assert(contentRuleLists.isEmpty, "installGlobalContentRuleLists should be called after removing all Content Rule Lists") guard self.privacyConfigurationManager.privacyConfig.isEnabled(featureKey: .contentBlocking) else { - os_log(.debug, log: .contentBlocking, "\(self): ❗️ content blocking disabled, removing all content rule lists") + Logger.contentBlocking.debug("\(self): ❗️ content blocking disabled, removing all content rule lists") removeAllContentRuleLists() return } - os_log(.debug, log: .contentBlocking, "\(self): ❇️ installing global rule lists: \(globalContentRuleLists))") + Logger.contentBlocking.debug("\(self): ❇️ installing global rule lists: \(globalContentRuleLists))") contentRuleLists = globalContentRuleLists.reduce(into: [:]) { $0[.global($1.key)] = $1.value } @@ -166,12 +168,12 @@ final public class UserContentController: WKUserContentController { // when enabling from a $contentBlockingAssets subscription, the ruleList gets // to contentRuleLists before contentBlockingAssets value is set ?? contentRuleLists[.global(identifier)] else { - os_log(.debug, log: .contentBlocking, "\(self): ❗️ canβ€˜t enable rule list `\(identifier)` as itβ€˜s not available") + Logger.contentBlocking.debug("\(self): ❗️ canβ€˜t enable rule list `\(identifier)` as itβ€˜s not available") throw ContentRulesNotFoundError() } guard contentRuleLists[.global(identifier)] == nil else { return /* already enabled */ } - os_log(.debug, log: .contentBlocking, "\(self): 🟩 enabling rule list `\(identifier)`") + Logger.contentBlocking.debug("\(self): 🟩 enabling rule list `\(identifier)`") contentRuleLists[.global(identifier)] = ruleList add(ruleList) } @@ -180,11 +182,11 @@ final public class UserContentController: WKUserContentController { @MainActor public func disableGlobalContentRuleList(withIdentifier identifier: String) throws { guard let ruleList = contentRuleLists[.global(identifier)] else { - os_log(.debug, log: .contentBlocking, "\(self): ❗️ canβ€˜t disable rule list `\(identifier)` as itβ€˜s not enabled") + Logger.contentBlocking.debug("\(self): ❗️ canβ€˜t disable rule list `\(identifier)` as itβ€˜s not enabled") throw ContentRulesNotEnabledError() } - os_log(.debug, log: .contentBlocking, "\(self): πŸ”» disabling rule list `\(identifier)`") + Logger.contentBlocking.debug("\(self): πŸ”» disabling rule list `\(identifier)`") contentRuleLists[.global(identifier)] = nil remove(ruleList) } @@ -194,7 +196,7 @@ final public class UserContentController: WKUserContentController { // replace if already installed removeLocalContentRuleList(withIdentifier: identifier) - os_log(.debug, log: .contentBlocking, "\(self): πŸ”Έ installing local rule list `\(identifier)`") + Logger.contentBlocking.debug("\(self): πŸ”Έ installing local rule list `\(identifier)`") contentRuleLists[.local(identifier)] = ruleList add(ruleList) } @@ -203,13 +205,13 @@ final public class UserContentController: WKUserContentController { public func removeLocalContentRuleList(withIdentifier identifier: String) { guard let ruleList = contentRuleLists.removeValue(forKey: .local(identifier)) else { return } - os_log(.debug, log: .contentBlocking, "\(self): πŸ”» removing local rule list `\(identifier)`") + Logger.contentBlocking.debug("\(self): πŸ”» removing local rule list `\(identifier)`") remove(ruleList) } @MainActor public override func removeAllContentRuleLists() { - os_log(.debug, log: .contentBlocking, "\(self): 🧹 removing all content rule lists") + Logger.contentBlocking.debug("\(self): 🧹 removing all content rule lists") contentRuleLists.removeAll(keepingCapacity: true) super.removeAllContentRuleLists() } @@ -222,7 +224,7 @@ final public class UserContentController: WKUserContentController { @MainActor public func cleanUpBeforeClosing() { - os_log(.debug, log: .contentBlocking, "\(self): πŸ’€ cleanUpBeforeClosing") + Logger.contentBlocking.debug("\(self): πŸ’€ cleanUpBeforeClosing") self.removeAllUserScripts() @@ -275,7 +277,7 @@ public extension UserContentController { @MainActor var awaitContentBlockingAssetsInstalled: () async -> Void { guard !contentBlockingAssetsInstalled else { return {} } - os_log(.debug, log: .contentBlocking, "\(self): πŸ›‘ will wait for content blocking assets installed") + Logger.contentBlocking.debug("\(self): πŸ›‘ will wait for content blocking assets installed") let startTime = CACurrentMediaTime() return { [weak self, selfDescr=self.description] in // merge $contentBlockingAssets with Task cancellation completion event publisher @@ -297,7 +299,7 @@ public extension UserContentController { } cancellable = throwingPublisher.sink /* completion: */ { _ in withExtendedLifetime(cancellable) { - os_log(.debug, log: .contentBlocking, "\(selfDescr): ❌ wait cancelled after \(elapsedTime)") + Logger.contentBlocking.debug("\(selfDescr): ❌ wait cancelled after \(elapsedTime)") c.resume(with: .failure(CancellationError())) cancellable.cancel() @@ -305,7 +307,7 @@ public extension UserContentController { } receiveValue: { assets in guard assets != nil else { return } withExtendedLifetime(cancellable) { - os_log(.debug, log: .contentBlocking, "\(selfDescr): 🏁 content blocking assets installed (\(elapsedTime))") + Logger.contentBlocking.debug("\(selfDescr): 🏁 content blocking assets installed (\(elapsedTime))") c.resume(with: .success( () )) cancellable.cancel() diff --git a/Sources/Common/DecodableHelper.swift b/Sources/Common/DecodableHelper.swift index 5a66476be..e72a85bed 100644 --- a/Sources/Common/DecodableHelper.swift +++ b/Sources/Common/DecodableHelper.swift @@ -17,6 +17,7 @@ // import Foundation +import os.log public struct DecodableHelper { public static func decode(from input: Input) -> Target? { diff --git a/Sources/Common/Extensions/NSObjectExtension.swift b/Sources/Common/Extensions/NSObjectExtension.swift index 783bfecfa..d948ec2e9 100644 --- a/Sources/Common/Extensions/NSObjectExtension.swift +++ b/Sources/Common/Extensions/NSObjectExtension.swift @@ -18,6 +18,7 @@ import Combine import Foundation +import os.log extension NSObject { diff --git a/Sources/Common/Logging.swift b/Sources/Common/Logging.swift index c7493bc73..e987197e1 100644 --- a/Sources/Common/Logging.swift +++ b/Sources/Common/Logging.swift @@ -17,46 +17,49 @@ // import Foundation -import os // swiftlint:disable:this enforce_os_log_wrapper +import os.log + +public extension Logger { + private static let bundleIdentifier = Bundle.main.bundleIdentifier ?? "DuckDuckGo" + + static var contentBlocking: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Content Blocking") }() +// static var userScripts: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "User Scripts") }() +// static var passwordManager: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Password Manager") }() +// static var remoteMessaging: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Remote Messaging") }() +// static var subscription: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Subscription") }() +// static var history: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "History") }() +// static var general: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "General") }() +// static var autofill: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Autofill") }() +} public typealias OSLog = os.OSLog +@available(*, deprecated, message: "Use Logger.yourFeature instead, see https://app.asana.com/0/1202500774821704/1208001254061393/f") extension OSLog { - /// "exporting" `OSLog.disabled` symbol without needing to `import os.log` to disambiguate `os_log` wrapper calls and suppress "implicit import" warning - /// this declaration shadows the real `OSLog.disabled` - public static let disabled: OSLog = { - // the `OSLog` object returned by `OSLog.disabled` is in fact an `os_log_t *` pointer defined by C `_os_log_disabled` symbol - guard let disabledLog = dlsym(/*RTLD_DEFAULT*/ UnsafeMutableRawPointer(bitPattern: -2), "_os_log_disabled") else { - // just in case it fails for whatever reason (but it shouldnβ€˜t) - return some log object - assertionFailure("_os_log_disabled symbol not found") - return .init(subsystem: "", category: "") - } - return unsafeBitCast(disabledLog, to: OSLog.self) - }() - public enum Categories: String, CaseIterable { - case contentBlocking = "Content Blocking" case userScripts = "User Scripts" case passwordManager = "Password Manager" case remoteMessaging = "Remote Messaging" - case subscription = "Subscription" case history = "History" case general = "General" case autofill = "Autofill" } #if DEBUG - // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! // To activate Logging Categories for DEBUG add categories here: - static var debugCategories: Set = [ /*.autofill*/ ] + static var debugCategories: Set = [ + .userScripts, + .passwordManager, + .remoteMessaging, + .history, + .general, + .autofill] #endif - @OSLogWrapper(.contentBlocking) public static var contentBlocking @OSLogWrapper(.userScripts) public static var userScripts @OSLogWrapper(.passwordManager) public static var passwordManager @OSLogWrapper(.remoteMessaging) public static var remoteMessaging - @OSLogWrapper(.subscription) public static var subscription @OSLogWrapper(.history) public static var history @OSLogWrapper(.general) public static var general @OSLogWrapper(.autofill) public static var autofill @@ -70,6 +73,7 @@ extension OSLog { static let subsystem = Bundle.main.bundleIdentifier ?? "DuckDuckGo" + @available(*, deprecated, message: "Use Logger.yourFeature instead, see https://app.asana.com/0/1202500774821704/1208001254061393/f") @propertyWrapper public struct OSLogWrapper { diff --git a/Sources/PixelKit/PixelKit.swift b/Sources/PixelKit/PixelKit.swift index c2e760976..3586415e7 100644 --- a/Sources/PixelKit/PixelKit.swift +++ b/Sources/PixelKit/PixelKit.swift @@ -17,7 +17,7 @@ // import Foundation -import os.log // swiftlint:disable:this enforce_os_log_wrapper +import os.log public final class PixelKit { /// `true` if a request is fired, `false` otherwise diff --git a/Sources/Subscription/API/APIService.swift b/Sources/Subscription/API/APIService.swift index 647a21c48..41c634706 100644 --- a/Sources/Subscription/API/APIService.swift +++ b/Sources/Subscription/API/APIService.swift @@ -18,6 +18,7 @@ import Foundation import Common +import os.log public enum APIServiceError: Swift.Error { case decodingError @@ -65,7 +66,7 @@ public struct DefaultAPIService: APIService { if let decodedResponse = decode(T.self, from: data) { return .success(decodedResponse) } else { - os_log(.error, log: .subscription, "Service error: APIServiceError.decodingError") + Logger.subscription.error("Service error: APIServiceError.decodingError") return .failure(.decodingError) } } else { @@ -76,11 +77,11 @@ public struct DefaultAPIService: APIService { } let errorLogMessage = "/\(endpoint) \(httpResponse.statusCode): \(errorString ?? "")" - os_log(.error, log: .subscription, "Service error: %{public}@", errorLogMessage) + Logger.subscription.error("Service error: \(errorLogMessage, privacy: .public)") return .failure(.serverError(statusCode: httpResponse.statusCode, error: errorString)) } } catch { - os_log(.error, log: .subscription, "Service error: %{public}@", error.localizedDescription) + Logger.subscription.error("Service error: \(error.localizedDescription, privacy: .public)") return .failure(.connectionError) } } @@ -111,7 +112,7 @@ public struct DefaultAPIService: APIService { let statusCode = (response as? HTTPURLResponse)!.statusCode let stringData = String(data: data, encoding: .utf8) ?? "" - os_log(.info, log: .subscription, "[API] %d %{public}s /%{public}s :: %{public}s", statusCode, method, endpoint, stringData) + Logger.subscription.info("[API] \(statusCode) \(method, privacy: .public) \(endpoint, privacy: .public) :: \(stringData, privacy: .public)") } public func makeAuthorizationHeader(for token: String) -> [String: String] { diff --git a/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift b/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift index acbf4ab82..bb955ccde 100644 --- a/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStoreAccountManagementFlow.swift @@ -18,7 +18,7 @@ import Foundation import StoreKit -import Common +import os.log public enum AppStoreAccountManagementFlowError: Swift.Error { case noPastTransaction @@ -45,12 +45,12 @@ public final class DefaultAppStoreAccountManagementFlow: AppStoreAccountManageme @discardableResult public func refreshAuthTokenIfNeeded() async -> Result { - os_log(.info, log: .subscription, "[AppStoreAccountManagementFlow] refreshAuthTokenIfNeeded") + Logger.subscription.info("[AppStoreAccountManagementFlow] refreshAuthTokenIfNeeded") var authToken = accountManager.authToken ?? "" // Check if auth token if still valid if case let .failure(validateTokenError) = await authEndpointService.validateToken(accessToken: authToken) { - os_log(.error, log: .subscription, "[AppStoreAccountManagementFlow] validateToken error: %{public}s", String(reflecting: validateTokenError)) + Logger.subscription.error("[AppStoreAccountManagementFlow] validateToken error: \(String(reflecting: validateTokenError), privacy: .public)") // In case of invalid token attempt store based authentication to obtain a new one guard let lastTransactionJWSRepresentation = await storePurchaseManager.mostRecentTransaction() else { return .failure(.noPastTransaction) } @@ -62,7 +62,7 @@ public final class DefaultAppStoreAccountManagementFlow: AppStoreAccountManageme accountManager.storeAuthToken(token: authToken) } case .failure(let storeLoginError): - os_log(.error, log: .subscription, "[AppStoreAccountManagementFlow] storeLogin error: %{public}s", String(reflecting: storeLoginError)) + Logger.subscription.error("[AppStoreAccountManagementFlow] storeLogin error: \(String(reflecting: storeLoginError), privacy: .public)") return .failure(.authenticatingWithTransactionFailed) } } diff --git a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift index 4239be257..22896cd5a 100644 --- a/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStorePurchaseFlow.swift @@ -18,7 +18,7 @@ import Foundation import StoreKit -import Common +import os.log public enum AppStorePurchaseFlowError: Swift.Error { case noProductsFound @@ -60,7 +60,7 @@ public final class DefaultAppStorePurchaseFlow: AppStorePurchaseFlow { } public func purchaseSubscription(with subscriptionIdentifier: String, emailAccessToken: String?) async -> Result { - os_log(.info, log: .subscription, "[AppStorePurchaseFlow] purchaseSubscription") + Logger.subscription.info("[AppStorePurchaseFlow] purchaseSubscription") let externalID: String // If the current account is a third party expired account, we want to purchase and attach subs to it @@ -72,10 +72,10 @@ public final class DefaultAppStorePurchaseFlow: AppStorePurchaseFlow { // Check for past transactions most recent switch await appStoreRestoreFlow.restoreAccountFromPastPurchase() { case .success: - os_log(.info, log: .subscription, "[AppStorePurchaseFlow] purchaseSubscription -> restoreAccountFromPastPurchase: activeSubscriptionAlreadyPresent") + Logger.subscription.info("[AppStorePurchaseFlow] purchaseSubscription -> restoreAccountFromPastPurchase: activeSubscriptionAlreadyPresent") return .failure(.activeSubscriptionAlreadyPresent) case .failure(let error): - os_log(.info, log: .subscription, "[AppStorePurchaseFlow] purchaseSubscription -> restoreAccountFromPastPurchase: %{public}s", String(reflecting: error)) + Logger.subscription.info("[AppStorePurchaseFlow] purchaseSubscription -> restoreAccountFromPastPurchase: \(String(reflecting: error), privacy: .public)") switch error { case .subscriptionExpired(let expiredAccountDetails): externalID = expiredAccountDetails.externalID @@ -92,7 +92,7 @@ public final class DefaultAppStorePurchaseFlow: AppStorePurchaseFlow { accountManager.storeAccount(token: accessToken, email: accountDetails.email, externalID: accountDetails.externalID) } case .failure(let error): - os_log(.error, log: .subscription, "[AppStorePurchaseFlow] createAccount error: %{public}s", String(reflecting: error)) + Logger.subscription.error("[AppStorePurchaseFlow] createAccount error: \(String(reflecting: error), privacy: .public)") return .failure(.accountCreationFailed) } } @@ -104,7 +104,7 @@ public final class DefaultAppStorePurchaseFlow: AppStorePurchaseFlow { case .success(let transactionJWS): return .success(transactionJWS) case .failure(let error): - os_log(.error, log: .subscription, "[AppStorePurchaseFlow] purchaseSubscription error: %{public}s", String(reflecting: error)) + Logger.subscription.error("[AppStorePurchaseFlow] purchaseSubscription error: \(String(reflecting: error), privacy: .public)") accountManager.signOut(skipNotification: true) switch error { case .purchaseCancelledByUser: @@ -121,7 +121,7 @@ public final class DefaultAppStorePurchaseFlow: AppStorePurchaseFlow { // Clear subscription Cache subscriptionEndpointService.signOut() - os_log(.info, log: .subscription, "[AppStorePurchaseFlow] completeSubscriptionPurchase") + Logger.subscription.info("[AppStorePurchaseFlow] completeSubscriptionPurchase") guard let accessToken = accountManager.accessToken else { return .failure(.missingEntitlements) } diff --git a/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift b/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift index 15a8b5c94..03055379a 100644 --- a/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift +++ b/Sources/Subscription/Flows/AppStore/AppStoreRestoreFlow.swift @@ -18,7 +18,7 @@ import Foundation import StoreKit -import Common +import os.log public typealias RestoredAccountDetails = (authToken: String, accessToken: String, externalID: String, email: String?) @@ -59,10 +59,10 @@ public final class DefaultAppStoreRestoreFlow: AppStoreRestoreFlow { // Clear subscription Cache subscriptionEndpointService.signOut() - os_log(.info, log: .subscription, "[AppStoreRestoreFlow] restoreAccountFromPastPurchase") + Logger.subscription.info("[AppStoreRestoreFlow] restoreAccountFromPastPurchase") guard let lastTransactionJWSRepresentation = await storePurchaseManager.mostRecentTransaction() else { - os_log(.error, log: .subscription, "[AppStoreRestoreFlow] Error: missingAccountOrTransactions") + Logger.subscription.error("[AppStoreRestoreFlow] Error: missingAccountOrTransactions") return .failure(.missingAccountOrTransactions) } @@ -73,7 +73,7 @@ public final class DefaultAppStoreRestoreFlow: AppStoreRestoreFlow { case .success(let response): authToken = response.authToken case .failure: - os_log(.error, log: .subscription, "[AppStoreRestoreFlow] Error: pastTransactionAuthenticationError") + Logger.subscription.error("[AppStoreRestoreFlow] Error: pastTransactionAuthenticationError") return .failure(.pastTransactionAuthenticationError) } @@ -85,7 +85,7 @@ public final class DefaultAppStoreRestoreFlow: AppStoreRestoreFlow { case .success(let exchangedAccessToken): accessToken = exchangedAccessToken case .failure: - os_log(.error, log: .subscription, "[AppStoreRestoreFlow] Error: failedToObtainAccessToken") + Logger.subscription.error("[AppStoreRestoreFlow] Error: failedToObtainAccessToken") return .failure(.failedToObtainAccessToken) } @@ -94,7 +94,7 @@ public final class DefaultAppStoreRestoreFlow: AppStoreRestoreFlow { email = accountDetails.email externalID = accountDetails.externalID case .failure: - os_log(.error, log: .subscription, "[AppStoreRestoreFlow] Error: failedToFetchAccountDetails") + Logger.subscription.error("[AppStoreRestoreFlow] Error: failedToFetchAccountDetails") return .failure(.failedToFetchAccountDetails) } @@ -104,7 +104,7 @@ public final class DefaultAppStoreRestoreFlow: AppStoreRestoreFlow { case .success(let subscription): isSubscriptionActive = subscription.isActive case .failure: - os_log(.error, log: .subscription, "[AppStoreRestoreFlow] Error: failedToFetchSubscriptionDetails") + Logger.subscription.error("[AppStoreRestoreFlow] Error: failedToFetchSubscriptionDetails") return .failure(.failedToFetchSubscriptionDetails) } @@ -114,7 +114,7 @@ public final class DefaultAppStoreRestoreFlow: AppStoreRestoreFlow { return .success(()) } else { let details = RestoredAccountDetails(authToken: authToken, accessToken: accessToken, externalID: externalID, email: email) - os_log(.error, log: .subscription, "[AppStoreRestoreFlow] Error: subscriptionExpired") + Logger.subscription.error("[AppStoreRestoreFlow] Error: subscriptionExpired") return .failure(.subscriptionExpired(accountDetails: details)) } } diff --git a/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift b/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift index 2e561882a..eaab96697 100644 --- a/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift +++ b/Sources/Subscription/Flows/Stripe/StripePurchaseFlow.swift @@ -18,7 +18,7 @@ import Foundation import StoreKit -import Common +import os.log public enum StripePurchaseFlowError: Swift.Error { case noProductsFound @@ -45,10 +45,10 @@ public final class DefaultStripePurchaseFlow: StripePurchaseFlow { } public func subscriptionOptions() async -> Result { - os_log(.info, log: .subscription, "[StripePurchaseFlow] subscriptionOptions") + Logger.subscription.info("[StripePurchaseFlow] subscriptionOptions") guard case let .success(products) = await subscriptionEndpointService.getProducts(), !products.isEmpty else { - os_log(.error, log: .subscription, "[StripePurchaseFlow] Error: noProductsFound") + Logger.subscription.error("[StripePurchaseFlow] Error: noProductsFound") return .failure(.noProductsFound) } @@ -79,7 +79,7 @@ public final class DefaultStripePurchaseFlow: StripePurchaseFlow { } public func prepareSubscriptionPurchase(emailAccessToken: String?) async -> Result { - os_log(.info, log: .subscription, "[StripePurchaseFlow] prepareSubscriptionPurchase") + Logger.subscription.info("[StripePurchaseFlow] prepareSubscriptionPurchase") // Clear subscription Cache subscriptionEndpointService.signOut() @@ -95,7 +95,7 @@ public final class DefaultStripePurchaseFlow: StripePurchaseFlow { token = response.authToken accountManager.storeAuthToken(token: token) case .failure: - os_log(.error, log: .subscription, "[StripePurchaseFlow] Error: accountCreationFailed") + Logger.subscription.error("[StripePurchaseFlow] Error: accountCreationFailed") return .failure(.accountCreationFailed) } } @@ -115,7 +115,7 @@ public final class DefaultStripePurchaseFlow: StripePurchaseFlow { // Clear subscription Cache subscriptionEndpointService.signOut() - os_log(.info, log: .subscription, "[StripePurchaseFlow] completeSubscriptionPurchase") + Logger.subscription.info("[StripePurchaseFlow] completeSubscriptionPurchase") if !accountManager.isUserAuthenticated, let authToken = accountManager.authToken { if case let .success(accessToken) = await accountManager.exchangeAuthTokenToAccessToken(authToken), diff --git a/Sources/Subscription/Logger+Subscription.swift b/Sources/Subscription/Logger+Subscription.swift new file mode 100644 index 000000000..3b564483f --- /dev/null +++ b/Sources/Subscription/Logger+Subscription.swift @@ -0,0 +1,25 @@ +// +// File.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 Foundation +import os.log + +public extension Logger { + static var subscription: Logger = { Logger(subsystem: Bundle.main.bundleIdentifier ?? "DuckDuckGo", category: "Subscription") }() +} diff --git a/Sources/Subscription/Managers/AccountManager.swift b/Sources/Subscription/Managers/AccountManager.swift index caf5e8db1..951432a6f 100644 --- a/Sources/Subscription/Managers/AccountManager.swift +++ b/Sources/Subscription/Managers/AccountManager.swift @@ -18,6 +18,7 @@ import Foundation import Common +import os.log public protocol AccountManagerKeychainAccessDelegate: AnyObject { func accountManagerKeychainAccessFailed(accessType: AccountKeychainAccessType, error: AccountKeychainAccessError) @@ -146,7 +147,7 @@ public final class DefaultAccountManager: AccountManager { } public func storeAuthToken(token: String) { - os_log(.info, log: .subscription, "[AccountManager] storeAuthToken") + Logger.subscription.info("[AccountManager] storeAuthToken") do { try storage.store(authToken: token) @@ -160,7 +161,7 @@ public final class DefaultAccountManager: AccountManager { } public func storeAccount(token: String, email: String?, externalID: String?) { - os_log(.info, log: .subscription, "[AccountManager] storeAccount") + Logger.subscription.info("[AccountManager] storeAccount") do { try accessTokenStorage.store(accessToken: token) @@ -199,7 +200,7 @@ public final class DefaultAccountManager: AccountManager { } public func signOut(skipNotification: Bool = false) { - os_log(.info, log: .subscription, "[AccountManager] signOut") + Logger.subscription.info("[AccountManager] signOut") do { try storage.clearAuthenticationState() @@ -264,7 +265,7 @@ public final class DefaultAccountManager: AccountManager { return .success(entitlements) case .failure(let error): - os_log(.error, log: .subscription, "[AccountManager] fetchEntitlements error: %{public}@", error.localizedDescription) + Logger.subscription.error("[AccountManager] fetchEntitlements error: \(error.localizedDescription, privacy: .public)") return .failure(error) } } @@ -316,7 +317,7 @@ public final class DefaultAccountManager: AccountManager { case .success(let response): return .success(response.accessToken) case .failure(let error): - os_log(.error, log: .subscription, "[AccountManager] exchangeAuthTokenToAccessToken error: %{public}@", error.localizedDescription) + Logger.subscription.error("[AccountManager] exchangeAuthTokenToAccessToken error: \(error.localizedDescription, privacy: .public)") return .failure(error) } } @@ -326,7 +327,7 @@ public final class DefaultAccountManager: AccountManager { case .success(let response): return .success(AccountDetails(email: response.account.email, externalID: response.account.externalID)) case .failure(let error): - os_log(.error, log: .subscription, "[AccountManager] fetchAccountDetails error: %{public}@", error.localizedDescription) + Logger.subscription.error("[AccountManager] fetchAccountDetails error: \(error.localizedDescription, privacy: .public)") return .failure(error) } } diff --git a/Sources/Subscription/Managers/StorePurchaseManager.swift b/Sources/Subscription/Managers/StorePurchaseManager.swift index 25f550d8c..765f00252 100644 --- a/Sources/Subscription/Managers/StorePurchaseManager.swift +++ b/Sources/Subscription/Managers/StorePurchaseManager.swift @@ -18,7 +18,7 @@ import Foundation import StoreKit -import Common +import os.log public enum StoreError: Error { case failedVerification @@ -85,27 +85,27 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM do { purchaseQueue.removeAll() - os_log(.info, log: .subscription, "[StorePurchaseManager] Before AppStore.sync()") + Logger.subscription.info("[StorePurchaseManager] Before AppStore.sync()") try await AppStore.sync() - os_log(.info, log: .subscription, "[StorePurchaseManager] After AppStore.sync()") + Logger.subscription.info("[StorePurchaseManager] After AppStore.sync()") await updatePurchasedProducts() await updateAvailableProducts() } catch { - os_log(.error, log: .subscription, "[StorePurchaseManager] Error: %{public}s (%{public}s)", String(reflecting: error), error.localizedDescription) + Logger.subscription.error("[StorePurchaseManager] Error: \(String(reflecting: error), privacy: .public) (\(error.localizedDescription, privacy: .public))") throw error } } public func subscriptionOptions() async -> SubscriptionOptions? { - os_log(.info, log: .subscription, "[AppStorePurchaseFlow] subscriptionOptions") + Logger.subscription.info("[AppStorePurchaseFlow] subscriptionOptions") let products = availableProducts let monthly = products.first(where: { $0.subscription?.subscriptionPeriod.unit == .month && $0.subscription?.subscriptionPeriod.value == 1 }) let yearly = products.first(where: { $0.subscription?.subscriptionPeriod.unit == .year && $0.subscription?.subscriptionPeriod.value == 1 }) guard let monthly, let yearly else { - os_log(.error, log: .subscription, "[AppStorePurchaseFlow] No products found") + Logger.subscription.error("[AppStorePurchaseFlow] No products found") return nil } @@ -126,23 +126,23 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM @MainActor public func updateAvailableProducts() async { - os_log(.info, log: .subscription, "[StorePurchaseManager] updateAvailableProducts") + Logger.subscription.info("[StorePurchaseManager] updateAvailableProducts") do { let availableProducts = try await Product.products(for: productIdentifiers) - os_log(.info, log: .subscription, "[StorePurchaseManager] updateAvailableProducts fetched %d products", availableProducts.count) + Logger.subscription.info("[StorePurchaseManager] updateAvailableProducts fetched \(availableProducts.count) products") if self.availableProducts != availableProducts { self.availableProducts = availableProducts } } catch { - os_log(.error, log: .subscription, "[StorePurchaseManager] Error: %{public}s", String(reflecting: error)) + Logger.subscription.error("[StorePurchaseManager] Error: \(String(reflecting: error), privacy: .public)") } } @MainActor public func updatePurchasedProducts() async { - os_log(.info, log: .subscription, "[StorePurchaseManager] updatePurchasedProducts") + Logger.subscription.info("[StorePurchaseManager] updatePurchasedProducts") var purchasedSubscriptions: [String] = [] @@ -158,10 +158,10 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM } } } catch { - os_log(.error, log: .subscription, "[StorePurchaseManager] Error: %{public}s", String(reflecting: error)) + Logger.subscription.error("[StorePurchaseManager] Error: \(String(reflecting: error), privacy: .public)") } - os_log(.info, log: .subscription, "[StorePurchaseManager] updatePurchasedProducts fetched %d active subscriptions", purchasedSubscriptions.count) + Logger.subscription.info("[StorePurchaseManager] updatePurchasedProducts fetched \(purchasedSubscriptions.count) active subscriptions") if self.purchasedProductIDs != purchasedSubscriptions { self.purchasedProductIDs = purchasedSubscriptions @@ -172,7 +172,7 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM @MainActor public func mostRecentTransaction() async -> String? { - os_log(.info, log: .subscription, "[StorePurchaseManager] mostRecentTransaction") + Logger.subscription.info("[StorePurchaseManager] mostRecentTransaction") var transactions: [VerificationResult] = [] @@ -180,14 +180,14 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM transactions.append(result) } - os_log(.info, log: .subscription, "[StorePurchaseManager] mostRecentTransaction fetched %d transactions", transactions.count) + Logger.subscription.info("[StorePurchaseManager] mostRecentTransaction fetched \(transactions.count) transactions") return transactions.first?.jwsRepresentation } @MainActor public func hasActiveSubscription() async -> Bool { - os_log(.info, log: .subscription, "[StorePurchaseManager] hasActiveSubscription") + Logger.subscription.info("[StorePurchaseManager] hasActiveSubscription") var transactions: [VerificationResult] = [] @@ -195,7 +195,7 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM transactions.append(result) } - os_log(.info, log: .subscription, "[StorePurchaseManager] hasActiveSubscription fetched %d transactions", transactions.count) + Logger.subscription.info("[StorePurchaseManager] hasActiveSubscription fetched \(transactions.count) transactions") return !transactions.isEmpty } @@ -205,7 +205,7 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM guard let product = availableProducts.first(where: { $0.id == identifier }) else { return .failure(StorePurchaseManagerError.productNotFound) } - os_log(.info, log: .subscription, "[StorePurchaseManager] purchaseSubscription %{public}s (%{public}s)", product.displayName, externalID) + Logger.subscription.info("[StorePurchaseManager] purchaseSubscription \(product.displayName, privacy: .public) (\(externalID, privacy: .public))") purchaseQueue.append(product.id) @@ -214,7 +214,7 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM if let token = UUID(uuidString: externalID) { options.insert(.appAccountToken(token)) } else { - os_log(.error, log: .subscription, "[StorePurchaseManager] Error: Failed to create UUID") + Logger.subscription.error("[StorePurchaseManager] Error: Failed to create UUID") return .failure(StorePurchaseManagerError.externalIDisNotAValidUUID) } @@ -222,11 +222,11 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM do { purchaseResult = try await product.purchase(options: options) } catch { - os_log(.error, log: .subscription, "[StorePurchaseManager] Error: %{public}s", String(reflecting: error)) + Logger.subscription.error("[StorePurchaseManager] Error: \(String(reflecting: error), privacy: .public)") return .failure(StorePurchaseManagerError.purchaseFailed) } - os_log(.info, log: .subscription, "[StorePurchaseManager] purchaseSubscription complete") + Logger.subscription.info("[StorePurchaseManager] purchaseSubscription complete") purchaseQueue.removeAll() @@ -234,27 +234,27 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM case let .success(verificationResult): switch verificationResult { case let .verified(transaction): - os_log(.info, log: .subscription, "[StorePurchaseManager] purchaseSubscription result: success") + Logger.subscription.info("[StorePurchaseManager] purchaseSubscription result: success") // Successful purchase await transaction.finish() await self.updatePurchasedProducts() return .success(verificationResult.jwsRepresentation) case let .unverified(_, error): - os_log(.info, log: .subscription, "[StorePurchaseManager] purchaseSubscription result: success /unverified/ - %{public}s", String(reflecting: error)) + Logger.subscription.info("[StorePurchaseManager] purchaseSubscription result: success /unverified/ - \(String(reflecting: error), privacy: .public)") // Successful purchase but transaction/receipt can't be verified // Could be a jailbroken phone return .failure(StorePurchaseManagerError.transactionCannotBeVerified) } case .pending: - os_log(.info, log: .subscription, "[StorePurchaseManager] purchaseSubscription result: pending") + Logger.subscription.info("[StorePurchaseManager] purchaseSubscription result: pending") // Transaction waiting on SCA (Strong Customer Authentication) or // approval from Ask to Buy return .failure(StorePurchaseManagerError.transactionPendingAuthentication) case .userCancelled: - os_log(.info, log: .subscription, "[StorePurchaseManager] purchaseSubscription result: user cancelled") + Logger.subscription.info("[StorePurchaseManager] purchaseSubscription result: user cancelled") return .failure(StorePurchaseManagerError.purchaseCancelledByUser) @unknown default: - os_log(.info, log: .subscription, "[StorePurchaseManager] purchaseSubscription result: unknown") + Logger.subscription.info("[StorePurchaseManager] purchaseSubscription result: unknown") return .failure(StorePurchaseManagerError.unknownError) } } @@ -275,7 +275,7 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM Task.detached { [weak self] in for await result in Transaction.updates { - os_log(.info, log: .subscription, "[StorePurchaseManager] observeTransactionUpdates") + Logger.subscription.info("[StorePurchaseManager] observeTransactionUpdates") if case .verified(let transaction) = result { await transaction.finish() @@ -290,7 +290,7 @@ public final class DefaultStorePurchaseManager: ObservableObject, StorePurchaseM Task.detached { [weak self] in for await result in Storefront.updates { - os_log(.info, log: .subscription, "[StorePurchaseManager] observeStorefrontChanges: %s", result.countryCode) + Logger.subscription.info("[StorePurchaseManager] observeStorefrontChanges: \(result.countryCode)") await self?.updatePurchasedProducts() await self?.updateAvailableProducts() } From 1af483544c6a73bb3fbb05840202f005c765d69a Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Tue, 13 Aug 2024 10:27:46 +0100 Subject: [PATCH 02/10] lint --- .swiftlint.yml | 2 ++ Sources/Subscription/Logger+Subscription.swift | 5 ++--- .../Resources/privacy-reference-tests | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 1888792bb..7d91e82a7 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -16,6 +16,8 @@ disabled_rules: - function_body_length - file_length - type_body_length + - static_over_final_class + - non_optional_string_data_conversion opt_in_rules: - file_header diff --git a/Sources/Subscription/Logger+Subscription.swift b/Sources/Subscription/Logger+Subscription.swift index 3b564483f..a883da272 100644 --- a/Sources/Subscription/Logger+Subscription.swift +++ b/Sources/Subscription/Logger+Subscription.swift @@ -1,8 +1,7 @@ // -// File.swift -// DuckDuckGo +// Logger+Subscription.swift // -// Copyright Β© 2024 DuckDuckGo. All rights reserved. +// Copyright Β© 2023 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. diff --git a/Tests/BrowserServicesKitTests/Resources/privacy-reference-tests b/Tests/BrowserServicesKitTests/Resources/privacy-reference-tests index a603ff9af..afb4f6128 160000 --- a/Tests/BrowserServicesKitTests/Resources/privacy-reference-tests +++ b/Tests/BrowserServicesKitTests/Resources/privacy-reference-tests @@ -1 +1 @@ -Subproject commit a603ff9af22ca3ff7ce2e7ffbfe18c447d9f23e8 +Subproject commit afb4f6128a3b50d53ddcb1897ea1fb4df6858aa1 From 3a7819a3d147e6c0d976c49b291b7f7e87101415 Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Tue, 13 Aug 2024 10:38:53 +0100 Subject: [PATCH 03/10] user script logs refactored --- .../BrowserServicesKit/Autofill/AutofillUserScript.swift | 6 +++--- .../ContentScopeScript/UserContentController.swift | 2 +- Sources/Common/Logging.swift | 5 +---- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift b/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift index dd2f379c7..b7bfa2e11 100644 --- a/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift +++ b/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift @@ -19,6 +19,7 @@ import Common import WebKit import UserScript +import os.log var previousIncontextSignupPermanentlyDismissedAt: Double? var previousEmailSignedIn: Bool? @@ -115,11 +116,10 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti public func messageHandlerFor(_ messageName: String) -> MessageHandler? { guard let message = MessageName(rawValue: messageName) else { - os_log("Failed to parse Autofill User Script message: '%{public}s'", log: .userScripts, type: .debug, messageName) + Logger.userScripts.error("Failed to parse Autofill User Script message: '\(messageName)'") return nil } - - os_log("AutofillUserScript: received '%{public}s'", log: .userScripts, type: .debug, messageName) + Logger.userScripts.debug("AutofillUserScript: received '\(messageName)'") switch message { case .emailHandlerStoreToken: return emailStoreToken diff --git a/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift b/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift index 2aa3fca82..d304212ac 100644 --- a/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift +++ b/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift @@ -83,7 +83,7 @@ final public class UserContentController: WKUserContentController { if let contentBlockingAssets = newValue { Logger.contentBlocking.debug("πŸ“š installing \(contentBlockingAssets.debugDescription)") self.installGlobalContentRuleLists(contentBlockingAssets.globalRuleLists) - os_log(.debug, log: .userScripts, "\(self): πŸ“œ installing user scripts") + Logger.userScripts.debug("πŸ“œ installing user scripts") self.installUserScripts(contentBlockingAssets.wkUserScripts, handlers: contentBlockingAssets.userScripts.userScripts) Logger.contentBlocking.debug("βœ… installing content blocking assets done") } diff --git a/Sources/Common/Logging.swift b/Sources/Common/Logging.swift index e987197e1..9e6eb7673 100644 --- a/Sources/Common/Logging.swift +++ b/Sources/Common/Logging.swift @@ -23,7 +23,7 @@ public extension Logger { private static let bundleIdentifier = Bundle.main.bundleIdentifier ?? "DuckDuckGo" static var contentBlocking: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Content Blocking") }() -// static var userScripts: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "User Scripts") }() + static var userScripts: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "User Scripts") }() // static var passwordManager: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Password Manager") }() // static var remoteMessaging: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Remote Messaging") }() // static var subscription: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Subscription") }() @@ -38,7 +38,6 @@ public typealias OSLog = os.OSLog extension OSLog { public enum Categories: String, CaseIterable { - case userScripts = "User Scripts" case passwordManager = "Password Manager" case remoteMessaging = "Remote Messaging" case history = "History" @@ -49,7 +48,6 @@ extension OSLog { #if DEBUG // To activate Logging Categories for DEBUG add categories here: static var debugCategories: Set = [ - .userScripts, .passwordManager, .remoteMessaging, .history, @@ -57,7 +55,6 @@ extension OSLog { .autofill] #endif - @OSLogWrapper(.userScripts) public static var userScripts @OSLogWrapper(.passwordManager) public static var passwordManager @OSLogWrapper(.remoteMessaging) public static var remoteMessaging @OSLogWrapper(.history) public static var history From f36b09b139a903ab7fdf4e4e8a63a6e897d0a00a Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Tue, 13 Aug 2024 12:14:18 +0100 Subject: [PATCH 04/10] SecureVault logs --- .../CredentialsDatabaseCleaner.swift | 17 +++----- .../SecureVault/Logger+SecureVault.swift | 25 +++++++++++ .../SecureVault/SecureVaultManager.swift | 43 ++++++++++--------- Sources/Common/Logging.swift | 5 +-- 4 files changed, 53 insertions(+), 37 deletions(-) create mode 100644 Sources/BrowserServicesKit/SecureVault/Logger+SecureVault.swift diff --git a/Sources/BrowserServicesKit/SecureVault/CredentialsDatabaseCleaner.swift b/Sources/BrowserServicesKit/SecureVault/CredentialsDatabaseCleaner.swift index c4e974101..34f88c0fd 100644 --- a/Sources/BrowserServicesKit/SecureVault/CredentialsDatabaseCleaner.swift +++ b/Sources/BrowserServicesKit/SecureVault/CredentialsDatabaseCleaner.swift @@ -21,6 +21,7 @@ import Combine import Common import GRDB import SecureStorage +import os.log public struct CredentialsCleanupError: Error { public let cleanupError: Error @@ -37,14 +38,12 @@ public final class CredentialsDatabaseCleaner { public convenience init( secureVaultFactory: AutofillVaultFactory, secureVaultErrorReporter: SecureVaultReporting, - errorEvents: EventMapping?, - log: @escaping @autoclosure () -> OSLog = .disabled + errorEvents: EventMapping? ) { self.init( secureVaultFactory: secureVaultFactory, secureVaultErrorReporter: secureVaultErrorReporter, errorEvents: errorEvents, - log: log(), removeSyncMetadataPendingDeletion: Self.removeSyncMetadataPendingDeletion(in:) ) } @@ -53,13 +52,11 @@ public final class CredentialsDatabaseCleaner { secureVaultFactory: AutofillVaultFactory, secureVaultErrorReporter: SecureVaultReporting, errorEvents: EventMapping?, - log: @escaping @autoclosure () -> OSLog = .disabled, removeSyncMetadataPendingDeletion: @escaping ((Database) throws -> Int) ) { self.secureVaultFactory = secureVaultFactory self.secureVaultErrorReporter = secureVaultErrorReporter self.errorEvents = errorEvents - self.getLog = log self.removeSyncMetadataPendingDeletion = removeSyncMetadataPendingDeletion cleanupCancellable = Publishers @@ -108,9 +105,10 @@ public final class CredentialsDatabaseCleaner { numberOfDeletedEntries = try self.removeSyncMetadataPendingDeletion(database) } if numberOfDeletedEntries == 0 { - os_log(.debug, log: log, "No syncable credentials metadata pending deletion") + Logger.secureVault.debug("No syncable credentials metadata pending deletion") + } else { - os_log(.debug, log: log, "Successfully purged %{public}d sync credentials metadata", numberOfDeletedEntries) + Logger.secureVault.debug("Successfully purged %{public}d sync credentials metadata") } break } catch { @@ -161,9 +159,4 @@ public final class CredentialsDatabaseCleaner { private var cleanupCancellable: AnyCancellable? private var scheduleCleanupCancellable: AnyCancellable? private let removeSyncMetadataPendingDeletion: (Database) throws -> Int - - private let getLog: () -> OSLog - private var log: OSLog { - getLog() - } } diff --git a/Sources/BrowserServicesKit/SecureVault/Logger+SecureVault.swift b/Sources/BrowserServicesKit/SecureVault/Logger+SecureVault.swift new file mode 100644 index 000000000..86156d747 --- /dev/null +++ b/Sources/BrowserServicesKit/SecureVault/Logger+SecureVault.swift @@ -0,0 +1,25 @@ +// +// File.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 Foundation +import os.log + +public extension Logger { + static var secureVault: Logger = { Logger(subsystem: Bundle.main.bundleIdentifier ?? "DuckDuckGo", category: "Secure Vault") }() +} diff --git a/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift b/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift index 95953f2ea..b852395da 100644 --- a/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift +++ b/Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift @@ -20,6 +20,7 @@ import Foundation import Combine import Common import SecureStorage +import os.log public enum AutofillType { case password @@ -170,7 +171,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { getCredentials(forDomain: domain, from: vault, or: passwordManager, withPartialMatches: includePartialAccountMatches) { [weak self] credentials, error in guard let self = self else { return } if let error = error { - os_log(.error, "Error requesting autofill init data: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting autofill init data: \(error.localizedDescription, privacy: .public)") completionHandler([], [], [], self.credentialsProvider) } else { completionHandler(credentials, identities, cards, self.credentialsProvider) @@ -181,7 +182,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { } } catch { - os_log(.error, "Error requesting autofill init data: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting autofill init data: \(error.localizedDescription, privacy: .public)") completionHandler([], [], [], credentialsProvider) } } @@ -306,7 +307,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { } } catch { - os_log(.error, "Error storing data: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error storing data: \(error.localizedDescription, privacy: .public)") } } @@ -322,14 +323,14 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { withPartialMatches: includePartialAccountMatches) { [weak self] accounts, error in guard let self = self else { return } if let error = error { - os_log(.error, "Error requesting accounts: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting accounts: \(error.localizedDescription, privacy: .public)") completionHandler([], self.credentialsProvider) } else { completionHandler(accounts, self.credentialsProvider) } } } catch { - os_log(.error, "Error requesting accounts: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting accounts: \(error.localizedDescription, privacy: .public)") completionHandler([], credentialsProvider) } @@ -351,7 +352,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { withPartialMatches: includePartialAccountMatches) { [weak self] accounts, error in guard let self = self else { return } if let error = error { - os_log(.error, "Error requesting accounts: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting accounts: \(error.localizedDescription, privacy: .public)") completionHandler(nil, self.credentialsProvider, .none) } @@ -364,7 +365,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { } if accounts.count == 0 { - os_log(.debug, "Not showing the modal, no suitable accounts found") + Logger.secureVault.debug("Not showing the modal, no suitable accounts found") completionHandler(nil, self.credentialsProvider, .none) return } @@ -387,7 +388,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { self.getCredentials(for: accountID, from: vault, or: self.passwordManager) { [weak self] credentials, error in guard let self = self else { return } if let error = error { - os_log(.error, "Error requesting credentials: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting credentials: \(error.localizedDescription, privacy: .public)") completionHandler(nil, self.credentialsProvider, .none) } else { completionHandler(credentials, self.credentialsProvider, .fill) @@ -396,7 +397,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { }) } } catch { - os_log(.error, "Error requesting accounts: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting accounts: \(error.localizedDescription, privacy: .public)") completionHandler(nil, credentialsProvider, .none) } } @@ -416,7 +417,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { self.getCredentials(for: accountId, from: vault, or: self.passwordManager) { [weak self] credentials, error in guard let self = self else { return } if let error = error { - os_log(.error, "Error requesting credentials: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting credentials: \(error.localizedDescription, privacy: .public)") completionHandler(nil, self.credentialsProvider) } else { completionHandler(credentials, self.credentialsProvider) @@ -429,7 +430,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { }) } catch { - os_log(.error, "Error requesting credentials: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting credentials: \(error.localizedDescription, privacy: .public)") completionHandler(nil, credentialsProvider) } @@ -452,7 +453,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { delegate?.secureVaultManager(self, didAutofill: .card, withObjectId: String(creditCardId)) } catch { - os_log(.error, "Error requesting credit card: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting credit card: \(error.localizedDescription, privacy: .public)") completionHandler(nil) } } @@ -466,7 +467,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { delegate?.secureVaultManager(self, didAutofill: .identity, withObjectId: String(identityId)) } catch { - os_log(.error, "Error requesting identity: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting identity: \(error.localizedDescription, privacy: .public)") completionHandler(nil) } } @@ -482,7 +483,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { passwordManager.websiteCredentialsFor(domain: domain) { [weak self] credentials, error in guard let self = self else { return } if let error = error { - os_log(.error, "Error requesting credentials: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting credentials: \(error.localizedDescription, privacy: .public)") completionHandler([], [], [], self.credentialsProvider) } else { do { @@ -491,7 +492,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { let cards = try vault.creditCards() completionHandler(credentials, identities, cards, self.credentialsProvider) } catch { - os_log(.error, "Error requesting identities or cards: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting identities or cards: \(error.localizedDescription, privacy: .public)") completionHandler([], [], [], self.credentialsProvider) } } @@ -510,7 +511,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { passwordManager.websiteCredentialsFor(domain: domain) { [weak self] credentials, error in guard let self = self else { return } if let error = error { - os_log(.error, "Error requesting credentials: %{public}@", error.localizedDescription) + Logger.secureVault.error("Error requesting credentials: \(error.localizedDescription, privacy: .public)") completionHandler([], self.credentialsProvider) } else { completionHandler(credentials, self.credentialsProvider) @@ -548,7 +549,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { guard autogeneratedCredentials, let credentials = autofillData.credentials else { - os_log("Did not meet conditions for silently saving autogenerated credentials, returning early", log: .passwordManager) + Logger.passwordManager.error("Did not meet conditions for silently saving autogenerated credentials, returning early") return } @@ -654,10 +655,10 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { private func existingIdentity(with autofillData: AutofillUserScript.DetectedAutofillData, vault: any AutofillSecureVault) throws -> SecureVaultModels.Identity? { if let identity = autofillData.identity, try vault.existingIdentityForAutofill(matching: identity) == nil { - os_log("Got new identity/address to save", log: .passwordManager) + Logger.passwordManager.debug("Got new identity/address to save") return identity } else { - os_log("No new identity/address found, avoid prompting user", log: .passwordManager) + Logger.passwordManager.debug("No new identity/address found, avoid prompting user") return nil } } @@ -711,10 +712,10 @@ extension SecureVaultManager: AutofillSecureVaultDelegate { private func existingPaymentMethod(with autofillData: AutofillUserScript.DetectedAutofillData, vault: any AutofillSecureVault) throws -> SecureVaultModels.CreditCard? { if let card = autofillData.creditCard, try vault.existingCardForAutofill(matching: card) == nil { - os_log("Got new payment method to save", log: .passwordManager) + Logger.passwordManager.debug("Got new payment method to save") return card } else { - os_log("No new payment method found, avoid prompting user", log: .passwordManager) + Logger.passwordManager.debug("No new payment method found, avoid prompting user") return nil } } diff --git a/Sources/Common/Logging.swift b/Sources/Common/Logging.swift index 9e6eb7673..43a8124de 100644 --- a/Sources/Common/Logging.swift +++ b/Sources/Common/Logging.swift @@ -24,7 +24,7 @@ public extension Logger { static var contentBlocking: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Content Blocking") }() static var userScripts: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "User Scripts") }() -// static var passwordManager: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Password Manager") }() + static var passwordManager: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Password Manager") }() // static var remoteMessaging: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Remote Messaging") }() // static var subscription: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Subscription") }() // static var history: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "History") }() @@ -38,7 +38,6 @@ public typealias OSLog = os.OSLog extension OSLog { public enum Categories: String, CaseIterable { - case passwordManager = "Password Manager" case remoteMessaging = "Remote Messaging" case history = "History" case general = "General" @@ -48,14 +47,12 @@ extension OSLog { #if DEBUG // To activate Logging Categories for DEBUG add categories here: static var debugCategories: Set = [ - .passwordManager, .remoteMessaging, .history, .general, .autofill] #endif - @OSLogWrapper(.passwordManager) public static var passwordManager @OSLogWrapper(.remoteMessaging) public static var remoteMessaging @OSLogWrapper(.history) public static var history @OSLogWrapper(.general) public static var general From 34f0099d8e715d4edb45dcb62c96bce3922396a2 Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Tue, 13 Aug 2024 14:52:24 +0100 Subject: [PATCH 05/10] remote messaging log --- Sources/Common/Logging.swift | 5 +---- .../Mappers/JsonToRemoteConfigModelMapper.swift | 7 ++++--- .../Mappers/JsonToRemoteMessageModelMapper.swift | 5 +++-- .../RemoteMessagingConfigMatcher.swift | 11 ++++++----- .../RemoteMessagingConfigProcessor.swift | 7 ++++--- .../RemoteMessaging/RemoteMessagingProcessing.swift | 9 +++++---- 6 files changed, 23 insertions(+), 21 deletions(-) diff --git a/Sources/Common/Logging.swift b/Sources/Common/Logging.swift index 43a8124de..f6237ae21 100644 --- a/Sources/Common/Logging.swift +++ b/Sources/Common/Logging.swift @@ -25,7 +25,7 @@ public extension Logger { static var contentBlocking: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Content Blocking") }() static var userScripts: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "User Scripts") }() static var passwordManager: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Password Manager") }() -// static var remoteMessaging: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Remote Messaging") }() + static var remoteMessaging: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Remote Messaging") }() // static var subscription: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Subscription") }() // static var history: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "History") }() // static var general: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "General") }() @@ -38,7 +38,6 @@ public typealias OSLog = os.OSLog extension OSLog { public enum Categories: String, CaseIterable { - case remoteMessaging = "Remote Messaging" case history = "History" case general = "General" case autofill = "Autofill" @@ -47,13 +46,11 @@ extension OSLog { #if DEBUG // To activate Logging Categories for DEBUG add categories here: static var debugCategories: Set = [ - .remoteMessaging, .history, .general, .autofill] #endif - @OSLogWrapper(.remoteMessaging) public static var remoteMessaging @OSLogWrapper(.history) public static var history @OSLogWrapper(.general) public static var general @OSLogWrapper(.autofill) public static var autofill diff --git a/Sources/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapper.swift b/Sources/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapper.swift index 981db0ee1..1fa828872 100644 --- a/Sources/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapper.swift +++ b/Sources/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapper.swift @@ -16,8 +16,9 @@ // limitations under the License. // -import Common import Foundation +import Common +import os.log struct JsonToRemoteConfigModelMapper { @@ -27,9 +28,9 @@ struct JsonToRemoteConfigModelMapper { jsonRemoteMessages: remoteMessagingConfig.messages, surveyActionMapper: surveyActionMapper ) - os_log("remoteMessages mapped = %s", log: .remoteMessaging, type: .debug, String(describing: remoteMessages)) + Logger.remoteMessaging.debug("remoteMessages mapped = \(String(describing: remoteMessages), privacy: .public)") let rules = JsonToRemoteMessageModelMapper.maps(jsonRemoteRules: remoteMessagingConfig.rules) - os_log("rules mapped = %s", log: .remoteMessaging, type: .debug, String(describing: rules)) + Logger.remoteMessaging.debug("rules mapped = \(String(describing: rules), privacy: .public)") return RemoteConfigModel(messages: remoteMessages, rules: rules) } diff --git a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift index af3134372..7d7e83bc8 100644 --- a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift +++ b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift @@ -16,8 +16,9 @@ // limitations under the License. // -import Common import Foundation +import Common +import os.log private enum AttributesKey: String, CaseIterable { case locale @@ -252,7 +253,7 @@ struct JsonToRemoteMessageModelMapper { if let key = AttributesKey(rawValue: attribute.key) { return key.matchingAttribute(jsonMatchingAttribute: attribute.value) } else { - os_log("Unknown attribute key %s", log: .remoteMessaging, type: .debug, attribute.key) + Logger.remoteMessaging.debug("Unknown attribute key \(attribute.key, privacy: .public)") return UnknownMatchingAttribute(jsonMatchingAttribute: attribute.value) } } diff --git a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift index b76b0331b..1fc896e16 100644 --- a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift +++ b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift @@ -16,8 +16,9 @@ // limitations under the License. // -import Common import Foundation +import Common +import os.log public struct RemoteMessagingConfigMatcher { @@ -78,7 +79,7 @@ public struct RemoteMessagingConfigMatcher { let userPercentile = percentileStore.percentile(forMessageId: messageID) if userPercentile > messagePercentile { - os_log("Matching rule percentile check failed for message with ID %s", log: .remoteMessaging, type: .debug, messageID) + Logger.remoteMessaging.debug("Matching rule percentile check failed for message with ID \(messageID, privacy: .public)") return .fail } } @@ -88,7 +89,7 @@ public struct RemoteMessagingConfigMatcher { for attribute in matchingRule.attributes { result = evaluateAttribute(matchingAttribute: attribute) if result == .fail || result == .nextMessage { - os_log("First failing matching attribute %s", log: .remoteMessaging, type: .debug, String(describing: attribute)) + Logger.remoteMessaging.debug("First failing matching attribute \(String(describing: attribute), privacy: .public)") break } } @@ -112,7 +113,7 @@ public struct RemoteMessagingConfigMatcher { let userPercentile = percentileStore.percentile(forMessageId: messageID) if userPercentile > messagePercentile { - os_log("Exclusion rule percentile check failed for message with ID %s", log: .remoteMessaging, type: .debug, messageID) + Logger.remoteMessaging.debug("Exclusion rule percentile check failed for message with ID \(messageID, privacy: .public)") return .fail } } @@ -122,7 +123,7 @@ public struct RemoteMessagingConfigMatcher { for attribute in matchingRule.attributes { result = evaluateAttribute(matchingAttribute: attribute) if result == .fail || result == .nextMessage { - os_log("First failing exclusion attribute %s", log: .remoteMessaging, type: .debug, String(describing: attribute)) + Logger.remoteMessaging.debug("First failing exclusion attribute \(String(describing: attribute), privacy: .public)") break } } diff --git a/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift b/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift index 68d7cfb56..ca94351ce 100644 --- a/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift +++ b/Sources/RemoteMessaging/RemoteMessagingConfigProcessor.swift @@ -16,8 +16,9 @@ // limitations under the License. // -import Common import Foundation +import Common +import os.log /** * This protocol defines API for processing RMF config file @@ -45,7 +46,7 @@ public struct RemoteMessagingConfigProcessor: RemoteMessagingConfigProcessing { public func process(jsonRemoteMessagingConfig: RemoteMessageResponse.JsonRemoteMessagingConfig, currentConfig: RemoteMessagingConfig?) -> ProcessorResult? { - os_log("Processing version %s", log: .remoteMessaging, type: .debug, String(jsonRemoteMessagingConfig.version)) + Logger.remoteMessaging.debug("Processing version \(jsonRemoteMessagingConfig.version, privacy: .public)") let currentVersion = currentConfig?.version ?? 0 let newVersion = jsonRemoteMessagingConfig.version @@ -58,7 +59,7 @@ public struct RemoteMessagingConfigProcessor: RemoteMessagingConfigProcessing { surveyActionMapper: remoteMessagingConfigMatcher.surveyActionMapper ) let message = remoteMessagingConfigMatcher.evaluate(remoteConfig: config) - os_log("Message to present next: %s", log: .remoteMessaging, type: .debug, message.debugDescription) + Logger.remoteMessaging.debug("Message to present next: \(message.debugDescription, privacy: .public)") return ProcessorResult(version: jsonRemoteMessagingConfig.version, message: message) } diff --git a/Sources/RemoteMessaging/RemoteMessagingProcessing.swift b/Sources/RemoteMessaging/RemoteMessagingProcessing.swift index eb14d1803..bfc980161 100644 --- a/Sources/RemoteMessaging/RemoteMessagingProcessing.swift +++ b/Sources/RemoteMessaging/RemoteMessagingProcessing.swift @@ -16,10 +16,11 @@ // limitations under the License. // +import Foundation import BrowserServicesKit import Common import Configuration -import Foundation +import os.log /** * This protocol defines API for providing RMF config matcher @@ -76,12 +77,12 @@ public extension RemoteMessagingProcessing { func fetchAndProcess(using store: RemoteMessagingStoring) async throws { guard remoteMessagingAvailabilityProvider.isRemoteMessagingAvailable else { - os_log("Remote messaging feature flag is disabled, skipping fetching messages", log: .remoteMessaging, type: .debug) + Logger.remoteMessaging.debug("Remote messaging feature flag is disabled, skipping fetching messages") return } do { let jsonConfig = try await configFetcher.fetchRemoteMessagingConfig() - os_log("Successfully fetched remote messages", log: .remoteMessaging, type: .debug) + Logger.remoteMessaging.debug("Successfully fetched remote messages") let remoteMessagingConfigMatcher = await configMatcherProvider.refreshConfigMatcher(using: store) @@ -93,7 +94,7 @@ public extension RemoteMessagingProcessing { } } catch { - os_log("Failed to fetch remote messages", log: .remoteMessaging, type: .error) + Logger.remoteMessaging.error("Failed to fetch remote messages \(error.localizedDescription, privacy: .public)") throw error } } From ee8e651fbe17d944408ab63e8e44fc16527ce3b1 Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Tue, 13 Aug 2024 15:02:56 +0100 Subject: [PATCH 06/10] remote messaging logs --- Sources/Configuration/ConfigurationFetching.swift | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/Sources/Configuration/ConfigurationFetching.swift b/Sources/Configuration/ConfigurationFetching.swift index 26a7fb58f..d80b3f73d 100644 --- a/Sources/Configuration/ConfigurationFetching.swift +++ b/Sources/Configuration/ConfigurationFetching.swift @@ -41,27 +41,20 @@ public final class ConfigurationFetcher: ConfigurationFetching { private var store: ConfigurationStoring private let validator: ConfigurationValidating private let urlSession: URLSession - private let getLog: () -> OSLog - private var log: OSLog { - getLog() - } public convenience init(store: ConfigurationStoring, urlSession: URLSession = .shared, - log: @escaping @autoclosure () -> OSLog = .disabled, eventMapping: EventMapping? = nil) { let validator = ConfigurationValidator(eventMapping: eventMapping) - self.init(store: store, validator: validator, log: log()) + self.init(store: store, validator: validator) } init(store: ConfigurationStoring, validator: ConfigurationValidating, - urlSession: URLSession = .shared, - log: @escaping @autoclosure () -> OSLog = .disabled) { + urlSession: URLSession = .shared) { self.store = store self.validator = validator self.urlSession = urlSession - self.getLog = log } /** @@ -132,8 +125,7 @@ public final class ConfigurationFetcher: ConfigurationFetching { let configuration = APIRequest.Configuration(url: url, headers: APIRequest.Headers(etag: etag), cachePolicy: .reloadIgnoringLocalCacheData) - let log = log - let request = APIRequest(configuration: configuration, requirements: requirements, urlSession: urlSession, log: log) + let request = APIRequest(configuration: configuration, requirements: requirements, urlSession: urlSession) let (data, response) = try await request.fetch() return (response.etag ?? "", data) } From d7c290787a012af76cb2d4a2f7248ee3c57e7f4a Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Tue, 13 Aug 2024 16:05:57 +0100 Subject: [PATCH 07/10] Logger.history --- Sources/Common/Logging.swift | 6 +--- Sources/History/HistoryCoordinator.swift | 37 ++++++++++-------------- Sources/History/HistoryStore.swift | 5 ++-- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/Sources/Common/Logging.swift b/Sources/Common/Logging.swift index f6237ae21..b8deeb7bf 100644 --- a/Sources/Common/Logging.swift +++ b/Sources/Common/Logging.swift @@ -26,8 +26,7 @@ public extension Logger { static var userScripts: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "User Scripts") }() static var passwordManager: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Password Manager") }() static var remoteMessaging: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Remote Messaging") }() -// static var subscription: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Subscription") }() -// static var history: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "History") }() + static var history: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "History") }() // static var general: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "General") }() // static var autofill: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Autofill") }() } @@ -38,7 +37,6 @@ public typealias OSLog = os.OSLog extension OSLog { public enum Categories: String, CaseIterable { - case history = "History" case general = "General" case autofill = "Autofill" } @@ -46,12 +44,10 @@ extension OSLog { #if DEBUG // To activate Logging Categories for DEBUG add categories here: static var debugCategories: Set = [ - .history, .general, .autofill] #endif - @OSLogWrapper(.history) public static var history @OSLogWrapper(.general) public static var general @OSLogWrapper(.autofill) public static var autofill diff --git a/Sources/History/HistoryCoordinator.swift b/Sources/History/HistoryCoordinator.swift index 54d2df181..191cfc558 100644 --- a/Sources/History/HistoryCoordinator.swift +++ b/Sources/History/HistoryCoordinator.swift @@ -19,6 +19,7 @@ import Foundation import Combine import Common +import os.log public typealias BrowsingHistory = [HistoryEntry] @@ -86,7 +87,7 @@ final public class HistoryCoordinator: HistoryCoordinating { @discardableResult public func addVisit(of url: URL) -> Visit? { guard let historyDictionary = historyDictionary else { - os_log("Visit of %s ignored", log: .history, url.absoluteString) + Logger.history.debug("Visit of \(url.absoluteString) ignored") return nil } @@ -102,12 +103,12 @@ final public class HistoryCoordinator: HistoryCoordinating { public func addBlockedTracker(entityName: String, on url: URL) { guard let historyDictionary = historyDictionary else { - os_log("Add tracker to %s ignored, no history", log: .history, url.absoluteString) + Logger.history.debug("Add tracker to \(url.absoluteString) ignored, no history") return } guard let entry = historyDictionary[url] else { - os_log("Add tracker to %s ignored, no entry", log: .history, url.absoluteString) + Logger.history.debug("Add tracker to \(url.absoluteString) ignored, no entry") return } @@ -116,12 +117,12 @@ final public class HistoryCoordinator: HistoryCoordinating { public func trackerFound(on url: URL) { guard let historyDictionary = historyDictionary else { - os_log("Add tracker to %s ignored, no history", log: .history, url.absoluteString) + Logger.history.debug("Add tracker to \(url.absoluteString) ignored, no history") return } guard let entry = historyDictionary[url] else { - os_log("Add tracker to %s ignored, no entry", log: .history, url.absoluteString) + Logger.history.debug("Add tracker to \(url.absoluteString) ignored, no entry") return } @@ -131,7 +132,7 @@ final public class HistoryCoordinator: HistoryCoordinating { public func updateTitleIfNeeded(title: String, url: URL) { guard let historyDictionary = historyDictionary else { return } guard let entry = historyDictionary[url] else { - os_log("Title update ignored - URL not part of history yet", log: .history, type: .debug) + Logger.history.debug("Title update ignored - URL not part of history yet") return } guard !title.isEmpty, entry.title != title else { return } @@ -205,10 +206,10 @@ final public class HistoryCoordinator: HistoryCoordinating { .sink(receiveCompletion: { completion in switch completion { case .finished: - os_log("History cleaned successfully", log: .history) + Logger.history.debug("History cleaned successfully") case .failure(let error): // This should really be a pixel - os_log("Cleaning of history failed: %s", log: .history, type: .error, error.localizedDescription) + Logger.history.error("Cleaning of history failed: \(error.localizedDescription)") } onCleanFinished?() }, receiveValue: { [weak self] history in @@ -230,11 +231,11 @@ final public class HistoryCoordinator: HistoryCoordinating { .sink(receiveCompletion: { completion in switch completion { case .finished: - os_log("Entries removed successfully", log: .history) + Logger.history.debug("Entries removed successfully") completionHandler?(nil) case .failure(let error): assertionFailure("Removal failed") - os_log("Removal failed: %s", log: .history, type: .error, error.localizedDescription) + Logger.history.error("Removal failed: \(error.localizedDescription)") completionHandler?(error) } }, receiveValue: {}) @@ -271,12 +272,12 @@ final public class HistoryCoordinator: HistoryCoordinating { .sink(receiveCompletion: { [weak self] completion in switch completion { case .finished: - os_log("Visits removed successfully", log: .history) + Logger.history.debug("Visits removed successfully") // Remove entries with no remaining visits self?.removeEntries(entriesToRemove, completionHandler: completionHandler) case .failure(let error): assertionFailure("Removal failed") - os_log("Removal failed: %s", log: .history, type: .error, error.localizedDescription) + Logger.history.error("Removal failed: \(error.localizedDescription)") completionHandler?(error) } }, receiveValue: {}) @@ -319,14 +320,9 @@ final public class HistoryCoordinator: HistoryCoordinating { .sink(receiveCompletion: { completion in switch completion { case .finished: - os_log("Visit entry updated successfully. URL: %s, Title: %s, Number of visits: %d, failed to load: %s", - log: .history, - entry.url.absoluteString, - entry.title ?? "", - entry.numberOfTotalVisits, - entry.failedToLoad ? "yes" : "no") + Logger.history.debug("Visit entry updated successfully. URL: \(entry.url.absoluteString), Title: \(entry.title ?? "-"), Number of visits: \(entry.numberOfTotalVisits), failed to load: \(entry.failedToLoad ? "yes" : "no")") case .failure(let error): - os_log("Saving of history entry failed: %s", log: .history, type: .error, error.localizedDescription) + Logger.history.error("Saving of history entry failed: \(error.localizedDescription)") } }, receiveValue: { result in for (id, date) in result { @@ -342,8 +338,7 @@ final public class HistoryCoordinator: HistoryCoordinating { /// Does the same for the root URL if it has no visits private func mark(url: URL, keyPath: WritableKeyPath, value: Bool) { guard let historyDictionary = historyDictionary, var entry = historyDictionary[url] else { - os_log("Marking of %s not saved. History not loaded yet or entry doesn't exist", - log: .history, url.absoluteString) + Logger.history.debug("Marking of \(url.absoluteString) not saved. History not loaded yet or entry doesn't exist") return } diff --git a/Sources/History/HistoryStore.swift b/Sources/History/HistoryStore.swift index 9e3e04620..ed42b1a4d 100644 --- a/Sources/History/HistoryStore.swift +++ b/Sources/History/HistoryStore.swift @@ -20,6 +20,7 @@ import Common import Foundation import CoreData import Combine +import os.log final public class HistoryStore: HistoryStoring { @@ -102,7 +103,7 @@ final public class HistoryStore: HistoryStoring { for entry in entriesToDelete { context.delete(entry) } - os_log("%d items cleaned from history", log: .history, entriesToDelete.count) + Logger.history.debug("\(entriesToDelete.count) items cleaned from history") } catch { eventMapper.fire(.removeFailed, error: error) self.context.reset() @@ -126,7 +127,7 @@ final public class HistoryStore: HistoryStoring { fetchRequest.returnsObjectsAsFaults = false do { let historyEntries = try context.fetch(fetchRequest) - os_log("%d entries loaded from history", log: .history, historyEntries.count) + Logger.history.debug("\(historyEntries.count) entries loaded from history") let history = BrowsingHistory(historyEntries: historyEntries) return .success(history) } catch { From 7842fa414f402c7ff46d4e87cf911c3af3d68082 Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Tue, 13 Aug 2024 17:36:55 +0100 Subject: [PATCH 08/10] Logger.autofill --- .../AutofillKeyStoreProvider.swift | 13 +++---- Sources/Common/Logging.swift | 36 ++----------------- Sources/Common/UserDefaultsCache.swift | 12 ++++--- 3 files changed, 14 insertions(+), 47 deletions(-) diff --git a/Sources/BrowserServicesKit/SecureVault/AutofillKeyStoreProvider.swift b/Sources/BrowserServicesKit/SecureVault/AutofillKeyStoreProvider.swift index 88c8515de..0a22e1f0a 100644 --- a/Sources/BrowserServicesKit/SecureVault/AutofillKeyStoreProvider.swift +++ b/Sources/BrowserServicesKit/SecureVault/AutofillKeyStoreProvider.swift @@ -19,6 +19,7 @@ import Common import Foundation import SecureStorage +import os.log final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { @@ -67,17 +68,11 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { } let keychainService: KeychainService - private let getLog: () -> OSLog - private var log: OSLog { - getLog() - } private var reporter: SecureVaultReporting? init(keychainService: KeychainService = DefaultKeychainService(), - log: @escaping @autoclosure () -> OSLog = .disabled, reporter: SecureVaultReporting? = nil) { self.keychainService = keychainService - self.getLog = log self.reporter = reporter } @@ -108,7 +103,7 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { /// - Returns: Optional data private func readOrMigrate(named name: String, serviceName: String) throws -> Data? { if let data = try read(named: name, serviceName: serviceName) { - os_log("Autofill Keystore data retrieved", log: .autofill, type: .debug) + Logger.autofill.debug("Autofill Keystore data retrieved") return data } else { guard let entryName = EntryName(name) else { return nil } @@ -117,11 +112,11 @@ final class AutofillKeyStoreProvider: SecureStorageKeyStoreProvider { // Look for items in V2 vault (i.e pre-bundle-specifc Keychain storage) if let data = try migrateEntry(entryName: entryName, serviceName: Constants.v2ServiceName) { - os_log("Migrated V2 Autofill Keystore data", log: .autofill, type: .debug) + Logger.autofill.debug("Migrated V2 Autofill Keystore data") return data // Look for items in V1 vault } else if let data = try migrateEntry(entryName: entryName, serviceName: Constants.v1ServiceName) { - os_log("Migrated V1 Autofill Keystore data", log: .autofill, type: .debug) + Logger.autofill.debug("Migrated V1 Autofill Keystore data") return data } diff --git a/Sources/Common/Logging.swift b/Sources/Common/Logging.swift index b8deeb7bf..db38799c5 100644 --- a/Sources/Common/Logging.swift +++ b/Sources/Common/Logging.swift @@ -27,30 +27,15 @@ public extension Logger { static var passwordManager: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Password Manager") }() static var remoteMessaging: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Remote Messaging") }() static var history: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "History") }() -// static var general: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "General") }() -// static var autofill: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Autofill") }() + static var autofill: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Autofill") }() } +@available(*, deprecated, message: "Use Logger.yourFeature instead, see https://app.asana.com/0/1202500774821704/1208001254061393/f") public typealias OSLog = os.OSLog @available(*, deprecated, message: "Use Logger.yourFeature instead, see https://app.asana.com/0/1202500774821704/1208001254061393/f") extension OSLog { - public enum Categories: String, CaseIterable { - case general = "General" - case autofill = "Autofill" - } - -#if DEBUG - // To activate Logging Categories for DEBUG add categories here: - static var debugCategories: Set = [ - .general, - .autofill] -#endif - - @OSLogWrapper(.general) public static var general - @OSLogWrapper(.autofill) public static var autofill - public static var enabledLoggingCategories = Set() static let isRunningInDebugEnvironment: Bool = { @@ -71,28 +56,13 @@ extension OSLog { } public var wrappedValue: OSLog { - var isEnabled = OSLog.enabledLoggingCategories.contains(category) -#if CI - isEnabled = true -#elseif DEBUG - isEnabled = isEnabled || Categories(rawValue: category).map(OSLog.debugCategories.contains) == true -#endif - - return isEnabled ? OSLog(subsystem: OSLog.subsystem, category: category) : .disabled + return OSLog(subsystem: OSLog.subsystem, category: category) } } } -public extension OSLog.OSLogWrapper { - - init(_ category: OSLog.Categories) { - self.init(rawValue: category.rawValue) - } - -} - extension ProcessInfo { enum Constants { static let osActivityMode = "OS_ACTIVITY_MODE" diff --git a/Sources/Common/UserDefaultsCache.swift b/Sources/Common/UserDefaultsCache.swift index ac64a69d1..4502eb224 100644 --- a/Sources/Common/UserDefaultsCache.swift +++ b/Sources/Common/UserDefaultsCache.swift @@ -17,6 +17,7 @@ // import Foundation +import os.log public struct UserDefaultsCacheSettings { public let defaultExpirationInterval: TimeInterval @@ -43,6 +44,7 @@ public class UserDefaultsCache { let object: ObjectType } + let logger: Logger = { Logger(subsystem: Bundle.main.bundleIdentifier ?? "DuckDuckGo", category: "UserDefaultsCache") }() private var userDefaults: UserDefaults public private(set) var settings: UserDefaultsCacheSettings @@ -63,7 +65,7 @@ public class UserDefaultsCache { do { let data = try encoder.encode(cacheObject) userDefaults.set(data, forKey: key.rawValue) - os_log(.debug, log: .general, "Cache Set: \(cacheObject)") + logger.debug("Cache Set: \(String(describing: cacheObject))") } catch { assertionFailure("Failed to encode CacheObject: \(error)") } @@ -75,21 +77,21 @@ public class UserDefaultsCache { do { let cacheObject = try decoder.decode(CacheObject.self, from: data) if cacheObject.expires > Date() { - os_log(.debug, log: .general, "Cache Hit: \(ObjectType.self)") + logger.debug("Cache Hit: \(ObjectType.self)") return cacheObject.object } else { - os_log(.debug, log: .general, "Cache Miss: \(ObjectType.self)") + logger.debug("Cache Miss: \(ObjectType.self)") reset() // Clear expired data return nil } } catch let error { - os_log(.error, log: .general, "Cache Decode Error: \(error)") + logger.error("Cache Decode Error: \(error)") return nil } } public func reset() { - os_log(.debug, log: .general, "Cache Clean: \(ObjectType.self)") + logger.debug("Cache Clean: \(ObjectType.self)") userDefaults.removeObject(forKey: key.rawValue) } } From 6c67e6a44aa36e48ba5b407282db572ea522b631 Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Tue, 13 Aug 2024 18:03:08 +0100 Subject: [PATCH 09/10] lint and Logger categories moved into frameworks --- Sources/BloomFilterObjC/BloomFilterObjC.mm | 1 - .../BloomFilterObjC/include/BloomFilterObjC.h | 1 - .../Autofill/AutofillUserScript.swift | 4 ++-- .../Autofill/Logger+Autofill.swift | 24 +++++++++++++++++++ .../UserScripts/contentblockerrules.js | 1 - .../ContentBlocking/UserScripts/surrogates.js | 1 - .../UserContentController.swift | 2 +- .../SecureVault/Logger+SecureVault.swift | 3 +-- Sources/Common/Logging.swift | 4 ---- Sources/History/Logger+History.swift | 24 +++++++++++++++++++ .../Logger+RemoteMessaging.swift | 24 +++++++++++++++++++ 11 files changed, 76 insertions(+), 13 deletions(-) create mode 100644 Sources/BrowserServicesKit/Autofill/Logger+Autofill.swift create mode 100644 Sources/History/Logger+History.swift create mode 100644 Sources/RemoteMessaging/Logger+RemoteMessaging.swift diff --git a/Sources/BloomFilterObjC/BloomFilterObjC.mm b/Sources/BloomFilterObjC/BloomFilterObjC.mm index c32bf6dc8..e3ce3765f 100644 --- a/Sources/BloomFilterObjC/BloomFilterObjC.mm +++ b/Sources/BloomFilterObjC/BloomFilterObjC.mm @@ -1,6 +1,5 @@ // // BloomFilterWrapper.m -// DuckDuckGo // // Copyright Β© 2018 DuckDuckGo. All rights reserved. // diff --git a/Sources/BloomFilterObjC/include/BloomFilterObjC.h b/Sources/BloomFilterObjC/include/BloomFilterObjC.h index 3be291b38..7cf9fd12e 100644 --- a/Sources/BloomFilterObjC/include/BloomFilterObjC.h +++ b/Sources/BloomFilterObjC/include/BloomFilterObjC.h @@ -1,6 +1,5 @@ // // BloomFilterWrapper.h -// DuckDuckGo // // Copyright Β© 2018 DuckDuckGo. All rights reserved. // diff --git a/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift b/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift index b7bfa2e11..aa51d430e 100644 --- a/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift +++ b/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift @@ -116,10 +116,10 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti public func messageHandlerFor(_ messageName: String) -> MessageHandler? { guard let message = MessageName(rawValue: messageName) else { - Logger.userScripts.error("Failed to parse Autofill User Script message: '\(messageName)'") + Logger.autofill.error("Failed to parse Autofill User Script message: '\(messageName)'") return nil } - Logger.userScripts.debug("AutofillUserScript: received '\(messageName)'") + Logger.autofill.debug("AutofillUserScript: received '\(messageName)'") switch message { case .emailHandlerStoreToken: return emailStoreToken diff --git a/Sources/BrowserServicesKit/Autofill/Logger+Autofill.swift b/Sources/BrowserServicesKit/Autofill/Logger+Autofill.swift new file mode 100644 index 000000000..8c2c118d2 --- /dev/null +++ b/Sources/BrowserServicesKit/Autofill/Logger+Autofill.swift @@ -0,0 +1,24 @@ +// +// Logger+Autofill.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 autofill: Logger = { Logger(subsystem: Bundle.main.bundleIdentifier ?? "DuckDuckGo", category: "Autofill") }() +} diff --git a/Sources/BrowserServicesKit/ContentBlocking/UserScripts/contentblockerrules.js b/Sources/BrowserServicesKit/ContentBlocking/UserScripts/contentblockerrules.js index 4e4b2651c..7ca771a6e 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/UserScripts/contentblockerrules.js +++ b/Sources/BrowserServicesKit/ContentBlocking/UserScripts/contentblockerrules.js @@ -1,6 +1,5 @@ // // contentblockerrules.js -// DuckDuckGo // // Copyright Β© 2020 DuckDuckGo. All rights reserved. // diff --git a/Sources/BrowserServicesKit/ContentBlocking/UserScripts/surrogates.js b/Sources/BrowserServicesKit/ContentBlocking/UserScripts/surrogates.js index 35ec6f289..9dd4fe34a 100644 --- a/Sources/BrowserServicesKit/ContentBlocking/UserScripts/surrogates.js +++ b/Sources/BrowserServicesKit/ContentBlocking/UserScripts/surrogates.js @@ -1,6 +1,5 @@ // // surrogates.js -// DuckDuckGo // // Copyright Β© 2017 DuckDuckGo. All rights reserved. // diff --git a/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift b/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift index d304212ac..a5311be8d 100644 --- a/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift +++ b/Sources/BrowserServicesKit/ContentScopeScript/UserContentController.swift @@ -83,7 +83,7 @@ final public class UserContentController: WKUserContentController { if let contentBlockingAssets = newValue { Logger.contentBlocking.debug("πŸ“š installing \(contentBlockingAssets.debugDescription)") self.installGlobalContentRuleLists(contentBlockingAssets.globalRuleLists) - Logger.userScripts.debug("πŸ“œ installing user scripts") + Logger.contentBlocking.debug("πŸ“œ installing user scripts") self.installUserScripts(contentBlockingAssets.wkUserScripts, handlers: contentBlockingAssets.userScripts.userScripts) Logger.contentBlocking.debug("βœ… installing content blocking assets done") } diff --git a/Sources/BrowserServicesKit/SecureVault/Logger+SecureVault.swift b/Sources/BrowserServicesKit/SecureVault/Logger+SecureVault.swift index 86156d747..95f1e893e 100644 --- a/Sources/BrowserServicesKit/SecureVault/Logger+SecureVault.swift +++ b/Sources/BrowserServicesKit/SecureVault/Logger+SecureVault.swift @@ -1,6 +1,5 @@ // -// File.swift -// DuckDuckGo +// Logger+SecureVault.swift // // Copyright Β© 2024 DuckDuckGo. All rights reserved. // diff --git a/Sources/Common/Logging.swift b/Sources/Common/Logging.swift index db38799c5..02aa3cbda 100644 --- a/Sources/Common/Logging.swift +++ b/Sources/Common/Logging.swift @@ -23,11 +23,7 @@ public extension Logger { private static let bundleIdentifier = Bundle.main.bundleIdentifier ?? "DuckDuckGo" static var contentBlocking: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Content Blocking") }() - static var userScripts: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "User Scripts") }() static var passwordManager: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Password Manager") }() - static var remoteMessaging: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Remote Messaging") }() - static var history: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "History") }() - static var autofill: Logger = { Logger(subsystem: Logger.bundleIdentifier, category: "Autofill") }() } @available(*, deprecated, message: "Use Logger.yourFeature instead, see https://app.asana.com/0/1202500774821704/1208001254061393/f") diff --git a/Sources/History/Logger+History.swift b/Sources/History/Logger+History.swift new file mode 100644 index 000000000..e649fac4a --- /dev/null +++ b/Sources/History/Logger+History.swift @@ -0,0 +1,24 @@ +// +// Logger+History.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 history: Logger = { Logger(subsystem: Bundle.main.bundleIdentifier ?? "DuckDuckGo", category: "History") }() +} diff --git a/Sources/RemoteMessaging/Logger+RemoteMessaging.swift b/Sources/RemoteMessaging/Logger+RemoteMessaging.swift new file mode 100644 index 000000000..0107629a3 --- /dev/null +++ b/Sources/RemoteMessaging/Logger+RemoteMessaging.swift @@ -0,0 +1,24 @@ +// +// Logger+RemoteMessaging.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 remoteMessaging: Logger = { Logger(subsystem: Bundle.main.bundleIdentifier ?? "DuckDuckGo", category: "Remote Messaging") }() +} From 293eaa96cea2d9414ae185b3576f06c8fc5e983d Mon Sep 17 00:00:00 2001 From: Federico Cappelli Date: Mon, 19 Aug 2024 10:42:36 +0100 Subject: [PATCH 10/10] log privacy fixed --- Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift b/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift index aa51d430e..0df93c9c0 100644 --- a/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift +++ b/Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift @@ -116,10 +116,10 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti public func messageHandlerFor(_ messageName: String) -> MessageHandler? { guard let message = MessageName(rawValue: messageName) else { - Logger.autofill.error("Failed to parse Autofill User Script message: '\(messageName)'") + Logger.autofill.error("Failed to parse Autofill User Script message: '\(messageName, privacy: .public)'") return nil } - Logger.autofill.debug("AutofillUserScript: received '\(messageName)'") + Logger.autofill.debug("AutofillUserScript: received '\(messageName, privacy: .public)'") switch message { case .emailHandlerStoreToken: return emailStoreToken