From 0856489831ae44f400640ee0e7cc425b0307e455 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 17 Apr 2024 12:57:34 +0200 Subject: [PATCH 1/2] Keep a weak reference to UserScriptMessageBroker (#2642) Task/Issue URL: https://app.asana.com/0/1177771139624306/1207088650619407/f Description: Fix a memory leak by holding a weak reference to UserScriptMessageBroker in IdentityTheftRestorationPagesUserScript and SubscriptionPagesUserScript. --- DuckDuckGo.xcodeproj/project.pbxproj | 2 +- .../xcshareddata/swiftpm/Package.resolved | 4 ++-- .../IdentityTheftRestorationPagesUserScript.swift | 2 +- .../Subscription/SubscriptionPagesUserScript.swift | 8 ++++++-- LocalPackages/DataBrokerProtection/Package.swift | 2 +- LocalPackages/NetworkProtectionMac/Package.swift | 2 +- LocalPackages/SubscriptionUI/Package.swift | 2 +- 7 files changed, 13 insertions(+), 9 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 2a93ad416b..6204a3116e 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -14544,7 +14544,7 @@ repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = "134.1.0-1"; + version = "134.1.0-2"; }; }; 9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 05b5e0a844..b5b5f7b1f0 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "543398a776f522998918eefd3df4150fae041838", - "version" : "134.1.0-1" + "revision" : "4b5a3bb837c54b48bd731bc32db5407c66e962bc", + "version" : "134.1.0-2" } }, { diff --git a/DuckDuckGo/Tab/UserScripts/IdentityTheftRestorationPagesUserScript.swift b/DuckDuckGo/Tab/UserScripts/IdentityTheftRestorationPagesUserScript.swift index ca8efc779e..e6102c290c 100644 --- a/DuckDuckGo/Tab/UserScripts/IdentityTheftRestorationPagesUserScript.swift +++ b/DuckDuckGo/Tab/UserScripts/IdentityTheftRestorationPagesUserScript.swift @@ -72,7 +72,7 @@ extension IdentityTheftRestorationPagesUserScript: WKScriptMessageHandler { /// Use Subscription sub-feature /// final class IdentityTheftRestorationPagesFeature: Subfeature { - var broker: UserScriptMessageBroker? + weak var broker: UserScriptMessageBroker? var featureName = "useIdentityTheftRestoration" diff --git a/DuckDuckGo/Tab/UserScripts/Subscription/SubscriptionPagesUserScript.swift b/DuckDuckGo/Tab/UserScripts/Subscription/SubscriptionPagesUserScript.swift index 76cdb94e7c..bad36edff7 100644 --- a/DuckDuckGo/Tab/UserScripts/Subscription/SubscriptionPagesUserScript.swift +++ b/DuckDuckGo/Tab/UserScripts/Subscription/SubscriptionPagesUserScript.swift @@ -78,7 +78,7 @@ extension SubscriptionPagesUserScript: WKScriptMessageHandler { /// Use Subscription sub-feature /// final class SubscriptionPagesUseSubscriptionFeature: Subfeature { - var broker: UserScriptMessageBroker? + weak var broker: UserScriptMessageBroker? var featureName = "useSubscription" var messageOriginPolicy: MessageOriginPolicy = .only(rules: [ .exact(hostname: "duckduckgo.com"), @@ -468,7 +468,11 @@ final class SubscriptionPagesUseSubscriptionFeature: Subfeature { } func pushAction(method: SubscribeActionName, webView: WKWebView, params: Encodable) { - let broker = UserScriptMessageBroker(context: SubscriptionPagesUserScript.context, requiresRunInPageContentWorld: true ) + guard let broker else { + assertionFailure("Cannot continue without broker instance") + return + } + broker.push(method: method.rawValue, params: params, for: self, into: webView) } } diff --git a/LocalPackages/DataBrokerProtection/Package.swift b/LocalPackages/DataBrokerProtection/Package.swift index 464b91ee35..e94ac595f0 100644 --- a/LocalPackages/DataBrokerProtection/Package.swift +++ b/LocalPackages/DataBrokerProtection/Package.swift @@ -29,7 +29,7 @@ let package = Package( targets: ["DataBrokerProtection"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "134.1.0-1"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "134.1.0-2"), .package(path: "../PixelKit"), .package(path: "../SwiftUIExtensions"), .package(path: "../XPCHelper"), diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift index 8ff43b2ca5..14c851463f 100644 --- a/LocalPackages/NetworkProtectionMac/Package.swift +++ b/LocalPackages/NetworkProtectionMac/Package.swift @@ -31,7 +31,7 @@ let package = Package( .library(name: "NetworkProtectionUI", targets: ["NetworkProtectionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "134.1.0-1"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "134.1.0-2"), .package(path: "../XPCHelper"), .package(path: "../SwiftUIExtensions"), .package(path: "../LoginItems"), diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift index 6715c19137..7958bf34d6 100644 --- a/LocalPackages/SubscriptionUI/Package.swift +++ b/LocalPackages/SubscriptionUI/Package.swift @@ -12,7 +12,7 @@ let package = Package( targets: ["SubscriptionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "134.1.0-1"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "134.1.0-2"), .package(path: "../SwiftUIExtensions") ], targets: [ From 3948b09e63ac9f86295601acaae3285dd7ba94f8 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Wed, 17 Apr 2024 11:40:38 -0300 Subject: [PATCH 2/2] DBP: Fix errors when editing a profile (#2639) --- .../DataBrokerProtectionDatabase.swift | 18 +++++-- .../Model/ProfileQuery.swift | 16 +++++- ...DataBrokerProtectionDatabaseProvider.swift | 9 +++- .../ProfileQueryTests.swift | 52 +++++++++++++++++++ 4 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/ProfileQueryTests.swift diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Database/DataBrokerProtectionDatabase.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Database/DataBrokerProtectionDatabase.swift index 8dbfe0b9cb..b32a2e42a1 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Database/DataBrokerProtectionDatabase.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Database/DataBrokerProtectionDatabase.swift @@ -431,11 +431,19 @@ extension DataBrokerProtectionDatabase { // The queries we need to create are the one that exist on the new ones but not in the database let profileQueriesToCreate = Set(newProfileQueries).subtracting(Set(databaseProfileQueries)) - // The queries that need update exist in both the new and the database - // We assume updated queries will be not deprecated - var profileQueriesToUpdate = Array(Set(databaseProfileQueries).intersection(Set(newProfileQueries))).map { - $0.with(deprecated: false) - } + // Updated profile queries. This is only for use for deprecated matches. + // We do not use it for updating a particular profile query. The reason is that + // updates do not exist because the UI returns a complete profile, and does not + // discriminate if a change was something new + // + // Examples: + // - If a user John Doe, Miami, FL. Changes its name to Jonathan, for us it will be a new profile query. + // and we should remove the John Doe one. + // - The same happens with addresses, if a user changes the address from Miami to Aventura. We want + // to delete all profile queries that have Miami, FL as an address, and add the new ones. + // + var profileQueriesToUpdate = [ProfileQuery]() + // The ones that we need to remove are the ones that exist in the database but not in the new ones var profileQueriesToRemove = Set(databaseProfileQueries).subtracting(Set(newProfileQueries)) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Model/ProfileQuery.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Model/ProfileQuery.swift index c2cbde5fa5..b75bda9fb4 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Model/ProfileQuery.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Model/ProfileQuery.swift @@ -80,7 +80,7 @@ extension ProfileQuery: Equatable { return lhs.firstName.lowercased() == rhs.firstName.lowercased() && lhs.lastName.lowercased() == rhs.lastName.lowercased() && - lhs.middleName?.lowercased() == rhs.middleName?.lowercased() && + lhs.middleName.normalized() == rhs.middleName.normalized() && lhs.suffix?.lowercased() == rhs.suffix?.lowercased() && lhs.city.lowercased() == rhs.city.lowercased() && lhs.state.lowercased() == rhs.state.lowercased() && @@ -94,6 +94,20 @@ extension ProfileQuery: Equatable { } } +extension Optional where Wrapped == String { + + /// Returns a comparable string optional for profile query optional fields. + /// - Returns nil if the string is blank + /// - Returns nil when the value is nil, or the lowercased String if present + func normalized() -> String? { + guard let nonNilString = self else { + return nil + } + + return nonNilString.isBlank ? nil : nonNilString.lowercased() + } +} + extension Address: Equatable { static func == (lhs: Address, rhs: Address) -> Bool { return lhs.city.lowercased() == rhs.city.lowercased() && diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseProvider.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseProvider.swift index a70ad31818..d346a51917 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseProvider.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Storage/DataBrokerProtectionDatabaseProvider.swift @@ -394,8 +394,13 @@ final class DefaultDataBrokerProtectionDatabaseProvider: GRDBSecureStorageDataba func update(_ profileQuery: ProfileQueryDB) throws -> Int64 { try db.write { db in - try profileQuery.upsert(db) - return db.lastInsertedRowID + if let id = profileQuery.id { + try profileQuery.update(db) + return id + } else { + try profileQuery.insert(db) + return db.lastInsertedRowID + } } } diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/ProfileQueryTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/ProfileQueryTests.swift new file mode 100644 index 0000000000..d52b4666b3 --- /dev/null +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/ProfileQueryTests.swift @@ -0,0 +1,52 @@ +// +// ProfileQueryTests.swift +// +// 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. +// 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 XCTest +import Foundation +@testable import DataBrokerProtection + +final class ProfileQueryTests: XCTestCase { + + func testWhenTwoProfileQueryHaveANilAndAnEmptyMiddleName_theyAreStillTheSameQuery() { + let profileQueryOne = ProfileQuery(firstName: "John", lastName: "Doe", middleName: nil, city: "Miami", state: "FL", birthYear: 1950) + let profileQueryTwo = ProfileQuery(firstName: "John", lastName: "Doe", middleName: "", city: "Miami", state: "FL", birthYear: 1950) + + XCTAssertTrue(profileQueryOne == profileQueryTwo) + } + + func testWhenTwoProfileQueryHaveANilAndABlankMiddleName_theyAreStillTheSameQuery() { + let profileQueryOne = ProfileQuery(firstName: "John", lastName: "Doe", middleName: nil, city: "Miami", state: "FL", birthYear: 1950) + let profileQueryTwo = ProfileQuery(firstName: "John", lastName: "Doe", middleName: " ", city: "Miami", state: "FL", birthYear: 1950) + + XCTAssertTrue(profileQueryOne == profileQueryTwo) + } + + func testWhenTwoProfileQueryHaveTheSameMiddleNames_theyAreTheSameQuery() { + let profileQueryOne = ProfileQuery(firstName: "John", lastName: "Doe", middleName: "M", city: "Miami", state: "FL", birthYear: 1950) + let profileQueryTwo = ProfileQuery(firstName: "John", lastName: "Doe", middleName: "M", city: "Miami", state: "FL", birthYear: 1950) + + XCTAssertTrue(profileQueryOne == profileQueryTwo) + } + + func testWhenTwoProfileQueryHaveDifferentMiddleNames_theyAreDifferentQueries() { + let profileQueryOne = ProfileQuery(firstName: "John", lastName: "Doe", middleName: "M.", city: "Miami", state: "FL", birthYear: 1950) + let profileQueryTwo = ProfileQuery(firstName: "John", lastName: "Doe", middleName: "J.", city: "Miami", state: "FL", birthYear: 1950) + + XCTAssertFalse(profileQueryOne == profileQueryTwo) + } +}