diff --git a/Core/DefaultVariantManager.swift b/Core/DefaultVariantManager.swift index 91859126e4..1f0a250f4e 100644 --- a/Core/DefaultVariantManager.swift +++ b/Core/DefaultVariantManager.swift @@ -46,21 +46,32 @@ public struct VariantIOS: Variant { return false } } - + + /// This variant is used for returning users to separate them from really new users. + static let returningUser = VariantIOS(name: "ru", weight: doNotAllocate, isIncluded: When.always, features: []) + static let doNotAllocate = 0 - - // Note: Variants with `doNotAllocate` weight, should always be included so that previous installations are unaffected + + /// The list of cohorts in active ATB experiments. + /// + /// Variants set to `doNotAllocate` are active, but not adding users to a new cohort, do not change them unless you're sure the experiment is finished. public static let defaultVariants: [Variant] = [ - // SERP testing VariantIOS(name: "sc", weight: doNotAllocate, isIncluded: When.always, features: []), VariantIOS(name: "sd", weight: doNotAllocate, isIncluded: When.always, features: []), - VariantIOS(name: "se", weight: doNotAllocate, isIncluded: When.always, features: []) - + VariantIOS(name: "se", weight: doNotAllocate, isIncluded: When.always, features: []), + returningUser ] - + + /// The name of the variant. Shuld be a two character string like `ma` or `mb` public var name: String + + /// The relative weight of this variant, e.g. if two variants have the same weight they will get 50% of the cohorts each. public var weight: Int + + /// Function to determine inclusion, e.g. if you want to only run an experiment on English users use `When.inEnglish` public var isIncluded: () -> Bool + + /// The experimental feature(s) being tested. public var features: [FeatureName] } @@ -81,13 +92,26 @@ public class DefaultVariantManager: VariantManager { private let variants: [Variant] private let storage: StatisticsStore private let rng: VariantRNG + private let returningUserMeasurement: ReturnUserMeasurement - public init(variants: [Variant] = VariantIOS.defaultVariants, - storage: StatisticsStore = StatisticsUserDefaults(), - rng: VariantRNG = Arc4RandomUniformVariantRNG()) { + init(variants: [Variant], + storage: StatisticsStore, + rng: VariantRNG, + returningUserMeasurement: ReturnUserMeasurement) { + self.variants = variants self.storage = storage self.rng = rng + self.returningUserMeasurement = returningUserMeasurement + } + + public convenience init() { + self.init( + variants: VariantIOS.defaultVariants, + storage: StatisticsUserDefaults(), + rng: Arc4RandomUniformVariantRNG(), + returningUserMeasurement: KeychainReturnUserMeasurement() + ) } public func isSupported(feature: FeatureName) -> Bool { @@ -118,6 +142,10 @@ public class DefaultVariantManager: VariantManager { } private func selectVariant() -> Variant? { + if returningUserMeasurement.isReturningUser { + return VariantIOS.returningUser + } + let totalWeight = variants.reduce(0, { $0 + $1.weight }) let randomPercent = rng.nextInt(upperBound: totalWeight) diff --git a/Core/Pixel.swift b/Core/Pixel.swift index 8955f627a7..b3606fbd86 100644 --- a/Core/Pixel.swift +++ b/Core/Pixel.swift @@ -111,6 +111,11 @@ public struct PixelParameters { public static let sheetResult = "success" public static let defaultBrowser = "default_browser" + + // Return user + public static let returnUserErrorCode = "error_code" + public static let returnUserOldATB = "old_atb" + public static let returnUserNewATB = "new_atb" } public struct PixelValues { diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index 34c1adecdb..1762d0a501 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -306,6 +306,9 @@ extension Pixel { case onboardingSetDefaultOpened case onboardingSetDefaultSkipped + // MARK: Return user measurement + case returnUser + // MARK: debug pixels case dbCrashDetected @@ -392,6 +395,11 @@ extension Pixel { case debugCantSaveBookmarkFix case debugCannotClearObservationsDatabase + + // Return user measurement + case debugReturnUserReadATB + case debugReturnUserAddATB + case debugReturnUserUpdateATB // Errors from Bookmarks Module case bookmarkFolderExpected @@ -881,6 +889,13 @@ extension Pixel.Event { case .emailIncontextModalDismissed: return "m_email_incontext_modal_dismissed" case .emailIncontextModalExitEarly: return "m_email_incontext_modal_exit_early" case .emailIncontextModalExitEarlyContinue: return "m_email_incontext_modal_exit_early_continue" + + // MARK: - Return user measurement + case .returnUser: return "m_return_user" + case .debugReturnUserAddATB: return "m_debug_return_user_add_atb" + case .debugReturnUserReadATB: return "m_debug_return_user_read_atb" + case .debugReturnUserUpdateATB: return "m_debug_return_user_update_atb" + } } diff --git a/Core/ReturnUserMeasurement.swift b/Core/ReturnUserMeasurement.swift new file mode 100644 index 0000000000..0dce183c42 --- /dev/null +++ b/Core/ReturnUserMeasurement.swift @@ -0,0 +1,150 @@ +// +// ReturnUserMeasurement.swift +// DuckDuckGo +// +// 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 Foundation +import BrowserServicesKit + +protocol ReturnUserMeasurement { + + var isReturningUser: Bool { get } + func installCompletedWithATB(_ atb: Atb) + func updateStoredATB(_ atb: Atb) + +} + +class KeychainReturnUserMeasurement: ReturnUserMeasurement { + + static let SecureATBKeychainName = "returning-user-atb" + + struct Measurement { + + let oldATB: String? + let newATB: String + + } + + /// Called from the `VariantManager` to determine which variant to use + var isReturningUser: Bool { + return hasAnyKeychainItems() + } + + func installCompletedWithATB(_ atb: Atb) { + if let oldATB = readSecureATB() { + sendReturnUserMeasurement(oldATB, atb.version) + } + writeSecureATB(atb.version) + } + + /// Update the stored ATB with an even more generalised version of the ATB, if present. + func updateStoredATB(_ atb: Atb) { + guard let atb = atb.updateVersion else { return } + writeSecureATB(atb) + } + + private func writeSecureATB(_ atb: String) { + let data = atb.data(using: .utf8)! + + var query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: Self.SecureATBKeychainName, + kSecValueData as String: data, + + // We expect to only need access when the app is in the foreground and we want it to be migrated to new devices. + kSecAttrAccessible as String: kSecAttrAccessibleWhenUnlocked, + + // Just to be explicit that we don't want this stored in the cloud + kSecAttrSynchronizable as String: false + ] + + var status = SecItemAdd(query as CFDictionary, nil) + if status == errSecDuplicateItem { + let attributesToUpdate: [String: Any] = [ + kSecValueData as String: data + ] + query.removeValue(forKey: kSecValueData as String) + status = SecItemUpdate(query as CFDictionary, attributesToUpdate as CFDictionary) + if status != errSecSuccess { + fireDebugPixel(.debugReturnUserUpdateATB, errorCode: status) + } + } else if status != errSecSuccess { + fireDebugPixel(.debugReturnUserAddATB, errorCode: status) + } + + } + + private func readSecureATB() -> String? { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrAccount as String: Self.SecureATBKeychainName, + kSecReturnData as String: kCFBooleanTrue!, + kSecMatchLimit as String: kSecMatchLimitOne + ] + + var dataTypeRef: AnyObject? + let status: OSStatus = SecItemCopyMatching(query as CFDictionary, &dataTypeRef) + if ![errSecSuccess, errSecItemNotFound].contains(status) { + fireDebugPixel(.debugReturnUserReadATB, errorCode: status) + } + + if let data = dataTypeRef as? Data { + return String(data: data, encoding: .utf8) + } + + return nil + } + + private func sendReturnUserMeasurement(_ oldATB: String, _ newATB: String) { + Pixel.fire(pixel: .returnUser, withAdditionalParameters: [ + PixelParameters.returnUserOldATB: oldATB, + PixelParameters.returnUserNewATB: newATB + ]) + } + + private func fireDebugPixel(_ event: Pixel.Event, errorCode: OSStatus) { + Pixel.fire(pixel: event, withAdditionalParameters: [ + PixelParameters.returnUserErrorCode: "\(errorCode)" + ]) + } + + /// Only check for keychain items created by *this* app. + private func hasAnyKeychainItems() -> Bool { + let possibleStorageClasses = [ + kSecClassGenericPassword, + kSecClassKey + ] + return possibleStorageClasses.first(where: hasKeychainItemsInClass(_:)) != nil + } + + private func hasKeychainItemsInClass(_ secClassCFString: CFString) -> Bool { + let query: [String: Any] = [ + kSecClass as String: secClassCFString, + kSecMatchLimit as String: kSecMatchLimitOne, + kSecReturnAttributes as String: true, // Needs to be true or returns nothing. + kSecReturnRef as String: true, + ] + var returnArrayRef: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &returnArrayRef) + guard status == errSecSuccess, + let returnArray = returnArrayRef as? [String: Any] else { + return false + } + return returnArray.count > 0 + } + +} diff --git a/Core/StatisticsLoader.swift b/Core/StatisticsLoader.swift index 3f59087d26..9fa6967c2e 100644 --- a/Core/StatisticsLoader.swift +++ b/Core/StatisticsLoader.swift @@ -29,10 +29,13 @@ public class StatisticsLoader { public static let shared = StatisticsLoader() private let statisticsStore: StatisticsStore + private let returnUserMeasurement: ReturnUserMeasurement private let parser = AtbParser() - init(statisticsStore: StatisticsStore = StatisticsUserDefaults()) { + init(statisticsStore: StatisticsStore = StatisticsUserDefaults(), + returnUserMeasurement: ReturnUserMeasurement = KeychainReturnUserMeasurement()) { self.statisticsStore = statisticsStore + self.returnUserMeasurement = returnUserMeasurement } public func load(completion: @escaping Completion = {}) { @@ -77,6 +80,7 @@ public class StatisticsLoader { } self.statisticsStore.installDate = Date() self.statisticsStore.atb = atb.version + self.returnUserMeasurement.installCompletedWithATB(atb) completion() } } @@ -134,6 +138,7 @@ public class StatisticsLoader { public func storeUpdateVersionIfPresent(_ atb: Atb) { if let updateVersion = atb.updateVersion { statisticsStore.atb = updateVersion + returnUserMeasurement.updateStoredATB(atb) } } } diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index bf939435bd..bc2d341bd0 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -470,6 +470,7 @@ 85DFEDF124C7EEA400973FE7 /* LargeOmniBarState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85DFEDF024C7EEA400973FE7 /* LargeOmniBarState.swift */; }; 85DFEDF724CB1CAB00973FE7 /* ShareSheet.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 85DFEDF624CB1CAB00973FE7 /* ShareSheet.xcassets */; }; 85DFEDF924CF3D0E00973FE7 /* TabsBarCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85DFEDF824CF3D0E00973FE7 /* TabsBarCell.swift */; }; + 85E242172AB1B54D000F3E28 /* ReturnUserMeasurement.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85E242162AB1B54D000F3E28 /* ReturnUserMeasurement.swift */; }; 85E5603026541D9E00F4DC44 /* AutocompleteRequestTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85E5602E26541D1D00F4DC44 /* AutocompleteRequestTests.swift */; }; 85E58C2C28FDA94F006A801A /* FavoritesViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85E58C2B28FDA94F006A801A /* FavoritesViewController.swift */; }; 85EE7F55224667DD000FE757 /* WebContainer.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 85EE7F54224667DD000FE757 /* WebContainer.storyboard */; }; @@ -1460,6 +1461,7 @@ 85DFEDF024C7EEA400973FE7 /* LargeOmniBarState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LargeOmniBarState.swift; sourceTree = ""; }; 85DFEDF624CB1CAB00973FE7 /* ShareSheet.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = ShareSheet.xcassets; sourceTree = ""; }; 85DFEDF824CF3D0E00973FE7 /* TabsBarCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabsBarCell.swift; sourceTree = ""; }; + 85E242162AB1B54D000F3E28 /* ReturnUserMeasurement.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReturnUserMeasurement.swift; sourceTree = ""; }; 85E5602E26541D1D00F4DC44 /* AutocompleteRequestTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutocompleteRequestTests.swift; sourceTree = ""; }; 85E58C2B28FDA94F006A801A /* FavoritesViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FavoritesViewController.swift; sourceTree = ""; }; 85EE7F54224667DD000FE757 /* WebContainer.storyboard */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; path = WebContainer.storyboard; sourceTree = ""; }; @@ -4393,6 +4395,7 @@ 1E05D1D729C46EDA00BF9A1F /* TimedPixel.swift */, 1E05D1D529C46EBB00BF9A1F /* DailyPixel.swift */, 8577C65F2A964BAC00788B3A /* SetAsDefaultStatistics.swift */, + 85E242162AB1B54D000F3E28 /* ReturnUserMeasurement.swift */, ); name = Statistics; sourceTree = ""; @@ -6619,6 +6622,7 @@ B652DF10287C2C1600C12A9C /* ContentBlocking.swift in Sources */, 4BE2756827304F57006B20B0 /* URLRequestExtension.swift in Sources */, 85BA79911F6FF75000F59015 /* ContentBlockerStoreConstants.swift in Sources */, + 85E242172AB1B54D000F3E28 /* ReturnUserMeasurement.swift in Sources */, 85BDC3142434D8F80053DB07 /* DebugUserScript.swift in Sources */, 85011867290028C400BDEE27 /* BookmarksDatabase.swift in Sources */, 85D2187B24BF9F85004373D2 /* FaviconUserScript.swift in Sources */, diff --git a/DuckDuckGoTests/VariantManagerTests.swift b/DuckDuckGoTests/VariantManagerTests.swift index b4b42f6ccf..edb1c05c9c 100644 --- a/DuckDuckGoTests/VariantManagerTests.swift +++ b/DuckDuckGoTests/VariantManagerTests.swift @@ -33,7 +33,8 @@ class VariantManagerTests: XCTestCase { func testWhenVariantIsExcludedThenItIsNotInVariantList() { - let subject = DefaultVariantManager(variants: testVariants, storage: MockStatisticsStore(), rng: MockVariantRNG(returnValue: 500)) + let subject = DefaultVariantManager(variants: testVariants, storage: MockStatisticsStore(), rng: MockVariantRNG(returnValue: 500), + returningUserMeasurement: MockReturningUserMeasurement()) XCTAssertTrue(!subject.isSupported(feature: .dummy)) } @@ -46,7 +47,8 @@ class VariantManagerTests: XCTestCase { let mockStore = MockStatisticsStore() mockStore.variant = "test" - let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: 0)) + let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: 0), + returningUserMeasurement: MockReturningUserMeasurement()) // temporarily use this feature name XCTAssertTrue(subject.isSupported(feature: .dummy)) @@ -62,7 +64,8 @@ class VariantManagerTests: XCTestCase { for i in 0 ..< 100 { - let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: i)) + let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: i), + returningUserMeasurement: MockReturningUserMeasurement()) subject.assignVariantIfNeeded { _ in } XCTAssertNotEqual("mt", subject.currentVariant?.name) @@ -75,7 +78,8 @@ class VariantManagerTests: XCTestCase { let mockStore = MockStatisticsStore() mockStore.atb = "atb" - let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: 0)) + let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: 0), + returningUserMeasurement: MockReturningUserMeasurement()) subject.assignVariantIfNeeded { _ in } XCTAssertNil(subject.currentVariant) @@ -84,7 +88,8 @@ class VariantManagerTests: XCTestCase { func testWhenVariantAssignedAndUsingDefaultRNGThenReturnsValidVariant() { let variant = VariantIOS(name: "anything", weight: 100, isIncluded: VariantIOS.When.always, features: []) - let subject = DefaultVariantManager(variants: [variant], storage: MockStatisticsStore()) + let subject = DefaultVariantManager(variants: [variant], storage: MockStatisticsStore(), rng: MockVariantRNG(returnValue: 0), + returningUserMeasurement: MockReturningUserMeasurement()) subject.assignVariantIfNeeded { _ in } XCTAssertEqual(variant.name, subject.currentVariant?.name) @@ -94,7 +99,8 @@ class VariantManagerTests: XCTestCase { let mockStore = MockStatisticsStore() mockStore.variant = "mb" - let subject = DefaultVariantManager(variants: testVariants, storage: mockStore) + let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: 0), + returningUserMeasurement: MockReturningUserMeasurement()) XCTAssertEqual("mb", subject.currentVariant?.name) XCTAssertEqual("mb", mockStore.variant) @@ -112,7 +118,8 @@ class VariantManagerTests: XCTestCase { func testWhenVariantAssignedThenReturnsRandomVariantAndSavesIt() { let mockStore = MockStatisticsStore() - let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: 0)) + let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: 0), + returningUserMeasurement: MockReturningUserMeasurement()) subject.assignVariantIfNeeded { _ in } XCTAssertEqual("mb", subject.currentVariant?.name) XCTAssertEqual("mb", mockStore.variant) @@ -122,18 +129,21 @@ class VariantManagerTests: XCTestCase { func testWhenNewThenCurrentVariantIsNil() { let mockStore = MockStatisticsStore() - let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: 0)) + let subject = DefaultVariantManager(variants: testVariants, storage: mockStore, rng: MockVariantRNG(returnValue: 0), + returningUserMeasurement: MockReturningUserMeasurement()) XCTAssertNil(subject.currentVariant) } func testWhenNoVariantsThenAssignsNothing() { - let subject = DefaultVariantManager(variants: [], storage: MockStatisticsStore()) + let subject = DefaultVariantManager(variants: [], storage: MockStatisticsStore(), rng: MockVariantRNG(returnValue: 0), + returningUserMeasurement: MockReturningUserMeasurement()) XCTAssertNil(subject.currentVariant) } private func assignedVariantManager(withRNG rng: VariantRNG) -> VariantManager { - let variantManager = DefaultVariantManager(variants: testVariants, storage: MockStatisticsStore(), rng: rng) + let variantManager = DefaultVariantManager(variants: testVariants, storage: MockStatisticsStore(), rng: rng, + returningUserMeasurement: MockReturningUserMeasurement()) variantManager.assignVariantIfNeeded { _ in } return variantManager } @@ -149,3 +159,11 @@ struct MockVariantRNG: VariantRNG { } } + +class MockReturningUserMeasurement: ReturnUserMeasurement { + var isReturningUser: Bool = false + func installCompletedWithATB(_ atb: Core.Atb) { + } + func updateStoredATB(_ atb: Core.Atb) { + } +}