From 73f9584d5baf66b5ddd832b196cb96b4eba14830 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Mon, 29 Nov 2021 19:40:36 +0700 Subject: [PATCH 1/7] ATB -> PixelDataStore --- DuckDuckGo.xcodeproj/project.pbxproj | 12 +- DuckDuckGo/Common/Database/Database.swift | 1 + .../EncryptedValueTransformer.swift | 20 ++- .../Utilities/UserDefaultsWrapper.swift | 1 - .../Statistics/ATB/AtbAndVariantCleanup.swift | 2 +- .../Statistics/ATB/LocalStatisticsStore.swift | 161 ++++++++++++++++++ .../Statistics/ATB/StatisticsLoader.swift | 2 +- .../ATB/StatisticsUserDefaults.swift | 49 ------ .../Statistics/ATB/VariantManager.swift | 2 +- .../PixelDataModel.xcdatamodel/contents | 6 +- DuckDuckGo/Statistics/PixelDataRecord.swift | 61 +++++++ DuckDuckGo/Statistics/PixelDataStore.swift | 110 +++++++----- Unit Tests/Statistics/PixelStoreMock.swift | 19 ++- 13 files changed, 340 insertions(+), 106 deletions(-) create mode 100644 DuckDuckGo/Statistics/ATB/LocalStatisticsStore.swift delete mode 100644 DuckDuckGo/Statistics/ATB/StatisticsUserDefaults.swift create mode 100644 DuckDuckGo/Statistics/PixelDataRecord.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 41df03b57b..c5a01f138c 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -507,6 +507,7 @@ B688B4DF27420D290087BEAF /* PDFSearchTextMenuItemHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = B688B4DE27420D290087BEAF /* PDFSearchTextMenuItemHandler.swift */; }; B689ECD526C247DB006FB0C5 /* BackForwardListItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = B689ECD426C247DB006FB0C5 /* BackForwardListItem.swift */; }; B68C92C1274E3EF4002AC6B0 /* PopUpWindow.swift in Sources */ = {isa = PBXBuildFile; fileRef = B68C92C0274E3EF4002AC6B0 /* PopUpWindow.swift */; }; + B68C92C42750EF76002AC6B0 /* PixelDataRecord.swift in Sources */ = {isa = PBXBuildFile; fileRef = B68C92C32750EF76002AC6B0 /* PixelDataRecord.swift */; }; B693954A26F04BEB0015B914 /* NibLoadable.swift in Sources */ = {isa = PBXBuildFile; fileRef = B693953C26F04BE70015B914 /* NibLoadable.swift */; }; B693954B26F04BEB0015B914 /* MouseOverView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B693953D26F04BE70015B914 /* MouseOverView.swift */; }; B693954C26F04BEB0015B914 /* FocusRingView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B693953E26F04BE70015B914 /* FocusRingView.swift */; }; @@ -532,7 +533,7 @@ B69B503C2726A12500758A2B /* StatisticsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50362726A12000758A2B /* StatisticsStore.swift */; }; B69B503D2726A12500758A2B /* VariantManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50372726A12000758A2B /* VariantManager.swift */; }; B69B503E2726A12500758A2B /* AtbParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50382726A12400758A2B /* AtbParser.swift */; }; - B69B503F2726A12500758A2B /* StatisticsUserDefaults.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50392726A12500758A2B /* StatisticsUserDefaults.swift */; }; + B69B503F2726A12500758A2B /* LocalStatisticsStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50392726A12500758A2B /* LocalStatisticsStore.swift */; }; B69B50452726C5C200758A2B /* AtbParserTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50412726C5C100758A2B /* AtbParserTests.swift */; }; B69B50462726C5C200758A2B /* AtbAndVariantCleanupTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50422726C5C100758A2B /* AtbAndVariantCleanupTests.swift */; }; B69B50472726C5C200758A2B /* VariantManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B69B50432726C5C100758A2B /* VariantManagerTests.swift */; }; @@ -1147,6 +1148,7 @@ B688B4DE27420D290087BEAF /* PDFSearchTextMenuItemHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PDFSearchTextMenuItemHandler.swift; sourceTree = ""; }; B689ECD426C247DB006FB0C5 /* BackForwardListItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackForwardListItem.swift; sourceTree = ""; }; B68C92C0274E3EF4002AC6B0 /* PopUpWindow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PopUpWindow.swift; sourceTree = ""; }; + B68C92C32750EF76002AC6B0 /* PixelDataRecord.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PixelDataRecord.swift; sourceTree = ""; }; B693953C26F04BE70015B914 /* NibLoadable.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NibLoadable.swift; sourceTree = ""; }; B693953D26F04BE70015B914 /* MouseOverView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MouseOverView.swift; sourceTree = ""; }; B693953E26F04BE70015B914 /* FocusRingView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FocusRingView.swift; sourceTree = ""; }; @@ -1174,7 +1176,7 @@ B69B50362726A12000758A2B /* StatisticsStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StatisticsStore.swift; sourceTree = ""; }; B69B50372726A12000758A2B /* VariantManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VariantManager.swift; sourceTree = ""; }; B69B50382726A12400758A2B /* AtbParser.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AtbParser.swift; sourceTree = ""; }; - B69B50392726A12500758A2B /* StatisticsUserDefaults.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StatisticsUserDefaults.swift; sourceTree = ""; }; + B69B50392726A12500758A2B /* LocalStatisticsStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LocalStatisticsStore.swift; sourceTree = ""; }; B69B50412726C5C100758A2B /* AtbParserTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AtbParserTests.swift; sourceTree = ""; }; B69B50422726C5C100758A2B /* AtbAndVariantCleanupTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AtbAndVariantCleanupTests.swift; sourceTree = ""; }; B69B50432726C5C100758A2B /* VariantManagerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VariantManagerTests.swift; sourceTree = ""; }; @@ -3131,7 +3133,7 @@ B69B50382726A12400758A2B /* AtbParser.swift */, B69B50342726A11F00758A2B /* StatisticsLoader.swift */, B69B50362726A12000758A2B /* StatisticsStore.swift */, - B69B50392726A12500758A2B /* StatisticsUserDefaults.swift */, + B69B50392726A12500758A2B /* LocalStatisticsStore.swift */, B69B50372726A12000758A2B /* VariantManager.swift */, B69B50562727D16900758A2B /* AtbAndVariantCleanup.swift */, ); @@ -3184,6 +3186,7 @@ B6A9E48826146ABF0067D1B9 /* PixelCounter.swift */, B6A9E4A2261475C70067D1B9 /* AppUsageActivityMonitor.swift */, B6DA44012616B28300DD1EC2 /* PixelDataStore.swift */, + B68C92C32750EF76002AC6B0 /* PixelDataRecord.swift */, B6DA44062616B30600DD1EC2 /* PixelDataModel.xcdatamodeld */, ); path = Statistics; @@ -3917,7 +3920,7 @@ 4B9292D12667123700AD2C21 /* BookmarkTableRowView.swift in Sources */, 4B6160E525B152FA007DE5B2 /* ContentBlockerUserScript.swift in Sources */, B66E9DD42670EB4A00E53BB5 /* WKDownload+WebKitDownload.swift in Sources */, - B69B503F2726A12500758A2B /* StatisticsUserDefaults.swift in Sources */, + B69B503F2726A12500758A2B /* LocalStatisticsStore.swift in Sources */, B689ECD526C247DB006FB0C5 /* BackForwardListItem.swift in Sources */, B69B50572727D16900758A2B /* AtbAndVariantCleanup.swift in Sources */, B693954A26F04BEB0015B914 /* NibLoadable.swift in Sources */, @@ -3948,6 +3951,7 @@ AAE71E3725F7869300D74437 /* HomepageCollectionViewItem.swift in Sources */, AAC5E4F625D6BF2C007F5990 /* AddressBarButtonsViewController.swift in Sources */, 4B59023D26B35F3600489384 /* ChromeDataImporter.swift in Sources */, + B68C92C42750EF76002AC6B0 /* PixelDataRecord.swift in Sources */, 853014D625E671A000FB8205 /* PageObserverUserScript.swift in Sources */, 4B139AFD26B60BD800894F82 /* NSImageExtensions.swift in Sources */, B6A9E45B261460350067D1B9 /* APIHeaders.swift in Sources */, diff --git a/DuckDuckGo/Common/Database/Database.swift b/DuckDuckGo/Common/Database/Database.swift index f9e3a6c6e3..dc767bb1b3 100644 --- a/DuckDuckGo/Common/Database/Database.swift +++ b/DuckDuckGo/Common/Database/Database.swift @@ -49,6 +49,7 @@ final class Database { try EncryptedValueTransformer.registerTransformer() try EncryptedValueTransformer.registerTransformer() try EncryptedValueTransformer.registerTransformer() + try EncryptedValueTransformer.registerTransformer() } catch { fatalError("Failed to register encryption value transformers") } diff --git a/DuckDuckGo/Common/FileSystem/EncryptedValueTransformer.swift b/DuckDuckGo/Common/FileSystem/EncryptedValueTransformer.swift index 999f2bd492..1e9f18825a 100644 --- a/DuckDuckGo/Common/FileSystem/EncryptedValueTransformer.swift +++ b/DuckDuckGo/Common/FileSystem/EncryptedValueTransformer.swift @@ -41,11 +41,16 @@ final class EncryptedValueTransformer: ValueTransf return nil } let archivedData: Data - do { - archivedData = try NSKeyedArchiver.archivedData(withRootObject: castValue, requiringSecureCoding: true) - } catch { - assertionFailure("Could not archive value \(castValue): \(error)") - return nil + // if T is Data + if let data = castValue as? Data { + archivedData = data + } else { + do { + archivedData = try NSKeyedArchiver.archivedData(withRootObject: castValue, requiringSecureCoding: true) + } catch { + assertionFailure("Could not archive value \(castValue): \(error)") + return nil + } } return try? DataEncryption.encrypt(data: archivedData, key: encryptionKey) @@ -55,6 +60,11 @@ final class EncryptedValueTransformer: ValueTransf guard let data = value as? Data, let decryptedData = try? DataEncryption.decrypt(data: data, key: encryptionKey) else { return nil } + // if T is Data + if let data = decryptedData as? T { + return data + } + return try? NSKeyedUnarchiver.unarchivedObject(ofClass: T.self, from: decryptedData as Data) } diff --git a/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift b/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift index 855441b131..3af4081f0d 100644 --- a/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift +++ b/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift @@ -55,7 +55,6 @@ public struct UserDefaultsWrapper { case atb = "statistics.atb.key" case searchRetentionAtb = "statistics.retentionatb.key" case appRetentionAtb = "statistics.appretentionatb.key" - case variant = "statistics.variant.key" case lastAppRetentionRequestDate = "statistics.appretentionatb.last.request.key" } diff --git a/DuckDuckGo/Statistics/ATB/AtbAndVariantCleanup.swift b/DuckDuckGo/Statistics/ATB/AtbAndVariantCleanup.swift index f8ecb52e3a..2ff120c9ee 100644 --- a/DuckDuckGo/Statistics/ATB/AtbAndVariantCleanup.swift +++ b/DuckDuckGo/Statistics/ATB/AtbAndVariantCleanup.swift @@ -21,7 +21,7 @@ import Foundation public class AtbAndVariantCleanup { - static func cleanup(statisticsStorage: StatisticsStore = StatisticsUserDefaults(), + static func cleanup(statisticsStorage: StatisticsStore = LocalStatisticsStore(), variantManager: VariantManager = DefaultVariantManager()) { guard let variant = statisticsStorage.variant else { return } diff --git a/DuckDuckGo/Statistics/ATB/LocalStatisticsStore.swift b/DuckDuckGo/Statistics/ATB/LocalStatisticsStore.swift new file mode 100644 index 0000000000..b55d3b8c53 --- /dev/null +++ b/DuckDuckGo/Statistics/ATB/LocalStatisticsStore.swift @@ -0,0 +1,161 @@ +// +// LocalStatisticsStore.swift +// DuckDuckGo +// +// Copyright © 2017 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 + +final class LocalStatisticsStore: StatisticsStore { + + private struct LegacyStatisticsStore { + @UserDefaultsWrapper(key: .atb, defaultValue: nil) + var atb: String? + + @UserDefaultsWrapper(key: .installDate, defaultValue: nil) + var installDate: Date? + + @UserDefaultsWrapper(key: .searchRetentionAtb, defaultValue: nil) + var searchRetentionAtb: String? + + @UserDefaultsWrapper(key: .appRetentionAtb, defaultValue: nil) + var appRetentionAtb: String? + + @UserDefaultsWrapper(key: .lastAppRetentionRequestDate, defaultValue: nil) + var lastAppRetentionRequestDate: Date? + + mutating func clear() { + atb = nil + installDate = nil + searchRetentionAtb = nil + appRetentionAtb = nil + lastAppRetentionRequestDate = nil + } + } + + private struct Keys { + static let installDate = "statistics.installdate.key" + static let atb = "statistics.atb.key" + static let searchRetentionAtb = "statistics.retentionatb.key" + static let appRetentionAtb = "statistics.appretentionatb.key" + static let variant = "statistics.variant.key" + static let lastAppRetentionRequestDate = "statistics.appretentionatb.last.request.key" + } + + private let pixelDataStore: PixelDataStore + + init(pixelDataStore: PixelDataStore = LocalPixelDataStore.shared) { + self.pixelDataStore = pixelDataStore + + var legacyStatisticsStore = LegacyStatisticsStore() + if let atb = legacyStatisticsStore.atb { + self.atb = atb + self.installDate = legacyStatisticsStore.installDate + self.searchRetentionAtb = legacyStatisticsStore.searchRetentionAtb + self.appRetentionAtb = legacyStatisticsStore.appRetentionAtb + self.lastAppRetentionRequestDate = legacyStatisticsStore.lastAppRetentionRequestDate + + legacyStatisticsStore.clear() + } + } + + var hasInstallStatistics: Bool { + return atb != nil + } + + var atb: String? { + get { + pixelDataStore.value(forKey: Keys.atb) + } + set { + if let value = newValue { + pixelDataStore.set(value, forKey: Keys.atb) + } else { + assertionFailure("Unexpected ATB removal") + pixelDataStore.removeValue(forKey: Keys.atb) + } + } + } + + var installDate: Date? { + get { + guard let timeInterval: Double = pixelDataStore.value(forKey: Keys.installDate) else { return nil } + return Date(timeIntervalSinceReferenceDate: timeInterval) + } + set { + if let value = newValue { + pixelDataStore.set(value.timeIntervalSinceReferenceDate, forKey: Keys.installDate) + } else { + assertionFailure("Unexpected ATB installDate removal") + pixelDataStore.removeValue(forKey: Keys.installDate) + } + } + } + + var searchRetentionAtb: String? { + get { + pixelDataStore.value(forKey: Keys.searchRetentionAtb) + } + set { + if let value = newValue { + pixelDataStore.set(value, forKey: Keys.searchRetentionAtb) + } else { + pixelDataStore.removeValue(forKey: Keys.searchRetentionAtb) + } + } + } + + var appRetentionAtb: String? { + get { + pixelDataStore.value(forKey: Keys.appRetentionAtb) + } + set { + if let value = newValue { + pixelDataStore.set(value, forKey: Keys.appRetentionAtb) + } else { + pixelDataStore.removeValue(forKey: Keys.appRetentionAtb) + } + } + } + + var variant: String? { + get { + pixelDataStore.value(forKey: Keys.variant) + } + set { + if let value = newValue { + pixelDataStore.set(value, forKey: Keys.variant) + } else { + pixelDataStore.removeValue(forKey: Keys.variant) + } + } + } + + var lastAppRetentionRequestDate: Date? { + get { + guard let timeInterval: Double = pixelDataStore.value(forKey: Keys.lastAppRetentionRequestDate) else { return nil } + return Date(timeIntervalSinceReferenceDate: timeInterval) + } + set { + if let value = newValue { + pixelDataStore.set(value.timeIntervalSinceReferenceDate, forKey: Keys.lastAppRetentionRequestDate) + } else { + pixelDataStore.removeValue(forKey: Keys.lastAppRetentionRequestDate) + } + } + } + +} diff --git a/DuckDuckGo/Statistics/ATB/StatisticsLoader.swift b/DuckDuckGo/Statistics/ATB/StatisticsLoader.swift index 41e6333e4e..2b67210e78 100644 --- a/DuckDuckGo/Statistics/ATB/StatisticsLoader.swift +++ b/DuckDuckGo/Statistics/ATB/StatisticsLoader.swift @@ -30,7 +30,7 @@ final class StatisticsLoader { private let parser = AtbParser() private var isAppRetentionRequestInProgress = false - init(statisticsStore: StatisticsStore = StatisticsUserDefaults()) { + init(statisticsStore: StatisticsStore = LocalStatisticsStore()) { self.statisticsStore = statisticsStore } diff --git a/DuckDuckGo/Statistics/ATB/StatisticsUserDefaults.swift b/DuckDuckGo/Statistics/ATB/StatisticsUserDefaults.swift deleted file mode 100644 index 064e6be4c2..0000000000 --- a/DuckDuckGo/Statistics/ATB/StatisticsUserDefaults.swift +++ /dev/null @@ -1,49 +0,0 @@ -// -// StatisticsUserDefaults.swift -// DuckDuckGo -// -// Copyright © 2017 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 - -final class StatisticsUserDefaults: StatisticsStore { - - init() { - } - - var hasInstallStatistics: Bool { - return atb != nil - } - - @UserDefaultsWrapper(key: .atb, defaultValue: nil) - var atb: String? - - @UserDefaultsWrapper(key: .installDate, defaultValue: nil) - var installDate: Date? - - @UserDefaultsWrapper(key: .searchRetentionAtb, defaultValue: nil) - var searchRetentionAtb: String? - - @UserDefaultsWrapper(key: .appRetentionAtb, defaultValue: nil) - var appRetentionAtb: String? - - @UserDefaultsWrapper(key: .variant, defaultValue: nil) - var variant: String? - - @UserDefaultsWrapper(key: .lastAppRetentionRequestDate, defaultValue: nil) - var lastAppRetentionRequestDate: Date? - -} diff --git a/DuckDuckGo/Statistics/ATB/VariantManager.swift b/DuckDuckGo/Statistics/ATB/VariantManager.swift index 32502880c7..a750afe84e 100644 --- a/DuckDuckGo/Statistics/ATB/VariantManager.swift +++ b/DuckDuckGo/Statistics/ATB/VariantManager.swift @@ -78,7 +78,7 @@ final class DefaultVariantManager: VariantManager { private let rng: VariantRNG init(variants: [Variant] = Variant.defaultVariants, - storage: StatisticsStore = StatisticsUserDefaults(), + storage: StatisticsStore = LocalStatisticsStore(), rng: VariantRNG = Arc4RandomUniformVariantRNG()) { self.variants = variants self.storage = storage diff --git a/DuckDuckGo/Statistics/PixelDataModel.xcdatamodeld/PixelDataModel.xcdatamodel/contents b/DuckDuckGo/Statistics/PixelDataModel.xcdatamodeld/PixelDataModel.xcdatamodel/contents index 3b763aa34f..1be3742398 100644 --- a/DuckDuckGo/Statistics/PixelDataModel.xcdatamodeld/PixelDataModel.xcdatamodel/contents +++ b/DuckDuckGo/Statistics/PixelDataModel.xcdatamodeld/PixelDataModel.xcdatamodel/contents @@ -1,13 +1,13 @@ - + - + - + \ No newline at end of file diff --git a/DuckDuckGo/Statistics/PixelDataRecord.swift b/DuckDuckGo/Statistics/PixelDataRecord.swift new file mode 100644 index 0000000000..c125b4c81b --- /dev/null +++ b/DuckDuckGo/Statistics/PixelDataRecord.swift @@ -0,0 +1,61 @@ +// +// PixelDataRecord.swift +// +// Copyright © 2021 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 CoreData + +struct PixelDataRecord { + + let key: String + let value: NSObject + +} + +extension PixelData { + + func valueRepresentation() -> PixelDataRecord? { + guard let key = self.key else { + assertionFailure("PixelData: Key should not be nil") + return nil + } + guard let data = self.valueEncrypted as? Data else { + assertionFailure("PixelData: valueEncrypted is not Data") + return nil + } + if let string = try? NSKeyedUnarchiver.unarchivedObject(ofClass: NSString.self, from: data) { + return PixelDataRecord(key: key, value: string as NSString) + } else if let number = try? NSKeyedUnarchiver.unarchivedObject(ofClass: NSNumber.self, from: data) { + return PixelDataRecord(key: key, value: number) + } else { + return nil + } + } + + func update(with record: PixelDataRecord) throws { + if self.key != record.key { + guard self.key == nil else { + assertionFailure("PixelData: Keys does not match") + return + } + self.key = record.key + } + + self.valueEncrypted = record.value + } + +} diff --git a/DuckDuckGo/Statistics/PixelDataStore.swift b/DuckDuckGo/Statistics/PixelDataStore.swift index 8d2f8be429..b9a05c80b5 100644 --- a/DuckDuckGo/Statistics/PixelDataStore.swift +++ b/DuckDuckGo/Statistics/PixelDataStore.swift @@ -27,12 +27,17 @@ protocol PixelDataStore { func value(forKey key: String) -> Int? func set(_ value: Int, forKey: String) + func value(forKey key: String) -> String? + func set(_ value: String, forKey: String) + + func removeValue(forKey key: String) + } final class LocalPixelDataStore: PixelDataStore { static let shared = LocalPixelDataStore() - private lazy var cache: [String: NSNumber] = loadAll() + private lazy var cache: [String: NSObject] = loadAll() private init() {} @@ -41,71 +46,100 @@ final class LocalPixelDataStore: PixelDataStore { } private lazy var context = Database.shared.makeContext(concurrencyType: .mainQueueConcurrencyType, name: "PixelData") - - private func loadAll() -> [String: NSNumber] { + private func loadAll() -> [String: NSObject] { let fetchRequest = PixelData.fetchRequest() as NSFetchRequest - var dict = [String: NSNumber]() + var dict = [String: NSObject]() do { let result = try context.fetch(fetchRequest) for item in result { - guard let key = item.key else { - assertionFailure("LocalPixelDataStore: Key should not be nil") - continue - } - guard let value = item.valueEncrypted as? NSNumber else { - assertionFailure("LocalPixelDataStore: Could not decrypt value") + guard let record = item.valueRepresentation() else { + assertionFailure("LocalPixelDataStore: Key should not load PixelDataRecord") continue } - dict[key] = value + + dict[record.key] = record.value } } catch { - assertionFailure("LocalPixelDataStore: loadAll failed \(error)") } return dict } - func value(forKey key: String) -> Int? { - return cache[key]?.intValue - } - - func value(forKey key: String) -> Double? { - return cache[key]?.doubleValue + private func predicate(forKey key: String) -> NSPredicate { + return NSPredicate(format: "key = %@", key) } - private func set(_ value: NSNumber, forKey key: String) { - cache[key] = value + private func update(record: PixelDataRecord) { + cache[record.key] = record.value + let predicate = self.predicate(forKey: record.key) context.perform { [context] in do { - let fetchRequest = PixelData.fetchRequest() as NSFetchRequest - fetchRequest.predicate = NSPredicate(format: "key = %@", key) - if let pixelData = try context.fetch(fetchRequest).first { - pixelData.valueEncrypted = value - } else { - let mobj = NSEntityDescription.insertNewObject(forEntityName: PixelData.className(), - into: self.context) - guard let pixelData = mobj as? PixelData else { - assertionFailure("LocalPixelDataStore: Failed to init PixelData") - return + let fetchRequest = NSFetchRequest(entityName: PixelData.className()) + fetchRequest.predicate = predicate + + let fetchResults = try context.fetch(fetchRequest) + let managedObject: PixelData = try { + if let managedObject = fetchResults.first as? PixelData { + return managedObject + } else if let managedObject = NSEntityDescription.insertNewObject(forEntityName: PixelData.className(), + into: context) as? PixelData { + return managedObject } + struct ObjectNotFoundError: Error {} + throw ObjectNotFoundError() + }() - pixelData.key = key - pixelData.valueEncrypted = value - } + try managedObject.update(with: record) + try context.save() - try self.context.save() } catch { - assertionFailure("LocalPixelDataStore: Saving of context failed") + assertionFailure("LocalPixelDataStore: Could not update record with \(error)") } } } - func set(_ value: Int, forKey key: String) { - self.set(NSNumber(value: value), forKey: key) + func value(forKey key: String) -> Double? { + return (cache[key] as? NSNumber)?.doubleValue + } + + func value(forKey key: String) -> Int? { + return (cache[key] as? NSNumber)?.intValue + } + + func value(forKey key: String) -> String? { + return cache[key] as? String } func set(_ value: Double, forKey key: String) { - self.set(NSNumber(value: value), forKey: key) + update(record: PixelDataRecord(key: key, value: NSNumber(value: value))) + } + + func set(_ value: Int, forKey key: String) { + update(record: PixelDataRecord(key: key, value: NSNumber(value: value))) + } + + func set(_ value: String, forKey key: String) { + update(record: PixelDataRecord(key: key, value: value as NSString)) + } + + func removeValue(forKey key: String) { + let predicate = self.predicate(forKey: key) + + context.perform { [context] in + let deleteRequest = NSFetchRequest(entityName: PixelData.className()) + deleteRequest.predicate = predicate + let batchDeleteRequest = NSBatchDeleteRequest(fetchRequest: deleteRequest) + batchDeleteRequest.resultType = .resultTypeObjectIDs + + do { + let result = try context.execute(batchDeleteRequest) as? NSBatchDeleteResult + let deletedObjects = result?.result as? [NSManagedObjectID] ?? [] + let changes: [AnyHashable: Any] = [NSDeletedObjectsKey: deletedObjects] + NSManagedObjectContext.mergeChanges(fromRemoteContextSave: changes, into: [context]) + } catch { + assertionFailure("LocalPixelDataStore: Could not remove record with \(error)") + } + } } } diff --git a/Unit Tests/Statistics/PixelStoreMock.swift b/Unit Tests/Statistics/PixelStoreMock.swift index b4ca55ee30..910168761f 100644 --- a/Unit Tests/Statistics/PixelStoreMock.swift +++ b/Unit Tests/Statistics/PixelStoreMock.swift @@ -20,10 +20,11 @@ import Foundation @testable import DuckDuckGo_Privacy_Browser final class PixelStoreMock: PixelDataStore { - var data = [String: NSNumber]() + + var data = [String: Any]() func value(forKey key: String) -> Int? { - return data[key]?.intValue + return (data[key] as? NSNumber)?.intValue } func set(_ value: Int, forKey key: String) { @@ -31,11 +32,23 @@ final class PixelStoreMock: PixelDataStore { } func value(forKey key: String) -> Double? { - return data[key]?.doubleValue + return (data[key] as? NSNumber)?.doubleValue } func set(_ value: Double, forKey key: String) { data[key] = NSNumber(value: value) } + func value(forKey key: String) -> String? { + return data[key] as? String + } + + func set(_ value: String, forKey key: String) { + data[key] = value + } + + func removeValue(forKey key: String) { + data[key] = nil + } + } From 7d8368116513eff48777192b1cd6444f8b096e0e Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 30 Nov 2021 12:07:35 +0700 Subject: [PATCH 2/7] Fix PixelData updating --- DuckDuckGo/Statistics/PixelDataRecord.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DuckDuckGo/Statistics/PixelDataRecord.swift b/DuckDuckGo/Statistics/PixelDataRecord.swift index c125b4c81b..b74b5ba354 100644 --- a/DuckDuckGo/Statistics/PixelDataRecord.swift +++ b/DuckDuckGo/Statistics/PixelDataRecord.swift @@ -55,7 +55,8 @@ extension PixelData { self.key = record.key } - self.valueEncrypted = record.value + let data = try NSKeyedArchiver.archivedData(withRootObject: record.value, requiringSecureCoding: true) + self.valueEncrypted = data as NSData } } From c819691f6fa945913de37af4ba0c2791c5b6c66f Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 30 Nov 2021 12:08:12 +0700 Subject: [PATCH 3/7] Added PixelStore migration test --- DuckDuckGo.xcodeproj/project.pbxproj | 18 +++ DuckDuckGo/Statistics/PixelDataStore.swift | 113 +++++++++++++----- .../NSPersistentContainerExtension.swift | 19 +-- .../PixelDataModel.xcdatamodel/contents | 13 ++ Unit Tests/Statistics/PixelStoreMock.swift | 12 +- Unit Tests/Statistics/PixelStoreTests.swift | 88 ++++++++++++++ 6 files changed, 220 insertions(+), 43 deletions(-) create mode 100644 Unit Tests/Statistics/OldPixelDataModel.xcdatamodeld/PixelDataModel.xcdatamodel/contents create mode 100644 Unit Tests/Statistics/PixelStoreTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index c5a01f138c..e97d3cecb0 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -486,6 +486,8 @@ B657841F25FA497600D8DB33 /* NSException+Catch.swift in Sources */ = {isa = PBXBuildFile; fileRef = B657841E25FA497600D8DB33 /* NSException+Catch.swift */; }; B65E6B9E26D9EC0800095F96 /* CircularProgressView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B65E6B9D26D9EC0800095F96 /* CircularProgressView.swift */; }; B65E6BA026D9F10600095F96 /* NSBezierPathExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = B65E6B9F26D9F10600095F96 /* NSBezierPathExtension.swift */; }; + B662D3D92755D7AD0035D4D6 /* PixelStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B662D3D82755D7AD0035D4D6 /* PixelStoreTests.swift */; }; + B662D3DC2755DF670035D4D6 /* OldPixelDataModel.xcdatamodeld in Sources */ = {isa = PBXBuildFile; fileRef = B662D3DA2755D8190035D4D6 /* OldPixelDataModel.xcdatamodeld */; }; B66E9DD22670EB2A00E53BB5 /* _WKDownload+WebKitDownload.swift in Sources */ = {isa = PBXBuildFile; fileRef = B66E9DD12670EB2A00E53BB5 /* _WKDownload+WebKitDownload.swift */; }; B66E9DD42670EB4A00E53BB5 /* WKDownload+WebKitDownload.swift in Sources */ = {isa = PBXBuildFile; fileRef = B66E9DD32670EB4A00E53BB5 /* WKDownload+WebKitDownload.swift */; }; B67C6C3D2654B897006C872E /* WebViewExtensionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B67C6C3C2654B897006C872E /* WebViewExtensionTests.swift */; }; @@ -1127,6 +1129,8 @@ B657841E25FA497600D8DB33 /* NSException+Catch.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NSException+Catch.swift"; sourceTree = ""; }; B65E6B9D26D9EC0800095F96 /* CircularProgressView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CircularProgressView.swift; sourceTree = ""; }; B65E6B9F26D9F10600095F96 /* NSBezierPathExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSBezierPathExtension.swift; sourceTree = ""; }; + B662D3D82755D7AD0035D4D6 /* PixelStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PixelStoreTests.swift; sourceTree = ""; }; + B662D3DB2755D81A0035D4D6 /* PixelDataModel.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = PixelDataModel.xcdatamodel; sourceTree = ""; }; B66E9DD12670EB2A00E53BB5 /* _WKDownload+WebKitDownload.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "_WKDownload+WebKitDownload.swift"; sourceTree = ""; }; B66E9DD32670EB4A00E53BB5 /* WKDownload+WebKitDownload.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WKDownload+WebKitDownload.swift"; sourceTree = ""; }; B67C6C3C2654B897006C872E /* WebViewExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebViewExtensionTests.swift; sourceTree = ""; }; @@ -3237,6 +3241,8 @@ children = ( B69B50402726C3F400758A2B /* ATB */, B6DA44102616C0FC00DD1EC2 /* PixelTests.swift */, + B662D3D82755D7AD0035D4D6 /* PixelStoreTests.swift */, + B662D3DA2755D8190035D4D6 /* OldPixelDataModel.xcdatamodeld */, B6DA44222616CABC00DD1EC2 /* PixelArgumentsTests.swift */, B6DA44272616CAE000DD1EC2 /* AppUsageActivityMonitorTests.swift */, B6DA441D2616C84600DD1EC2 /* PixelStoreMock.swift */, @@ -4002,6 +4008,7 @@ 142879DA24CE1179005419BB /* SuggestionViewModelTests.swift in Sources */, 4B9292C12667103100AD2C21 /* BookmarkMigrationTests.swift in Sources */, 4B9292BC2667103100AD2C21 /* BookmarkSidebarTreeControllerTests.swift in Sources */, + B662D3DC2755DF670035D4D6 /* OldPixelDataModel.xcdatamodeld in Sources */, B6DA44232616CABC00DD1EC2 /* PixelArgumentsTests.swift in Sources */, AAEC74BC2642F0F800C2EFBC /* History.xcdatamodeld in Sources */, 85F1B0C925EF9759004792B6 /* URLEventHandlerTests.swift in Sources */, @@ -4043,6 +4050,7 @@ AA652CDB25DDAB32009059CC /* BookmarkStoreMock.swift in Sources */, B62EB47C25BAD3BB005745C6 /* WKWebViewPrivateMethodsAvailabilityTests.swift in Sources */, B6106BA026A7BE0B0013B453 /* PermissionManagerTests.swift in Sources */, + B662D3D92755D7AD0035D4D6 /* PixelStoreTests.swift in Sources */, B6106BB526A809E60013B453 /* GeolocationProviderTests.swift in Sources */, B6A5A2A025B96E8300AA7ADA /* AppStateChangePublisherTests.swift in Sources */, B63ED0E326B3E7FA00A9DAD1 /* CLLocationManagerMock.swift in Sources */, @@ -5165,6 +5173,16 @@ sourceTree = ""; versionGroupType = wrapper.xcdatamodel; }; + B662D3DA2755D8190035D4D6 /* OldPixelDataModel.xcdatamodeld */ = { + isa = XCVersionGroup; + children = ( + B662D3DB2755D81A0035D4D6 /* PixelDataModel.xcdatamodel */, + ); + currentVersion = B662D3DB2755D81A0035D4D6 /* PixelDataModel.xcdatamodel */; + path = OldPixelDataModel.xcdatamodeld; + sourceTree = ""; + versionGroupType = wrapper.xcdatamodel; + }; B6C0B23226E71BCD0031CB7F /* Downloads.xcdatamodeld */ = { isa = XCVersionGroup; children = ( diff --git a/DuckDuckGo/Statistics/PixelDataStore.swift b/DuckDuckGo/Statistics/PixelDataStore.swift index b9a05c80b5..112e11362a 100644 --- a/DuckDuckGo/Statistics/PixelDataStore.swift +++ b/DuckDuckGo/Statistics/PixelDataStore.swift @@ -22,30 +22,63 @@ import CoreData protocol PixelDataStore { func value(forKey key: String) -> Double? - func set(_ value: Double, forKey: String) + func set(_ value: Double, forKey: String, completionHandler: ((Error?) -> Void)?) func value(forKey key: String) -> Int? - func set(_ value: Int, forKey: String) + func set(_ value: Int, forKey: String, completionHandler: ((Error?) -> Void)?) func value(forKey key: String) -> String? - func set(_ value: String, forKey: String) + func set(_ value: String, forKey: String, completionHandler: ((Error?) -> Void)?) - func removeValue(forKey key: String) + func removeValue(forKey key: String, completionHandler: ((Error?) -> Void)?) } +extension PixelDataStore { + func set(_ value: Double, forKey key: String) { + set(value, forKey: key, completionHandler: nil) + } -final class LocalPixelDataStore: PixelDataStore { - static let shared = LocalPixelDataStore() + func set(_ value: Int, forKey key: String) { + set(value, forKey: key, completionHandler: nil) + } + + func set(_ value: String, forKey key: String) { + set(value, forKey: key, completionHandler: nil) + } - private lazy var cache: [String: NSObject] = loadAll() + func removeValue(forKey key: String) { + removeValue(forKey: key, completionHandler: nil) + } +} - private init() {} +extension PixelData { + fileprivate static let sharedPixelDataStore = LocalPixelDataStore() +} +private extension LocalPixelDataStore where T == PixelData { + convenience init() { + self.init(context: Database.shared.makeContext(concurrencyType: .mainQueueConcurrencyType, name: "PixelData"), + updateModel: PixelData.update, + entityName: PixelData.className()) + } +} +enum PixelDataStoreError: Error { + case objectNotFound +} - init(context: NSManagedObjectContext) { +final class LocalPixelDataStore: PixelDataStore { + static var shared: LocalPixelDataStore { PixelData.sharedPixelDataStore } + + private let context: NSManagedObjectContext + private let entityName: String + private(set) lazy var cache: [String: NSObject] = loadAll() + private let updateModel: (T) -> (PixelDataRecord) throws -> Void + + init(context: NSManagedObjectContext, updateModel: @escaping (T) -> (PixelDataRecord) throws -> Void, entityName: String = T.className()) { + self.updateModel = updateModel self.context = context + self.entityName = entityName } - private lazy var context = Database.shared.makeContext(concurrencyType: .mainQueueConcurrencyType, name: "PixelData") private func loadAll() -> [String: NSObject] { let fetchRequest = PixelData.fetchRequest() as NSFetchRequest var dict = [String: NSObject]() @@ -53,7 +86,7 @@ final class LocalPixelDataStore: PixelDataStore { let result = try context.fetch(fetchRequest) for item in result { guard let record = item.valueRepresentation() else { - assertionFailure("LocalPixelDataStore: Key should not load PixelDataRecord") + assertionFailure("LocalPixelDataStore: could not load PixelDataRecord") continue } @@ -68,32 +101,39 @@ final class LocalPixelDataStore: PixelDataStore { return NSPredicate(format: "key = %@", key) } - private func update(record: PixelDataRecord) { + private func update(record: PixelDataRecord, completionHandler: ((Error?) -> Void)?) { cache[record.key] = record.value let predicate = self.predicate(forKey: record.key) - context.perform { [context] in + func mainQueueCompletion(_ error: Error?) { + guard completionHandler != nil else { return } + DispatchQueue.main.async { + completionHandler?(error) + } + } + + context.perform { [context, updateModel, entityName] in do { - let fetchRequest = NSFetchRequest(entityName: PixelData.className()) + let fetchRequest = NSFetchRequest(entityName: entityName) fetchRequest.predicate = predicate let fetchResults = try context.fetch(fetchRequest) - let managedObject: PixelData = try { - if let managedObject = fetchResults.first as? PixelData { + let managedObject: T = try { + if let managedObject = fetchResults.first as? T { return managedObject - } else if let managedObject = NSEntityDescription.insertNewObject(forEntityName: PixelData.className(), - into: context) as? PixelData { + } else if let managedObject = NSEntityDescription.insertNewObject(forEntityName: entityName, into: context) as? T { return managedObject } - struct ObjectNotFoundError: Error {} - throw ObjectNotFoundError() + assertionFailure("Could not insert new object of type \(entityName)") + throw PixelDataStoreError.objectNotFound }() - try managedObject.update(with: record) + try updateModel(managedObject)(record) try context.save() + mainQueueCompletion(nil) } catch { - assertionFailure("LocalPixelDataStore: Could not update record with \(error)") + mainQueueCompletion(error) } } } @@ -110,23 +150,30 @@ final class LocalPixelDataStore: PixelDataStore { return cache[key] as? String } - func set(_ value: Double, forKey key: String) { - update(record: PixelDataRecord(key: key, value: NSNumber(value: value))) + func set(_ value: Double, forKey key: String, completionHandler: ((Error?) -> Void)?) { + update(record: PixelDataRecord(key: key, value: NSNumber(value: value)), completionHandler: completionHandler) } - func set(_ value: Int, forKey key: String) { - update(record: PixelDataRecord(key: key, value: NSNumber(value: value))) + func set(_ value: Int, forKey key: String, completionHandler: ((Error?) -> Void)?) { + update(record: PixelDataRecord(key: key, value: NSNumber(value: value)), completionHandler: completionHandler) } - func set(_ value: String, forKey key: String) { - update(record: PixelDataRecord(key: key, value: value as NSString)) + func set(_ value: String, forKey key: String, completionHandler: ((Error?) -> Void)?) { + update(record: PixelDataRecord(key: key, value: value as NSString), completionHandler: completionHandler) } - func removeValue(forKey key: String) { + func removeValue(forKey key: String, completionHandler: ((Error?) -> Void)?) { let predicate = self.predicate(forKey: key) - context.perform { [context] in - let deleteRequest = NSFetchRequest(entityName: PixelData.className()) + func mainQueueCompletion(_ error: Error?) { + guard completionHandler != nil else { return } + DispatchQueue.main.async { + completionHandler?(error) + } + } + + context.perform { [context, entityName] in + let deleteRequest = NSFetchRequest(entityName: entityName) deleteRequest.predicate = predicate let batchDeleteRequest = NSBatchDeleteRequest(fetchRequest: deleteRequest) batchDeleteRequest.resultType = .resultTypeObjectIDs @@ -136,8 +183,10 @@ final class LocalPixelDataStore: PixelDataStore { let deletedObjects = result?.result as? [NSManagedObjectID] ?? [] let changes: [AnyHashable: Any] = [NSDeletedObjectsKey: deletedObjects] NSManagedObjectContext.mergeChanges(fromRemoteContextSave: changes, into: [context]) + + mainQueueCompletion(nil) } catch { - assertionFailure("LocalPixelDataStore: Could not remove record with \(error)") + mainQueueCompletion(error) } } } diff --git a/Unit Tests/Common/Extensions/NSPersistentContainerExtension.swift b/Unit Tests/Common/Extensions/NSPersistentContainerExtension.swift index 55fe4b42bc..9910b8da88 100644 --- a/Unit Tests/Common/Extensions/NSPersistentContainerExtension.swift +++ b/Unit Tests/Common/Extensions/NSPersistentContainerExtension.swift @@ -21,7 +21,8 @@ import CoreData extension NSPersistentContainer { - static func createInMemoryPersistentContainer(modelName: String, bundle: Bundle) -> NSPersistentContainer { + static func createPersistentContainer(at url: URL, modelName: String, bundle: Bundle) -> NSPersistentContainer { + guard let modelURL = bundle.url(forResource: modelName, withExtension: "momd") else { fatalError("Error loading model from bundle") } @@ -32,13 +33,8 @@ extension NSPersistentContainer { let container = NSPersistentContainer(name: modelName, managedObjectModel: objectModel) - // Creates a persistent store using the in-memory model, no state will be written to disk. - // This was the approach I had seen recommended in a WWDC session, but there is also a - // `NSInMemoryStoreType` option for doing this. - // - // This approach is apparently the recommended choice: https://www.donnywals.com/setting-up-a-core-data-store-for-unit-tests/ let description = NSPersistentStoreDescription() - description.url = URL(fileURLWithPath: "/dev/null") + description.url = url container.persistentStoreDescriptions = [description] container.loadPersistentStores(completionHandler: { _, error in @@ -50,4 +46,13 @@ extension NSPersistentContainer { return container } + static func createInMemoryPersistentContainer(modelName: String, bundle: Bundle) -> NSPersistentContainer { + // Creates a persistent store using the in-memory model, no state will be written to disk. + // This was the approach I had seen recommended in a WWDC session, but there is also a + // `NSInMemoryStoreType` option for doing this. + // + // This approach is apparently the recommended choice: https://www.donnywals.com/setting-up-a-core-data-store-for-unit-tests/ + return createPersistentContainer(at: URL(fileURLWithPath: "/dev/null"), modelName: modelName, bundle: bundle) + } + } diff --git a/Unit Tests/Statistics/OldPixelDataModel.xcdatamodeld/PixelDataModel.xcdatamodel/contents b/Unit Tests/Statistics/OldPixelDataModel.xcdatamodeld/PixelDataModel.xcdatamodel/contents new file mode 100644 index 0000000000..6cdd9ad587 --- /dev/null +++ b/Unit Tests/Statistics/OldPixelDataModel.xcdatamodeld/PixelDataModel.xcdatamodel/contents @@ -0,0 +1,13 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/Unit Tests/Statistics/PixelStoreMock.swift b/Unit Tests/Statistics/PixelStoreMock.swift index 910168761f..e8c6a87227 100644 --- a/Unit Tests/Statistics/PixelStoreMock.swift +++ b/Unit Tests/Statistics/PixelStoreMock.swift @@ -27,28 +27,32 @@ final class PixelStoreMock: PixelDataStore { return (data[key] as? NSNumber)?.intValue } - func set(_ value: Int, forKey key: String) { + func set(_ value: Int, forKey key: String, completionHandler: ((Error?) -> Void)?) { data[key] = NSNumber(value: value) + completionHandler?(nil) } func value(forKey key: String) -> Double? { return (data[key] as? NSNumber)?.doubleValue } - func set(_ value: Double, forKey key: String) { + func set(_ value: Double, forKey key: String, completionHandler: ((Error?) -> Void)?) { data[key] = NSNumber(value: value) + completionHandler?(nil) } func value(forKey key: String) -> String? { return data[key] as? String } - func set(_ value: String, forKey key: String) { + func set(_ value: String, forKey key: String, completionHandler: ((Error?) -> Void)?) { data[key] = value + completionHandler?(nil) } - func removeValue(forKey key: String) { + func removeValue(forKey key: String, completionHandler: ((Error?) -> Void)?) { data[key] = nil + completionHandler?(nil) } } diff --git a/Unit Tests/Statistics/PixelStoreTests.swift b/Unit Tests/Statistics/PixelStoreTests.swift new file mode 100644 index 0000000000..e05a259aad --- /dev/null +++ b/Unit Tests/Statistics/PixelStoreTests.swift @@ -0,0 +1,88 @@ +// +// PixelStoreTests.swift +// +// Copyright © 2021 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 +@testable import DuckDuckGo_Privacy_Browser +import Combine + +final class PixelStoreTests: XCTestCase { + + let fm = FileManager.default + var tempURL: URL! + let testFile = "pixel_db" + + func clearTemp() { + let tempDir = fm.temporaryDirectory + for file in (try? fm.contentsOfDirectory(atPath: tempDir.path)) ?? [] where file.hasPrefix(testFile) { + try? fm.removeItem(at: tempDir.appendingPathComponent(file)) + } + } + + override func setUp() { + clearTemp() + + self.tempURL = fm.temporaryDirectory + + try? EncryptedValueTransformer.registerTransformer() + try? EncryptedValueTransformer.registerTransformer() + } + + override func tearDown() { + clearTemp() + } + + func testPixelStoreMigration() throws { + let url = tempURL.appendingPathComponent(testFile) + var oldContainer: NSPersistentContainer! = NSPersistentContainer.createPersistentContainer(at: url, + modelName: "OldPixelDataModel", + bundle: Bundle(for: type(of: self))) + var oldContext: NSManagedObjectContext! = oldContainer.viewContext + func updateModelOld(_ managedObject: NSManagedObject) -> (PixelDataRecord) throws -> Void { + { record in + managedObject.setValue(record.key, forKey: #keyPath(PixelData.key)) + managedObject.setValue((record.value as? NSNumber)!, forKey: #keyPath(PixelData.valueEncrypted)) + } + } + var oldStore: LocalPixelDataStore! = LocalPixelDataStore(context: oldContext, updateModel: updateModelOld, entityName: PixelData.className()) + + let e1 = expectation(description: "Double saved") + oldStore.set(1.23, forKey: "a") { error in + XCTAssertNil(error) + e1.fulfill() + } + let e2 = expectation(description: "Int saved") + oldStore.set(1, forKey: "b") { error in + XCTAssertNil(error) + e2.fulfill() + } + + waitForExpectations(timeout: 1) + oldContext = nil + oldStore = nil + oldContainer = nil + + let newContainer = NSPersistentContainer.createPersistentContainer(at: url, + modelName: "PixelDataModel", + bundle: Bundle(for: DuckDuckGo_Privacy_Browser.PixelData.self)) + let newContext = newContainer.viewContext + let newStore = LocalPixelDataStore(context: newContext, updateModel: DuckDuckGo_Privacy_Browser.PixelData.update) + + XCTAssertEqual(newStore.cache, ["b": NSNumber(value: 1), "a": NSNumber(value: 1.23)]) + } + +} From f07cdbd3dc38b0fd42f4dfe159a536e09b557395 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 30 Nov 2021 12:48:50 +0700 Subject: [PATCH 4/7] Removed Pixel event counter --- DuckDuckGo.xcodeproj/project.pbxproj | 4 -- DuckDuckGo/Common/Database/Database.swift | 2 +- .../Configuration/ConfigurationManager.swift | 2 +- .../Store/HTTPSUpgradeStore.swift | 4 +- DuckDuckGo/Statistics/PixelCounter.swift | 53 ------------------- DuckDuckGo/Statistics/PixelEvent.swift | 4 +- DuckDuckGo/Statistics/PixelParameters.swift | 7 +-- Unit Tests/Statistics/PixelTests.swift | 28 ---------- 8 files changed, 7 insertions(+), 97 deletions(-) delete mode 100644 DuckDuckGo/Statistics/PixelCounter.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index e97d3cecb0..d24232f146 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -565,7 +565,6 @@ B6A9E47726146A570067D1B9 /* PixelEvent.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6A9E47626146A570067D1B9 /* PixelEvent.swift */; }; B6A9E47F26146A800067D1B9 /* PixelArguments.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6A9E47E26146A800067D1B9 /* PixelArguments.swift */; }; B6A9E48426146AAB0067D1B9 /* PixelParameters.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6A9E48326146AAB0067D1B9 /* PixelParameters.swift */; }; - B6A9E48926146ABF0067D1B9 /* PixelCounter.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6A9E48826146ABF0067D1B9 /* PixelCounter.swift */; }; B6A9E499261474120067D1B9 /* TimedPixel.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6A9E498261474120067D1B9 /* TimedPixel.swift */; }; B6A9E4A3261475C70067D1B9 /* AppUsageActivityMonitor.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6A9E4A2261475C70067D1B9 /* AppUsageActivityMonitor.swift */; }; B6AAAC2D260330580029438D /* PublishedAfter.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6AAAC2C260330580029438D /* PublishedAfter.swift */; }; @@ -1210,7 +1209,6 @@ B6A9E47626146A570067D1B9 /* PixelEvent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PixelEvent.swift; sourceTree = ""; }; B6A9E47E26146A800067D1B9 /* PixelArguments.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PixelArguments.swift; sourceTree = ""; }; B6A9E48326146AAB0067D1B9 /* PixelParameters.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PixelParameters.swift; sourceTree = ""; }; - B6A9E48826146ABF0067D1B9 /* PixelCounter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PixelCounter.swift; sourceTree = ""; }; B6A9E498261474120067D1B9 /* TimedPixel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimedPixel.swift; sourceTree = ""; }; B6A9E4A2261475C70067D1B9 /* AppUsageActivityMonitor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppUsageActivityMonitor.swift; sourceTree = ""; }; B6AAAC2C260330580029438D /* PublishedAfter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PublishedAfter.swift; sourceTree = ""; }; @@ -3187,7 +3185,6 @@ B6A9E47626146A570067D1B9 /* PixelEvent.swift */, B6A9E47E26146A800067D1B9 /* PixelArguments.swift */, B6A9E48326146AAB0067D1B9 /* PixelParameters.swift */, - B6A9E48826146ABF0067D1B9 /* PixelCounter.swift */, B6A9E4A2261475C70067D1B9 /* AppUsageActivityMonitor.swift */, B6DA44012616B28300DD1EC2 /* PixelDataStore.swift */, B68C92C32750EF76002AC6B0 /* PixelDataRecord.swift */, @@ -3819,7 +3816,6 @@ AA4BBA3B25C58FA200C4FB0F /* MainMenu.swift in Sources */, 4B8AC93326B3B06300879451 /* EdgeDataImporter.swift in Sources */, AA585D84248FD31100E9A3E2 /* BrowserTabViewController.swift in Sources */, - B6A9E48926146ABF0067D1B9 /* PixelCounter.swift in Sources */, B693954B26F04BEB0015B914 /* MouseOverView.swift in Sources */, AAE7527C263B056C00B973F8 /* HistoryStore.swift in Sources */, AAE246F32709EF3B00BEEAEE /* FirePopoverCollectionViewItem.swift in Sources */, diff --git a/DuckDuckGo/Common/Database/Database.swift b/DuckDuckGo/Common/Database/Database.swift index dc767bb1b3..36fd002c2a 100644 --- a/DuckDuckGo/Common/Database/Database.swift +++ b/DuckDuckGo/Common/Database/Database.swift @@ -60,7 +60,7 @@ final class Database { func loadStore(migrationHandler: @escaping (NSManagedObjectContext) -> Void = { _ in }) { container.loadPersistentStores { _, error in if let error = error { - Pixel.fire(.debug(event: .dbInitializationError, error: error, countedBy: .counter)) + Pixel.fire(.debug(event: .dbInitializationError, error: error)) // Give Pixel a chance to be sent, but not too long Thread.sleep(forTimeInterval: 1) fatalError("Could not load DB: \(error.localizedDescription)") diff --git a/DuckDuckGo/Configuration/ConfigurationManager.swift b/DuckDuckGo/Configuration/ConfigurationManager.swift index 36b790c4f5..bed98c3a7d 100644 --- a/DuckDuckGo/Configuration/ConfigurationManager.swift +++ b/DuckDuckGo/Configuration/ConfigurationManager.swift @@ -111,7 +111,7 @@ final class ConfigurationManager { if case .failure(let error) = completion { os_log("Failed to complete configuration update %s", log: .config, type: .error, error.localizedDescription) - Pixel.fire(.debug(event: .configurationFetchError, error: error, countedBy: .counter)) + Pixel.fire(.debug(event: .configurationFetchError, error: error)) tryAgainSoon() } else { diff --git a/DuckDuckGo/Smarter Encryption/Store/HTTPSUpgradeStore.swift b/DuckDuckGo/Smarter Encryption/Store/HTTPSUpgradeStore.swift index d93f3c43e9..595045f47a 100644 --- a/DuckDuckGo/Smarter Encryption/Store/HTTPSUpgradeStore.swift +++ b/DuckDuckGo/Smarter Encryption/Store/HTTPSUpgradeStore.swift @@ -134,7 +134,7 @@ final class HTTPSUpgradePersistence: HTTPSUpgradeStore { do { try context.save() } catch { - Pixel.fire(.debug(event: .dbSaveBloomFilterError, error: error, countedBy: .counter)) + Pixel.fire(.debug(event: .dbSaveBloomFilterError, error: error)) } } } @@ -170,7 +170,7 @@ final class HTTPSUpgradePersistence: HTTPSUpgradeStore { do { try context.save() } catch { - Pixel.fire(.debug(event: .dbSaveExcludedHTTPSDomainsError, error: error, countedBy: .counter)) + Pixel.fire(.debug(event: .dbSaveExcludedHTTPSDomainsError, error: error)) result = false } } diff --git a/DuckDuckGo/Statistics/PixelCounter.swift b/DuckDuckGo/Statistics/PixelCounter.swift deleted file mode 100644 index 796da62888..0000000000 --- a/DuckDuckGo/Statistics/PixelCounter.swift +++ /dev/null @@ -1,53 +0,0 @@ -// -// PixelCounter.swift -// -// Copyright © 2021 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 - -extension Pixel { - - struct Counter { - - private enum Constants { - static let maxCount = 20 - static let prefix = "c_" - } - - let store: PixelDataStore - - static let counter = Counter(store: LocalPixelDataStore.shared) - - init(store: PixelDataStore) { - self.store = store - } - - func incrementedCount(for event: Pixel.Event.Debug) -> Int { - let key = Constants.prefix + event.rawValue - var count = store.value(forKey: key) ?? 0 - - guard count < Constants.maxCount else { return count } - - count += 1 - - store.set(count, forKey: key) - - return count - } - - } - -} diff --git a/DuckDuckGo/Statistics/PixelEvent.swift b/DuckDuckGo/Statistics/PixelEvent.swift index a9e2192f5a..6e93cdde69 100644 --- a/DuckDuckGo/Statistics/PixelEvent.swift +++ b/DuckDuckGo/Statistics/PixelEvent.swift @@ -79,7 +79,7 @@ extension Pixel { case formAutofilled(kind: FormAutofillKind) case autofillItemSaved(kind: FormAutofillKind) - case debug(event: Debug, error: Error? = nil, countedBy: Pixel.Counter? = nil) + case debug(event: Debug, error: Error? = nil) enum Debug: String, CustomStringConvertible { var description: String { rawValue } @@ -195,7 +195,7 @@ extension Pixel.Event { case .autofillItemSaved(kind: let kind): return "m_mac_save_\(kind)" - case .debug(event: let event, error: _, countedBy: _): + case .debug(event: let event, error: _): return "m_mac_debug_\(event)" } } diff --git a/DuckDuckGo/Statistics/PixelParameters.swift b/DuckDuckGo/Statistics/PixelParameters.swift index a47b646608..a83f34cca6 100644 --- a/DuckDuckGo/Statistics/PixelParameters.swift +++ b/DuckDuckGo/Statistics/PixelParameters.swift @@ -46,14 +46,9 @@ extension Pixel.Event { var parameters: [String: String]? { switch self { - case .debug(event: let event, error: let error, countedBy: let counter): + case .debug(event: _, error: let error): var params = [String: String]() - if let counter = counter { - let count = counter.incrementedCount(for: event) - params[Pixel.Parameters.errorCount] = "\(count)" - } - if let error = error { let nsError = error as NSError diff --git a/Unit Tests/Statistics/PixelTests.swift b/Unit Tests/Statistics/PixelTests.swift index ed65f2202b..88c03a70e9 100644 --- a/Unit Tests/Statistics/PixelTests.swift +++ b/Unit Tests/Statistics/PixelTests.swift @@ -174,32 +174,4 @@ class PixelTests: XCTestCase { wait(for: [expectation], timeout: 1.0) } - func testWhenCountedPixelFiresThenCountIncreased() { - let store = PixelStoreMock() - - var expectation = XCTestExpectation() - stub(condition: isHost(host) && isPath("/t/m_mac_debug_cfgfetch")) { request -> HTTPStubsResponse in - XCTAssertEqual("1", try? request.url?.getParameter(name: "c")) - - expectation.fulfill() - return HTTPStubsResponse(data: Data(), statusCode: 200, headers: nil) - } - - Pixel.fire(.debug(event: .configurationFetchError, error: nil, countedBy: .init(store: store))) - - wait(for: [expectation], timeout: 1.0) - - expectation = XCTestExpectation() - stub(condition: isHost(host) && isPath("/t/m_mac_debug_cfgfetch")) { request -> HTTPStubsResponse in - XCTAssertEqual("2", try? request.url?.getParameter(name: "c")) - - expectation.fulfill() - return HTTPStubsResponse(data: Data(), statusCode: 200, headers: nil) - } - - Pixel.fire(.debug(event: .configurationFetchError, error: nil, countedBy: .init(store: store))) - - wait(for: [expectation], timeout: 1.0) - } - } From 39e11199e88e517b87cf7abae36f89a1b958c1a4 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 30 Nov 2021 15:01:09 +0700 Subject: [PATCH 5/7] fix failing test --- Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift b/Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift index 55f518c666..d438e3f9c3 100644 --- a/Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift +++ b/Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift @@ -165,11 +165,14 @@ final class TabViewModelTests: XCTestCase { tabViewModel.tab.setContent(.homepage) let faviconExpectation = expectation(description: "Favicon") + var fulfilled = false tabViewModel.$favicon.debounce(for: 0.1, scheduler: RunLoop.main).sink { favicon in XCTAssertNotNil(favicon) - if favicon == TabViewModel.Favicon.home { + if favicon == TabViewModel.Favicon.home, + !fulfilled { faviconExpectation.fulfill() + fulfilled = true } } .store(in: &cancellables) waitForExpectations(timeout: 5, handler: nil) From b97b0328f176fb01eb1cc270562c8d063bd4d8b4 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 30 Nov 2021 15:45:26 +0700 Subject: [PATCH 6/7] EncryptionKeyStoreMock --- DuckDuckGo.xcodeproj/project.pbxproj | 6 ++ DuckDuckGo/Common/Database/Database.swift | 35 ++++++----- .../EncryptedValueTransformer.swift | 16 ++--- .../CoreDataEncryptionTests.swift | 2 +- .../FileSystem/CoreDataEncryptionTests.swift | 2 +- .../FileSystem/EncryptionKeyStoreMock.swift | 60 +++++++++++++++++++ Unit Tests/Statistics/PixelStoreTests.swift | 4 +- 7 files changed, 93 insertions(+), 32 deletions(-) create mode 100644 Unit Tests/FileSystem/EncryptionKeyStoreMock.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index d24232f146..594de3f396 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -488,6 +488,8 @@ B65E6BA026D9F10600095F96 /* NSBezierPathExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = B65E6B9F26D9F10600095F96 /* NSBezierPathExtension.swift */; }; B662D3D92755D7AD0035D4D6 /* PixelStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B662D3D82755D7AD0035D4D6 /* PixelStoreTests.swift */; }; B662D3DC2755DF670035D4D6 /* OldPixelDataModel.xcdatamodeld in Sources */ = {isa = PBXBuildFile; fileRef = B662D3DA2755D8190035D4D6 /* OldPixelDataModel.xcdatamodeld */; }; + B662D3DE275613BB0035D4D6 /* EncryptionKeyStoreMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = B662D3DD275613BB0035D4D6 /* EncryptionKeyStoreMock.swift */; }; + B662D3DF275616FF0035D4D6 /* EncryptionKeyStoreMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = B662D3DD275613BB0035D4D6 /* EncryptionKeyStoreMock.swift */; }; B66E9DD22670EB2A00E53BB5 /* _WKDownload+WebKitDownload.swift in Sources */ = {isa = PBXBuildFile; fileRef = B66E9DD12670EB2A00E53BB5 /* _WKDownload+WebKitDownload.swift */; }; B66E9DD42670EB4A00E53BB5 /* WKDownload+WebKitDownload.swift in Sources */ = {isa = PBXBuildFile; fileRef = B66E9DD32670EB4A00E53BB5 /* WKDownload+WebKitDownload.swift */; }; B67C6C3D2654B897006C872E /* WebViewExtensionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B67C6C3C2654B897006C872E /* WebViewExtensionTests.swift */; }; @@ -1130,6 +1132,7 @@ B65E6B9F26D9F10600095F96 /* NSBezierPathExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSBezierPathExtension.swift; sourceTree = ""; }; B662D3D82755D7AD0035D4D6 /* PixelStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PixelStoreTests.swift; sourceTree = ""; }; B662D3DB2755D81A0035D4D6 /* PixelDataModel.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = PixelDataModel.xcdatamodel; sourceTree = ""; }; + B662D3DD275613BB0035D4D6 /* EncryptionKeyStoreMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EncryptionKeyStoreMock.swift; sourceTree = ""; }; B66E9DD12670EB2A00E53BB5 /* _WKDownload+WebKitDownload.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "_WKDownload+WebKitDownload.swift"; sourceTree = ""; }; B66E9DD32670EB4A00E53BB5 /* WKDownload+WebKitDownload.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WKDownload+WebKitDownload.swift"; sourceTree = ""; }; B67C6C3C2654B897006C872E /* WebViewExtensionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebViewExtensionTests.swift; sourceTree = ""; }; @@ -1730,6 +1733,7 @@ 4BA1A6DD258C100A00F6F690 /* FileStoreTests.swift */, 4B11060925903EAC0039B979 /* CoreDataEncryptionTests.swift */, 4B11060325903E570039B979 /* CoreDataEncryptionTesting.xcdatamodeld */, + B662D3DD275613BB0035D4D6 /* EncryptionKeyStoreMock.swift */, B6A5A27825B93FFE00AA7ADA /* StateRestorationManagerTests.swift */, B6A5A27D25B9403E00AA7ADA /* FileStoreMock.swift */, ); @@ -3574,6 +3578,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + B662D3DF275616FF0035D4D6 /* EncryptionKeyStoreMock.swift in Sources */, 4B1AD8E225FC390B00261379 /* EncryptionMocks.swift in Sources */, 4B1AD91725FC46FB00261379 /* CoreDataEncryptionTests.swift in Sources */, 7BA4727D26F01BC400EAA165 /* CoreDataTestUtilities.swift in Sources */, @@ -4013,6 +4018,7 @@ 4B9292BB2667103100AD2C21 /* BookmarkNodeTests.swift in Sources */, 4B0219A825E0646500ED7DEA /* WebsiteDataStoreTests.swift in Sources */, AAC9C01E24CB6BEB00AD1325 /* TabCollectionViewModelTests.swift in Sources */, + B662D3DE275613BB0035D4D6 /* EncryptionKeyStoreMock.swift in Sources */, AA0877BA26D5161D00B05660 /* WebKitVersionProviderTests.swift in Sources */, B69B50462726C5C200758A2B /* AtbAndVariantCleanupTests.swift in Sources */, 85E11C3725E7F1E100974CAF /* ExternalURLHandlerTests.swift in Sources */, diff --git a/DuckDuckGo/Common/Database/Database.swift b/DuckDuckGo/Common/Database/Database.swift index 36fd002c2a..d3d5625c94 100644 --- a/DuckDuckGo/Common/Database/Database.swift +++ b/DuckDuckGo/Common/Database/Database.swift @@ -25,7 +25,16 @@ final class Database { static let databaseName = "Database" } - static let shared = Database() + static let shared: Database = { +#if DEBUG + if AppDelegate.isRunningTests { + let keyStoreMockClass = (NSClassFromString("EncryptionKeyStoreMock") as? NSObject.Type)! + let keyStoreMock = (keyStoreMockClass.init() as? EncryptionKeyStoring)! + return Database(keyStore: keyStoreMock) + } +#endif + return Database() + }() private let container: NSPersistentContainer private let storeLoadedCondition = RunLoop.ResumeCondition() @@ -33,23 +42,17 @@ final class Database { var model: NSManagedObjectModel { return container.managedObjectModel } - - convenience init() { - let mainBundle = Bundle.main - guard let managedObjectModel = NSManagedObjectModel.mergedModel(from: [mainBundle]) else { fatalError("No DB scheme found") } - - self.init(name: Constants.databaseName, model: managedObjectModel) - } - - init(name: String, model: NSManagedObjectModel) { + init(name: String = Constants.databaseName, + model: NSManagedObjectModel = NSManagedObjectModel.mergedModel(from: [.main])!, + keyStore: EncryptionKeyStoring = EncryptionKeyStore(generator: EncryptionKeyGenerator())) { do { - try EncryptedValueTransformer.registerTransformer() - try EncryptedValueTransformer.registerTransformer() - try EncryptedValueTransformer.registerTransformer() - try EncryptedValueTransformer.registerTransformer() - try EncryptedValueTransformer.registerTransformer() - try EncryptedValueTransformer.registerTransformer() + try EncryptedValueTransformer.registerTransformer(keyStore: keyStore) + try EncryptedValueTransformer.registerTransformer(keyStore: keyStore) + try EncryptedValueTransformer.registerTransformer(keyStore: keyStore) + try EncryptedValueTransformer.registerTransformer(keyStore: keyStore) + try EncryptedValueTransformer.registerTransformer(keyStore: keyStore) + try EncryptedValueTransformer.registerTransformer(keyStore: keyStore) } catch { fatalError("Failed to register encryption value transformers") } diff --git a/DuckDuckGo/Common/FileSystem/EncryptedValueTransformer.swift b/DuckDuckGo/Common/FileSystem/EncryptedValueTransformer.swift index 1e9f18825a..168bd395f4 100644 --- a/DuckDuckGo/Common/FileSystem/EncryptedValueTransformer.swift +++ b/DuckDuckGo/Common/FileSystem/EncryptedValueTransformer.swift @@ -75,19 +75,11 @@ final class EncryptedValueTransformer: ValueTransf return NSValueTransformerName("\(className)Transformer") } - static func registerTransformer() throws { - #if CI - // Don't register transformers when running on the CI, they'll fail to read the Keychain due to the build machines not being provisioned. - return - #else - let generator = EncryptionKeyGenerator() - let keyStore = EncryptionKeyStore(generator: generator) - let key = try keyStore.readKey() - let transformer = EncryptedValueTransformer(encryptionKey: key) - - ValueTransformer.setValueTransformer(transformer, forName: transformerName) - #endif + static func registerTransformer(keyStore: EncryptionKeyStoring) throws { + let key = try keyStore.readKey() + let transformer = EncryptedValueTransformer(encryptionKey: key) + ValueTransformer.setValueTransformer(transformer, forName: transformerName) } } diff --git a/Integration Tests/CoreDataEncryptionTests.swift b/Integration Tests/CoreDataEncryptionTests.swift index 44bd1e5b80..089017272f 100644 --- a/Integration Tests/CoreDataEncryptionTests.swift +++ b/Integration Tests/CoreDataEncryptionTests.swift @@ -38,7 +38,7 @@ class CoreDataEncryptionTests: XCTestCase { super.setUp() mockValueTransformer.numberOfTransformations = 0 - try? EncryptedValueTransformer.registerTransformer() + try? EncryptedValueTransformer.registerTransformer(keyStore: EncryptionKeyStoreMock()) } func testSavingIncorrectValueTypes() { diff --git a/Unit Tests/FileSystem/CoreDataEncryptionTests.swift b/Unit Tests/FileSystem/CoreDataEncryptionTests.swift index f8b2a55655..b7457558c3 100644 --- a/Unit Tests/FileSystem/CoreDataEncryptionTests.swift +++ b/Unit Tests/FileSystem/CoreDataEncryptionTests.swift @@ -36,7 +36,7 @@ final class CoreDataEncryptionTests: XCTestCase { super.setUp() mockValueTransformer.numberOfTransformations = 0 - try? EncryptedValueTransformer.registerTransformer() + try? EncryptedValueTransformer.registerTransformer(keyStore: EncryptionKeyStoreMock()) } func testSavingEncryptedValues() { diff --git a/Unit Tests/FileSystem/EncryptionKeyStoreMock.swift b/Unit Tests/FileSystem/EncryptionKeyStoreMock.swift new file mode 100644 index 0000000000..c4a57910ca --- /dev/null +++ b/Unit Tests/FileSystem/EncryptionKeyStoreMock.swift @@ -0,0 +1,60 @@ +// +// EncryptionKeyStoreMock.swift +// +// Copyright © 2021 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 +@testable import DuckDuckGo_Privacy_Browser +import CryptoKit + +@objc(EncryptionKeyStoreMock) +final class EncryptionKeyStoreMock: NSObject, EncryptionKeyStoring { + let generator: EncryptionKeyGenerating + var key: SymmetricKey? + var error: Error? + + init(key: SymmetricKey?, generator: EncryptionKeyGenerating = EncryptionKeyGenerator()) { + self.key = key + self.generator = generator + super.init() + } + + override init() { + self.generator = EncryptionKeyGenerator() + super.init() + } + + func store(key: SymmetricKey) throws { + if let error = error { throw error } + self.key = key + } + + func readKey() throws -> SymmetricKey { + if let error = error { throw error } + if let key = self.key { + return key + } + let key = generator.randomKey() + self.key = key + return key + } + + func deleteKey() throws { + if let error = error { throw error } + self.key = nil + } + +} diff --git a/Unit Tests/Statistics/PixelStoreTests.swift b/Unit Tests/Statistics/PixelStoreTests.swift index e05a259aad..ec32dad499 100644 --- a/Unit Tests/Statistics/PixelStoreTests.swift +++ b/Unit Tests/Statistics/PixelStoreTests.swift @@ -38,8 +38,8 @@ final class PixelStoreTests: XCTestCase { self.tempURL = fm.temporaryDirectory - try? EncryptedValueTransformer.registerTransformer() - try? EncryptedValueTransformer.registerTransformer() + try? EncryptedValueTransformer.registerTransformer(keyStore: EncryptionKeyStoreMock()) + try? EncryptedValueTransformer.registerTransformer(keyStore: EncryptionKeyStoreMock()) } override func tearDown() { From f7806abf1d9d61aacda095491e3e4f91e936d000 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 30 Nov 2021 16:26:37 +0700 Subject: [PATCH 7/7] fixing tests --- Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift | 9 ++++----- Unit Tests/Statistics/PixelStoreTests.swift | 5 +++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift b/Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift index d438e3f9c3..b2d826e722 100644 --- a/Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift +++ b/Unit Tests/BrowserTab/ViewModel/TabViewModelTests.swift @@ -168,7 +168,7 @@ final class TabViewModelTests: XCTestCase { var fulfilled = false tabViewModel.$favicon.debounce(for: 0.1, scheduler: RunLoop.main).sink { favicon in - XCTAssertNotNil(favicon) + guard favicon != nil else { return } if favicon == TabViewModel.Favicon.home, !fulfilled { faviconExpectation.fulfill() @@ -185,11 +185,10 @@ final class TabViewModelTests: XCTestCase { let faviconExpectation = expectation(description: "Favicon") tabViewModel.$favicon.debounce(for: 1, scheduler: RunLoop.main).sink { favicon in - XCTAssertNotNil(favicon) + guard favicon != nil else { return } + XCTAssertNotEqual(favicon, TabViewModel.Favicon.home) - if favicon != nil { - faviconExpectation.fulfill() - } + faviconExpectation.fulfill() } .store(in: &cancellables) waitForExpectations(timeout: 5, handler: nil) } diff --git a/Unit Tests/Statistics/PixelStoreTests.swift b/Unit Tests/Statistics/PixelStoreTests.swift index ec32dad499..d9a79d3902 100644 --- a/Unit Tests/Statistics/PixelStoreTests.swift +++ b/Unit Tests/Statistics/PixelStoreTests.swift @@ -38,8 +38,9 @@ final class PixelStoreTests: XCTestCase { self.tempURL = fm.temporaryDirectory - try? EncryptedValueTransformer.registerTransformer(keyStore: EncryptionKeyStoreMock()) - try? EncryptedValueTransformer.registerTransformer(keyStore: EncryptionKeyStoreMock()) + let keyStore = EncryptionKeyStoreMock() + try? EncryptedValueTransformer.registerTransformer(keyStore: keyStore) + try? EncryptedValueTransformer.registerTransformer(keyStore: keyStore) } override func tearDown() {