From 8950b7a4757889f6e1ac3f965c38f0e037ca78ee Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Fri, 3 May 2024 17:53:33 -0700 Subject: [PATCH 01/10] Add JsonTargetPercentile to the config. --- .../Model/JsonRemoteMessagingConfig.swift | 5 +++++ .../JsonToRemoteConfigModelMapperTests.swift | 6 +++++- .../Resources/remote-messaging-config.json | 14 +++++++++++++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift b/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift index 1b73be27f..eda32d920 100644 --- a/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift +++ b/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift @@ -64,8 +64,13 @@ public enum RemoteMessageResponse { let secondaryActionText: String? } + struct JsonTargetPercentile: Decodable { + let before: Float? + } + struct JsonMatchingRule: Decodable { let id: Int + let targetPercentile: JsonTargetPercentile? let attributes: [String: AnyDecodable] } diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift index 6b0a038c3..486961eef 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift @@ -102,7 +102,7 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { func testWhenValidJsonParsedThenRulesMappedIntoRemoteConfig() throws { let config = try decodeAndMapJson(fileName: "Resources/remote-messaging-config.json") - XCTAssertTrue(config.rules.count == 4) + XCTAssertTrue(config.rules.count == 5) let rule5 = config.rules.filter { $0.key == 5 }.first XCTAssertNotNil(rule5) @@ -135,6 +135,10 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { attribs = rule8?.value.filter { $0 is IsNetPWaitlistUserMatchingAttribute } XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? IsNetPWaitlistUserMatchingAttribute, IsNetPWaitlistUserMatchingAttribute(value: true, fallback: nil)) + + let rule9 = config.rules.filter { $0.key == 8 }.first + XCTAssertNotNil(rule9) + XCTAssertTrue(rule9?.value.count == 2) } func testWhenJsonMessagesHaveUnknownTypesThenMessagesNotMappedIntoConfig() throws { diff --git a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json index fea7adc2d..e7a3d8fab 100644 --- a/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json +++ b/Tests/BrowserServicesKitTests/Resources/remote-messaging-config.json @@ -244,6 +244,18 @@ "min": 5 } } - } + }, + { + "id": 9, + "targetPercentile": { + "before": 0.9 + }, + "attributes": { + "daysSinceInstalled": { + "min": 5, + "max": 8 + }, + } + }, ] } From fe147e0a477e4b4984ac9af9bca90da5283a82dc Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Fri, 3 May 2024 18:11:21 -0700 Subject: [PATCH 02/10] Add a `RemoteConfigRule` object. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We used to only care about attributes and their rule ID, but now we’re starting to add additional properties so it’s time to turn rule into a class. --- .../JsonToRemoteMessageModelMapper.swift | 26 ++++++++++++------- .../Model/RemoteConfigModel.swift | 12 ++++++++- .../RemoteMessagingConfigMatcher.swift | 12 ++++----- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift index 5a119c7c2..88af0abce 100644 --- a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift +++ b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift @@ -197,21 +197,29 @@ struct JsonToRemoteMessageModelMapper { } } - static func maps(jsonRemoteRules: [RemoteMessageResponse.JsonMatchingRule]) -> [Int: [MatchingAttribute]] { - var rules: [Int: [MatchingAttribute]] = [:] - jsonRemoteRules.forEach { rule in - var matchingAttributes: [MatchingAttribute] = [] - rule.attributes.forEach { attribute in + static func maps(jsonRemoteRules: [RemoteMessageResponse.JsonMatchingRule]) -> [RemoteConfigRule] { + return jsonRemoteRules.map { jsonRule in + let mappedAttributes = jsonRule.attributes.map { attribute in if let key = AttributesKey(rawValue: attribute.key) { - matchingAttributes.append(key.matchingAttribute(jsonMatchingAttribute: attribute.value)) + return key.matchingAttribute(jsonMatchingAttribute: attribute.value) } else { os_log("Unknown attribute key %s", log: .remoteMessaging, type: .debug, attribute.key) - matchingAttributes.append(UnknownMatchingAttribute(jsonMatchingAttribute: attribute.value)) + return UnknownMatchingAttribute(jsonMatchingAttribute: attribute.value) } } - rules[rule.id] = matchingAttributes + + var mappedTargetPercentile: RemoteConfigTargetPercentile? + + if let jsonTargetPercentile = jsonRule.targetPercentile { + mappedTargetPercentile = .init(before: jsonTargetPercentile.before) + } + + return RemoteConfigRule( + id: jsonRule.id, + targetPercentile: mappedTargetPercentile, + attributes: mappedAttributes + ) } - return rules } static func getTranslation(from translations: [String: RemoteMessageResponse.JsonContentTranslation]?, diff --git a/Sources/RemoteMessaging/Model/RemoteConfigModel.swift b/Sources/RemoteMessaging/Model/RemoteConfigModel.swift index a94021801..01efdcccc 100644 --- a/Sources/RemoteMessaging/Model/RemoteConfigModel.swift +++ b/Sources/RemoteMessaging/Model/RemoteConfigModel.swift @@ -20,5 +20,15 @@ import Foundation public struct RemoteConfigModel { let messages: [RemoteMessageModel] - let rules: [Int: [MatchingAttribute]] + let rules: [RemoteConfigRule] +} + +public struct RemoteConfigRule { + let id: Int + let targetPercentile: RemoteConfigTargetPercentile? + let attributes: [MatchingAttribute] +} + +public struct RemoteConfigTargetPercentile { + let before: Float? } diff --git a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift index 88e251d28..0c8fce51b 100644 --- a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift +++ b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift @@ -60,16 +60,16 @@ public struct RemoteMessagingConfigMatcher { return nil } - func evaluateMatchingRules(_ matchingRules: [Int], fromRules rules: [Int: [MatchingAttribute]]) -> EvaluationResult { + func evaluateMatchingRules(_ matchingRules: [Int], fromRules rules: [RemoteConfigRule]) -> EvaluationResult { var result: EvaluationResult = .match for rule in matchingRules { - guard let matchingAttributes = rules[rule] else { + guard let matchingRule = rules.first(where: { $0.id == rule }) else { return .nextMessage } result = .match - for attribute in matchingAttributes { + for attribute in matchingRule.attributes { result = evaluateAttribute(matchingAttribute: attribute) if result == .fail || result == .nextMessage { os_log("First failing matching attribute %s", log: .remoteMessaging, type: .debug, String(describing: attribute)) @@ -84,16 +84,16 @@ public struct RemoteMessagingConfigMatcher { return result } - func evaluateExclusionRules(_ exclusionRules: [Int], fromRules rules: [Int: [MatchingAttribute]]) -> EvaluationResult { + func evaluateExclusionRules(_ exclusionRules: [Int], fromRules rules: [RemoteConfigRule]) -> EvaluationResult { var result: EvaluationResult = .fail for rule in exclusionRules { - guard let attributes = rules[rule] else { + guard let matchingRule = rules.first(where: { $0.id == rule }) else { return .nextMessage } result = .fail - for attribute in attributes { + for attribute in matchingRule.attributes { result = evaluateAttribute(matchingAttribute: attribute) if result == .fail || result == .nextMessage { os_log("First failing exclusion attribute %s", log: .remoteMessaging, type: .debug, String(describing: attribute)) From 99ccefdc4e2e251b2c65a1c7cd3ebde8cdc5d732 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Fri, 3 May 2024 18:36:57 -0700 Subject: [PATCH 03/10] Update tests. --- .../JsonToRemoteConfigModelMapperTests.swift | 46 ++--- .../RemoteMessagingConfigMatcherTests.swift | 168 ++++++++++++------ 2 files changed, 135 insertions(+), 79 deletions(-) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift index 486961eef..50c467cb6 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift @@ -104,41 +104,41 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { let config = try decodeAndMapJson(fileName: "Resources/remote-messaging-config.json") XCTAssertTrue(config.rules.count == 5) - let rule5 = config.rules.filter { $0.key == 5 }.first + let rule5 = config.rules.filter { $0.id == 5 }.first XCTAssertNotNil(rule5) - XCTAssertTrue(rule5?.value.count == 16) - var attribs = rule5?.value.filter { $0 is LocaleMatchingAttribute } + XCTAssertTrue(rule5?.attributes.count == 16) + var attribs = rule5?.attributes.filter { $0 is LocaleMatchingAttribute } XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? LocaleMatchingAttribute, LocaleMatchingAttribute(value: ["en-US", "en-GB"], fallback: true)) - let rule6 = config.rules.filter { $0.key == 6 }.first + let rule6 = config.rules.filter { $0.id == 6 }.first XCTAssertNotNil(rule6) - XCTAssertTrue(rule6?.value.count == 1) - attribs = rule6?.value.filter { $0 is LocaleMatchingAttribute } + XCTAssertTrue(rule6?.attributes.count == 1) + attribs = rule6?.attributes.filter { $0 is LocaleMatchingAttribute } XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? LocaleMatchingAttribute, LocaleMatchingAttribute(value: ["en-GB"], fallback: nil)) - let rule7 = config.rules.filter { $0.key == 7 }.first + let rule7 = config.rules.filter { $0.id == 7 }.first XCTAssertNotNil(rule7) - XCTAssertTrue(rule7?.value.count == 1) - attribs = rule7?.value.filter { $0 is WidgetAddedMatchingAttribute } + XCTAssertTrue(rule7?.attributes.count == 1) + attribs = rule7?.attributes.filter { $0 is WidgetAddedMatchingAttribute } XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? WidgetAddedMatchingAttribute, WidgetAddedMatchingAttribute(value: false, fallback: nil)) - let rule8 = config.rules.filter { $0.key == 8 }.first + let rule8 = config.rules.filter { $0.id == 8 }.first XCTAssertNotNil(rule8) - XCTAssertTrue(rule8?.value.count == 2) - attribs = rule8?.value.filter { $0 is DaysSinceNetPEnabledMatchingAttribute } + XCTAssertTrue(rule8?.attributes.count == 2) + attribs = rule8?.attributes.filter { $0 is DaysSinceNetPEnabledMatchingAttribute } XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? DaysSinceNetPEnabledMatchingAttribute, DaysSinceNetPEnabledMatchingAttribute(min: 5, fallback: nil)) - attribs = rule8?.value.filter { $0 is IsNetPWaitlistUserMatchingAttribute } + attribs = rule8?.attributes.filter { $0 is IsNetPWaitlistUserMatchingAttribute } XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? IsNetPWaitlistUserMatchingAttribute, IsNetPWaitlistUserMatchingAttribute(value: true, fallback: nil)) - let rule9 = config.rules.filter { $0.key == 8 }.first + let rule9 = config.rules.filter { $0.id == 8 }.first XCTAssertNotNil(rule9) - XCTAssertTrue(rule9?.value.count == 2) + XCTAssertTrue(rule9?.attributes.count == 2) } func testWhenJsonMessagesHaveUnknownTypesThenMessagesNotMappedIntoConfig() throws { @@ -151,15 +151,15 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { let config = try decodeAndMapJson(fileName: "Resources/remote-messaging-config-unsupported-items.json") XCTAssertTrue(config.rules.count == 2) - let rule6 = config.rules.filter { $0.key == 6 }.first + let rule6 = config.rules.filter { $0.id == 6 }.first XCTAssertNotNil(rule6) - var attribs = rule6?.value.filter { $0 is UnknownMatchingAttribute } + var attribs = rule6?.attributes.filter { $0 is UnknownMatchingAttribute } XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? UnknownMatchingAttribute, UnknownMatchingAttribute(fallback: true)) - let rule7 = config.rules.filter { $0.key == 7 }.first + let rule7 = config.rules.filter { $0.id == 7 }.first XCTAssertNotNil(rule7) - attribs = rule7?.value.filter { $0 is WidgetAddedMatchingAttribute } + attribs = rule7?.attributes.filter { $0 is WidgetAddedMatchingAttribute } XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? WidgetAddedMatchingAttribute, WidgetAddedMatchingAttribute(value: true, fallback: nil)) } @@ -171,11 +171,11 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { let config = JsonToRemoteConfigModelMapper.mapJson(remoteMessagingConfig: remoteMessagingConfig) XCTAssertTrue(config.rules.count == 2) - let rule6 = config.rules.filter { $0.key == 6 }.first + let rule6 = config.rules.filter { $0.id == 6 }.first XCTAssertNotNil(rule6) - XCTAssertEqual(rule6?.value.filter { $0 is LocaleMatchingAttribute }.count, 1) - XCTAssertEqual(rule6?.value.filter { $0 is OSMatchingAttribute }.count, 1) - XCTAssertEqual(rule6?.value.filter { $0 is UnknownMatchingAttribute }.count, 1) + XCTAssertEqual(rule6?.attributes.filter { $0 is LocaleMatchingAttribute }.count, 1) + XCTAssertEqual(rule6?.attributes.filter { $0 is OSMatchingAttribute }.count, 1) + XCTAssertEqual(rule6?.attributes.filter { $0 is UnknownMatchingAttribute }.count, 1) } func decodeAndMapJson(fileName: String) throws -> RemoteConfigModel { diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift index d56932647..73ea8fcb7 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift @@ -56,7 +56,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { } func testWhenEmptyConfigThenReturnNull() throws { - let emptyConfig = RemoteConfigModel(messages: [], rules: [:]) + let emptyConfig = RemoteConfigModel(messages: [], rules: []) XCTAssertNil(matcher.evaluate(remoteConfig: emptyConfig)) } @@ -64,41 +64,54 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { func testWhenNoMatchingRulesThenReturnFirstMessage() throws { let noRulesRemoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: []), mediumMessage(matchingRules: [], exclusionRules: [])], - rules: [:]) + rules: []) XCTAssertEqual(matcher.evaluate(remoteConfig: noRulesRemoteConfig), mediumMessage(matchingRules: [], exclusionRules: [])) } func testWhenNotExistingRuleThenReturnSkipMessage() throws { let noRulesRemoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: []), mediumMessage(matchingRules: [], exclusionRules: [])], - rules: [:]) + rules: []) XCTAssertEqual(matcher.evaluate(remoteConfig: noRulesRemoteConfig), mediumMessage(matchingRules: [], exclusionRules: [])) } func testWhenNoMessagesThenReturnNull() throws { let os = ProcessInfo().operatingSystemVersion - let noRulesRemoteConfig = RemoteConfigModel(messages: [], - rules: [1: [OSMatchingAttribute(min: "0.0", max: String(os.majorVersion + 1), fallback: nil)]]) + let noRulesRemoteConfig = RemoteConfigModel(messages: [], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [ + OSMatchingAttribute(min: "0.0", max: String(os.majorVersion + 1), fallback: nil) + ]) + ]) XCTAssertNil(matcher.evaluate(remoteConfig: noRulesRemoteConfig)) } func testWhenDeviceDoesNotMatchMessageRulesThenReturnNull() throws { let os = ProcessInfo().operatingSystemVersion - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: []), - mediumMessage(matchingRules: [1], exclusionRules: [])], - rules: [1: [OSMatchingAttribute(min: "0.0", max: String(os.majorVersion - 1), fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: []), + mediumMessage(matchingRules: [1], exclusionRules: []) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [ + OSMatchingAttribute(min: "0.0", max: String(os.majorVersion - 1), fallback: nil) + ]) + ]) XCTAssertNil(matcher.evaluate(remoteConfig: remoteConfig)) } func testWhenNoMatchingRulesThenReturnFirstNonExcludedMessage() { - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [], exclusionRules: [2]), - mediumMessage(matchingRules: [], exclusionRules: [3])], - rules: [1: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)], - 2: [LocaleMatchingAttribute(value: [LocaleMatchingAttribute.localeIdentifierAsJsonFormat(Locale.current.identifier)], fallback: nil)], - 3: [EmailEnabledMatchingAttribute(value: false, fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [], exclusionRules: [2]), + mediumMessage(matchingRules: [], exclusionRules: [3]) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]), + RemoteConfigRule(id: 2, targetPercentile: nil, attributes: [ + LocaleMatchingAttribute(value: [LocaleMatchingAttribute.localeIdentifierAsJsonFormat(Locale.current.identifier)], fallback: nil) + ]), + RemoteConfigRule(id: 3, targetPercentile: nil, attributes: [EmailEnabledMatchingAttribute(value: false, fallback: nil)]) + ]) XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(matchingRules: [], exclusionRules: [3])) } @@ -117,58 +130,82 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { daysSinceNetPEnabled: -1), dismissedMessageIds: []) - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: [2])], - rules: [1: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)], - 2: [LocaleMatchingAttribute(value: ["en-US"], fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: [2]) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]), + RemoteConfigRule(id: 2, targetPercentile: nil, attributes: [LocaleMatchingAttribute(value: ["en-US"], fallback: nil)]) + ]) XCTAssertNil(matcher.evaluate(remoteConfig: remoteConfig)) } func testWhenMatchingMessageShouldBeExcludedByOneOfMultipleRulesThenReturnNull() { - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: [4]), - mediumMessage(matchingRules: [1], exclusionRules: [2, 3]), - mediumMessage(matchingRules: [1], exclusionRules: [2, 3, 4]), - mediumMessage(matchingRules: [1], exclusionRules: [2, 4]), - mediumMessage(matchingRules: [1], exclusionRules: [4])], - rules: [1: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)], - 2: [EmailEnabledMatchingAttribute(value: true, fallback: nil), BookmarksMatchingAttribute(max: 10, fallback: nil)], - 3: [EmailEnabledMatchingAttribute(value: true, fallback: nil), BookmarksMatchingAttribute(max: 10, fallback: nil)], - 4: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)], - 5: [EmailEnabledMatchingAttribute(value: true, fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: [4]), + mediumMessage(matchingRules: [1], exclusionRules: [2, 3]), + mediumMessage(matchingRules: [1], exclusionRules: [2, 3, 4]), + mediumMessage(matchingRules: [1], exclusionRules: [2, 4]), + mediumMessage(matchingRules: [1], exclusionRules: [4]) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]), + RemoteConfigRule(id: 2, targetPercentile: nil, attributes: [ + EmailEnabledMatchingAttribute(value: true, fallback: nil), BookmarksMatchingAttribute(max: 10, fallback: nil) + ]), + RemoteConfigRule(id: 3, targetPercentile: nil, attributes: [ + EmailEnabledMatchingAttribute(value: true, fallback: nil), BookmarksMatchingAttribute(max: 10, fallback: nil) + ]), + RemoteConfigRule(id: 4, targetPercentile: nil, attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]), + RemoteConfigRule(id: 5, targetPercentile: nil, attributes: [EmailEnabledMatchingAttribute(value: true, fallback: nil)]) + ]) XCTAssertNil(matcher.evaluate(remoteConfig: remoteConfig)) } func testWhenMultipleMatchingMessagesAndSomeExcludedThenReturnFirstNonExcludedMatch() { - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: [2]), - mediumMessage(matchingRules: [1], exclusionRules: [2]), - mediumMessage(matchingRules: [1], exclusionRules: [])], - rules: [1: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)], - 2: [LocaleMatchingAttribute(value: [LocaleMatchingAttribute.localeIdentifierAsJsonFormat(Locale.current.identifier)], fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: [2]), + mediumMessage(matchingRules: [1], exclusionRules: [2]), + mediumMessage(matchingRules: [1], exclusionRules: []) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]), + RemoteConfigRule(id: 2, targetPercentile: nil, attributes: [ + LocaleMatchingAttribute(value: [LocaleMatchingAttribute.localeIdentifierAsJsonFormat(Locale.current.identifier)], fallback: nil) + ]) + ]) XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(matchingRules: [1], exclusionRules: [])) } func testWhenMessageMatchesAndExclusionRuleFailsThenReturnMessage() { - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: [2])], - rules: [1: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)], - 2: [EmailEnabledMatchingAttribute(value: false, fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: [2]) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]), + RemoteConfigRule(id: 2, targetPercentile: nil, attributes: [EmailEnabledMatchingAttribute(value: false, fallback: nil)]) + ]) XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(matchingRules: [1], exclusionRules: [2])) } func testWhenDeviceMatchesMessageRulesThenReturnFirstMatch() { - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: [])], - rules: [1: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: []) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]) + ]) XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(matchingRules: [1], exclusionRules: [])) } func testWhenDeviceMatchesMessageRulesForOneOfMultipleMessagesThenReturnMatch() { - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [2], exclusionRules: []), - mediumMessage(matchingRules: [1, 2], exclusionRules: [])], - rules: [1: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)], - 2: [EmailEnabledMatchingAttribute(value: false, fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [2], exclusionRules: []), + mediumMessage(matchingRules: [1, 2], exclusionRules: []) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]), + RemoteConfigRule(id: 2, targetPercentile: nil, attributes: [EmailEnabledMatchingAttribute(value: false, fallback: nil)]) + ]) XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(matchingRules: [1, 2], exclusionRules: [])) } @@ -186,17 +223,23 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { daysSinceNetPEnabled: -1), dismissedMessageIds: ["1"]) - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: []), - mediumMessage(id: "2", matchingRules: [1], exclusionRules: [])], - rules: [1: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: []), + mediumMessage(id: "2", matchingRules: [1], exclusionRules: []) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)]) + ]) XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(id: "2", matchingRules: [1], exclusionRules: [])) } func testWhenDeviceMatchesAnyRuleThenReturnFirstMatch() { - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1, 2], exclusionRules: [])], - rules: [1: [LocaleMatchingAttribute(value: [Locale.current.identifier], fallback: nil)], - 2: [OSMatchingAttribute(min: "0", max: "100", fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1, 2], exclusionRules: []) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [LocaleMatchingAttribute(value: [Locale.current.identifier], fallback: nil)]), + RemoteConfigRule(id: 2, targetPercentile: nil, attributes: [OSMatchingAttribute(min: "0", max: "100", fallback: nil)]) + ]) XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(matchingRules: [1, 2], exclusionRules: [])) } @@ -216,26 +259,39 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { daysSinceNetPEnabled: -1), dismissedMessageIds: []) - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1, 2], exclusionRules: []), - mediumMessage(matchingRules: [1, 2], exclusionRules: [])], - rules: [1: [OSMatchingAttribute(min: "0.0", max: String(os.majorVersion - 1), fallback: nil)], - 2: [OSMatchingAttribute(min: "0.0", max: String(os.majorVersion - 1), fallback: nil)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1, 2], exclusionRules: []), + mediumMessage(matchingRules: [1, 2], exclusionRules: []) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [ + OSMatchingAttribute(min: "0.0", max: String(os.majorVersion - 1), fallback: nil) + ]), + RemoteConfigRule(id: 2, targetPercentile: nil, attributes: [ + OSMatchingAttribute(min: "0.0", max: String(os.majorVersion - 1), fallback: nil) + ]) + ]) XCTAssertNil(matcher.evaluate(remoteConfig: remoteConfig)) } func testWhenUnknownRuleFailsThenReturnNull() { - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: []), - mediumMessage(matchingRules: [1], exclusionRules: [])], - rules: [1: [UnknownMatchingAttribute(fallback: false)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: []), + mediumMessage(matchingRules: [1], exclusionRules: []) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [UnknownMatchingAttribute(fallback: false)]) + ]) XCTAssertNil(matcher.evaluate(remoteConfig: remoteConfig)) } func testWhenUnknownRuleMatchesThenReturnFirstMatch() { - let remoteConfig = RemoteConfigModel(messages: [mediumMessage(matchingRules: [1], exclusionRules: []), - mediumMessage(id: "2", matchingRules: [1], exclusionRules: [])], - rules: [1: [UnknownMatchingAttribute(fallback: true)]]) + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: []), + mediumMessage(id: "2", matchingRules: [1], exclusionRules: []) + ], rules: [ + RemoteConfigRule(id: 1, targetPercentile: nil, attributes: [UnknownMatchingAttribute(fallback: true)]) + ]) XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(matchingRules: [1], exclusionRules: [])) } From 2e7f2d882cced8515c6043646e28674b0ee20285 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Fri, 3 May 2024 18:39:56 -0700 Subject: [PATCH 04/10] Test target percentile. --- .../Mappers/JsonToRemoteConfigModelMapperTests.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift index 50c467cb6..4cf9b2bc7 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mappers/JsonToRemoteConfigModelMapperTests.swift @@ -106,6 +106,7 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { let rule5 = config.rules.filter { $0.id == 5 }.first XCTAssertNotNil(rule5) + XCTAssertNil(rule5?.targetPercentile) XCTAssertTrue(rule5?.attributes.count == 16) var attribs = rule5?.attributes.filter { $0 is LocaleMatchingAttribute } XCTAssertEqual(attribs?.count, 1) @@ -113,6 +114,7 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { let rule6 = config.rules.filter { $0.id == 6 }.first XCTAssertNotNil(rule6) + XCTAssertNil(rule6?.targetPercentile) XCTAssertTrue(rule6?.attributes.count == 1) attribs = rule6?.attributes.filter { $0 is LocaleMatchingAttribute } XCTAssertEqual(attribs?.count, 1) @@ -120,6 +122,7 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { let rule7 = config.rules.filter { $0.id == 7 }.first XCTAssertNotNil(rule7) + XCTAssertNil(rule7?.targetPercentile) XCTAssertTrue(rule7?.attributes.count == 1) attribs = rule7?.attributes.filter { $0 is WidgetAddedMatchingAttribute } XCTAssertEqual(attribs?.count, 1) @@ -127,6 +130,7 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { let rule8 = config.rules.filter { $0.id == 8 }.first XCTAssertNotNil(rule8) + XCTAssertNil(rule8?.targetPercentile) XCTAssertTrue(rule8?.attributes.count == 2) attribs = rule8?.attributes.filter { $0 is DaysSinceNetPEnabledMatchingAttribute } XCTAssertEqual(attribs?.count, 1) @@ -136,9 +140,11 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { XCTAssertEqual(attribs?.count, 1) XCTAssertEqual(attribs?.first as? IsNetPWaitlistUserMatchingAttribute, IsNetPWaitlistUserMatchingAttribute(value: true, fallback: nil)) - let rule9 = config.rules.filter { $0.id == 8 }.first + let rule9 = config.rules.filter { $0.id == 9 }.first XCTAssertNotNil(rule9) - XCTAssertTrue(rule9?.attributes.count == 2) + XCTAssertNotNil(rule9?.targetPercentile) + XCTAssertTrue(rule9?.attributes.count == 1) + XCTAssertEqual(rule9?.targetPercentile?.before, 0.9) } func testWhenJsonMessagesHaveUnknownTypesThenMessagesNotMappedIntoConfig() throws { From 3d1e2f5ab1758b6483eba8cfed227711352cb0f2 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 5 May 2024 10:15:04 -0700 Subject: [PATCH 05/10] Check the message percentile when evaluating rules. --- .../RemoteMessagingConfigMatcher.swift | 17 ++++++++- .../RemoteMessagingPercentileStoring.swift | 37 +++++++++++++++++++ .../MockRemoteMessagePercentileStore.swift | 37 +++++++++++++++++++ .../RemoteMessagingConfigMatcherTests.swift | 4 ++ .../RemoteMessagingConfigProcessorTests.swift | 2 + 5 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift create mode 100644 Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessagePercentileStore.swift diff --git a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift index 0c8fce51b..8e8856faa 100644 --- a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift +++ b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift @@ -24,6 +24,7 @@ public struct RemoteMessagingConfigMatcher { private let appAttributeMatcher: AppAttributeMatcher private let deviceAttributeMatcher: DeviceAttributeMatcher private let userAttributeMatcher: UserAttributeMatcher + private let percentileStore: RemoteMessagingPercentileStoring private let dismissedMessageIds: [String] private let matchers: [AttributeMatcher] @@ -31,10 +32,12 @@ public struct RemoteMessagingConfigMatcher { public init(appAttributeMatcher: AppAttributeMatcher, deviceAttributeMatcher: DeviceAttributeMatcher = DeviceAttributeMatcher(), userAttributeMatcher: UserAttributeMatcher, + percentileStore: RemoteMessagingPercentileStoring, dismissedMessageIds: [String]) { self.appAttributeMatcher = appAttributeMatcher self.deviceAttributeMatcher = deviceAttributeMatcher self.userAttributeMatcher = userAttributeMatcher + self.percentileStore = percentileStore self.dismissedMessageIds = dismissedMessageIds matchers = [appAttributeMatcher, deviceAttributeMatcher, userAttributeMatcher] @@ -49,7 +52,7 @@ public struct RemoteMessagingConfigMatcher { return message } - let matchingResult = evaluateMatchingRules(message.matchingRules, fromRules: rules) + let matchingResult = evaluateMatchingRules(message.matchingRules, messageID: message.id, fromRules: rules) let exclusionResult = evaluateExclusionRules(message.exclusionRules, fromRules: rules) if matchingResult == .match && exclusionResult == .fail { @@ -60,13 +63,23 @@ public struct RemoteMessagingConfigMatcher { return nil } - func evaluateMatchingRules(_ matchingRules: [Int], fromRules rules: [RemoteConfigRule]) -> EvaluationResult { + func evaluateMatchingRules(_ matchingRules: [Int], messageID: String, fromRules rules: [RemoteConfigRule]) -> EvaluationResult { var result: EvaluationResult = .match for rule in matchingRules { guard let matchingRule = rules.first(where: { $0.id == rule }) else { return .nextMessage } + + if let percentile = matchingRule.targetPercentile, let messagePercentile = percentile.before { + let userPercentile = percentileStore.percentile(forMessageId: messageID) + + if userPercentile > messagePercentile { + os_log("Percentile check failed for message with ID %s", log: .remoteMessaging, type: .debug, messageID) + return .fail + } + } + result = .match for attribute in matchingRule.attributes { diff --git a/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift b/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift new file mode 100644 index 000000000..bfd3de722 --- /dev/null +++ b/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift @@ -0,0 +1,37 @@ +// +// RemoteMessagingPercentileStoring.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +public protocol RemoteMessagingPercentileStoring { + func percentile(forMessageId: String) -> Float +} + +public class RemoteMessagingPercentileUserDefaultsStore: RemoteMessagingPercentileStoring { + + private let userDefaults: UserDefaults + + init(userDefaults: UserDefaults) { + self.userDefaults = userDefaults + } + + public func percentile(forMessageId: String) -> Float { + return 0.95 + } + +} diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessagePercentileStore.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessagePercentileStore.swift new file mode 100644 index 000000000..781787ed8 --- /dev/null +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessagePercentileStore.swift @@ -0,0 +1,37 @@ +// +// MockRemoteMessagePercentileStore.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import RemoteMessaging + +class MockRemoteMessagePercentileStore: RemoteMessagingPercentileStoring { + + var percentileStorage: [String: Float] = [:] + var defaultPercentage: Float = 0 + + func percentile(forMessageId messageID: String) -> Float { + if let percentile = percentileStorage[messageID] { + return percentile + } + + percentileStorage[messageID] = defaultPercentage + return defaultPercentage + } + +} diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift index 73ea8fcb7..df142483a 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift @@ -45,6 +45,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isWidgetInstalled: false, isNetPWaitlistUser: false, daysSinceNetPEnabled: -1), + percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: [] ) } @@ -128,6 +129,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isWidgetInstalled: false, isNetPWaitlistUser: false, daysSinceNetPEnabled: -1), + percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: []) let remoteConfig = RemoteConfigModel(messages: [ @@ -221,6 +223,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isWidgetInstalled: false, isNetPWaitlistUser: false, daysSinceNetPEnabled: -1), + percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: ["1"]) let remoteConfig = RemoteConfigModel(messages: [ @@ -257,6 +260,7 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { isWidgetInstalled: false, isNetPWaitlistUser: false, daysSinceNetPEnabled: -1), + percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: []) let remoteConfig = RemoteConfigModel(messages: [ diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift index 24d378591..c42f273c0 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigProcessorTests.swift @@ -38,6 +38,7 @@ class RemoteMessagingConfigProcessorTests: XCTestCase { isWidgetInstalled: false, isNetPWaitlistUser: false, daysSinceNetPEnabled: -1), + percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: [] ) @@ -66,6 +67,7 @@ class RemoteMessagingConfigProcessorTests: XCTestCase { isWidgetInstalled: false, isNetPWaitlistUser: false, daysSinceNetPEnabled: -1), + percentileStore: MockRemoteMessagePercentileStore(), dismissedMessageIds: []) let processor = RemoteMessagingConfigProcessor(remoteMessagingConfigMatcher: remoteMessagingConfigMatcher) From 3e76183e2cdac2085d0e04760a42c6f71e7f9f91 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 5 May 2024 10:36:13 -0700 Subject: [PATCH 06/10] Add tests for percentile matching. --- .../RemoteMessagingConfigMatcher.swift | 16 ++- .../RemoteMessagingConfigMatcherTests.swift | 124 ++++++++++++++++++ 2 files changed, 137 insertions(+), 3 deletions(-) diff --git a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift index 8e8856faa..23d4c9471 100644 --- a/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift +++ b/Sources/RemoteMessaging/RemoteMessagingConfigMatcher.swift @@ -53,7 +53,7 @@ public struct RemoteMessagingConfigMatcher { } let matchingResult = evaluateMatchingRules(message.matchingRules, messageID: message.id, fromRules: rules) - let exclusionResult = evaluateExclusionRules(message.exclusionRules, fromRules: rules) + let exclusionResult = evaluateExclusionRules(message.exclusionRules, messageID: message.id, fromRules: rules) if matchingResult == .match && exclusionResult == .fail { return message @@ -75,7 +75,7 @@ public struct RemoteMessagingConfigMatcher { let userPercentile = percentileStore.percentile(forMessageId: messageID) if userPercentile > messagePercentile { - os_log("Percentile check failed for message with ID %s", log: .remoteMessaging, type: .debug, messageID) + os_log("Matching rule percentile check failed for message with ID %s", log: .remoteMessaging, type: .debug, messageID) return .fail } } @@ -97,13 +97,23 @@ public struct RemoteMessagingConfigMatcher { return result } - func evaluateExclusionRules(_ exclusionRules: [Int], fromRules rules: [RemoteConfigRule]) -> EvaluationResult { + func evaluateExclusionRules(_ exclusionRules: [Int], messageID: String, fromRules rules: [RemoteConfigRule]) -> EvaluationResult { var result: EvaluationResult = .fail for rule in exclusionRules { guard let matchingRule = rules.first(where: { $0.id == rule }) else { return .nextMessage } + + if let percentile = matchingRule.targetPercentile, let messagePercentile = percentile.before { + let userPercentile = percentileStore.percentile(forMessageId: messageID) + + if userPercentile > messagePercentile { + os_log("Exclusion rule percentile check failed for message with ID %s", log: .remoteMessaging, type: .debug, messageID) + return .fail + } + } + result = .fail for attribute in matchingRule.attributes { diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift index df142483a..8d904ed91 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingConfigMatcherTests.swift @@ -278,6 +278,130 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { XCTAssertNil(matcher.evaluate(remoteConfig: remoteConfig)) } + func testWhenDeviceMatchesMessageRules_AndIsPartOfPercentile_ThenReturnMatch() { + let percentileStore = MockRemoteMessagePercentileStore() + percentileStore.defaultPercentage = 0.1 + + matcher = RemoteMessagingConfigMatcher( + appAttributeMatcher: AppAttributeMatcher(statisticsStore: MockStatisticsStore(), variantManager: MockVariantManager()), + deviceAttributeMatcher: DeviceAttributeMatcher(osVersion: AppVersion.shared.osVersion, locale: "en-US"), + userAttributeMatcher: UserAttributeMatcher(statisticsStore: MockStatisticsStore(), + variantManager: MockVariantManager(), + bookmarksCount: 0, + favoritesCount: 0, + appTheme: "light", + isWidgetInstalled: false, + isNetPWaitlistUser: false, + daysSinceNetPEnabled: -1), + percentileStore: percentileStore, + dismissedMessageIds: []) + + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: []) + ], rules: [ + RemoteConfigRule( + id: 1, + targetPercentile: RemoteConfigTargetPercentile(before: 0.3), + attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)] + ) + ]) + + XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(matchingRules: [1], exclusionRules: [])) + } + + func testWhenDeviceMatchesMessageRules_AndIsNotPartOfPercentile_ThenReturnNull() { + let percentileStore = MockRemoteMessagePercentileStore() + percentileStore.defaultPercentage = 0.5 + + matcher = RemoteMessagingConfigMatcher( + appAttributeMatcher: AppAttributeMatcher(statisticsStore: MockStatisticsStore(), variantManager: MockVariantManager()), + deviceAttributeMatcher: DeviceAttributeMatcher(osVersion: AppVersion.shared.osVersion, locale: "en-US"), + userAttributeMatcher: UserAttributeMatcher(statisticsStore: MockStatisticsStore(), + variantManager: MockVariantManager(), + bookmarksCount: 0, + favoritesCount: 0, + appTheme: "light", + isWidgetInstalled: false, + isNetPWaitlistUser: false, + daysSinceNetPEnabled: -1), + percentileStore: percentileStore, + dismissedMessageIds: []) + + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [1], exclusionRules: []) + ], rules: [ + RemoteConfigRule( + id: 1, + targetPercentile: RemoteConfigTargetPercentile(before: 0.3), + attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)] + ) + ]) + + XCTAssertNil(matcher.evaluate(remoteConfig: remoteConfig)) + } + + func testWhenDeviceExcludesMessageRules_AndIsPartOfPercentile_ThenReturnNull() { + let percentileStore = MockRemoteMessagePercentileStore() + percentileStore.defaultPercentage = 0.3 + + matcher = RemoteMessagingConfigMatcher( + appAttributeMatcher: AppAttributeMatcher(statisticsStore: MockStatisticsStore(), variantManager: MockVariantManager()), + deviceAttributeMatcher: DeviceAttributeMatcher(osVersion: AppVersion.shared.osVersion, locale: "en-US"), + userAttributeMatcher: UserAttributeMatcher(statisticsStore: MockStatisticsStore(), + variantManager: MockVariantManager(), + bookmarksCount: 0, + favoritesCount: 0, + appTheme: "light", + isWidgetInstalled: false, + isNetPWaitlistUser: false, + daysSinceNetPEnabled: -1), + percentileStore: percentileStore, + dismissedMessageIds: []) + + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [], exclusionRules: [1]) + ], rules: [ + RemoteConfigRule( + id: 1, + targetPercentile: RemoteConfigTargetPercentile(before: 0.5), + attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)] + ) + ]) + + XCTAssertNil(matcher.evaluate(remoteConfig: remoteConfig)) + } + + func testWhenDeviceExcludesMessageRules_AndIsNotPartOfPercentile_ThenReturnMatch() { + let percentileStore = MockRemoteMessagePercentileStore() + percentileStore.defaultPercentage = 0.6 + + matcher = RemoteMessagingConfigMatcher( + appAttributeMatcher: AppAttributeMatcher(statisticsStore: MockStatisticsStore(), variantManager: MockVariantManager()), + deviceAttributeMatcher: DeviceAttributeMatcher(osVersion: AppVersion.shared.osVersion, locale: "en-US"), + userAttributeMatcher: UserAttributeMatcher(statisticsStore: MockStatisticsStore(), + variantManager: MockVariantManager(), + bookmarksCount: 0, + favoritesCount: 0, + appTheme: "light", + isWidgetInstalled: false, + isNetPWaitlistUser: false, + daysSinceNetPEnabled: -1), + percentileStore: percentileStore, + dismissedMessageIds: []) + + let remoteConfig = RemoteConfigModel(messages: [ + mediumMessage(matchingRules: [], exclusionRules: [1]) + ], rules: [ + RemoteConfigRule( + id: 1, + targetPercentile: RemoteConfigTargetPercentile(before: 0.5), + attributes: [OSMatchingAttribute(value: AppVersion.shared.osVersion, fallback: nil)] + ) + ]) + + XCTAssertEqual(matcher.evaluate(remoteConfig: remoteConfig), mediumMessage(matchingRules: [], exclusionRules: [1])) + } + func testWhenUnknownRuleFailsThenReturnNull() { let remoteConfig = RemoteConfigModel(messages: [ mediumMessage(matchingRules: [1], exclusionRules: []), From 4b336b137a78d283d34b615ef11e8dc8e21815e6 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 5 May 2024 19:52:14 -0700 Subject: [PATCH 07/10] Fix SwiftLint. --- .../RemoteMessaging/Mocks/MockRemoteMessagePercentileStore.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessagePercentileStore.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessagePercentileStore.swift index 781787ed8..f484d04e0 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessagePercentileStore.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/Mocks/MockRemoteMessagePercentileStore.swift @@ -1,6 +1,5 @@ // // MockRemoteMessagePercentileStore.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. // From 2bab68ab65f77f8695f82f72fb1a818f8765548c Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 5 May 2024 20:07:23 -0700 Subject: [PATCH 08/10] Add tests for user defaults store. --- .../RemoteMessagingPercentileStoring.swift | 18 ++++- ...gingPercentileUserDefaultsStoreTests.swift | 65 +++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift diff --git a/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift b/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift index bfd3de722..1bb2f092c 100644 --- a/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift +++ b/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift @@ -24,14 +24,28 @@ public protocol RemoteMessagingPercentileStoring { public class RemoteMessagingPercentileUserDefaultsStore: RemoteMessagingPercentileStoring { + enum Constants { + static let remoteMessagingPercentileMapping = "com.duckduckgo.app.remoteMessagingPercentileMapping" + } + private let userDefaults: UserDefaults init(userDefaults: UserDefaults) { self.userDefaults = userDefaults } - public func percentile(forMessageId: String) -> Float { - return 0.95 + public func percentile(forMessageId messageID: String) -> Float { + var percentileMapping = (userDefaults.dictionary(forKey: Constants.remoteMessagingPercentileMapping) as? [String: Float]) ?? [:] + + if let percentile = percentileMapping[messageID] { + return percentile + } else { + let newPercentile = Float.random(in: 0...1) + percentileMapping[messageID] = newPercentile + userDefaults.set(percentileMapping, forKey: Constants.remoteMessagingPercentileMapping) + + return newPercentile + } } } diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift new file mode 100644 index 000000000..9f62573af --- /dev/null +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift @@ -0,0 +1,65 @@ +// +// RemoteMessagingPercentileUserDefaultsStoreTests.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +@testable import BrowserServicesKit +@testable import RemoteMessaging + +class RemoteMessagingPercentileUserDefaultsStoreTests: XCTestCase { + + private var userDefaults: UserDefaults! + + override func setUp() { + super.setUp() + userDefaults = UserDefaults(suiteName: #file) + userDefaults.removePersistentDomain(forName: #file) + } + + func testWhenFetchingPercentileForFirstTime_ThenPercentileIsCreatedAndStored() { + let store = RemoteMessagingPercentileUserDefaultsStore(userDefaults: userDefaults) + let percentile = store.percentile(forMessageId: "message-1") + + XCTAssert(percentile >= 0.0) + XCTAssert(percentile <= 1.0) + } + + func testWhenFetchingPercentileMultipleTimes_ThenAllPercentileFetchesReturnSameValue() { + let store = RemoteMessagingPercentileUserDefaultsStore(userDefaults: userDefaults) + let percentile1 = store.percentile(forMessageId: "message-1") + let percentile2 = store.percentile(forMessageId: "message-1") + let percentile3 = store.percentile(forMessageId: "message-1") + + XCTAssertEqual(percentile1, percentile2) + XCTAssertEqual(percentile2, percentile3) + } + + func testWhenFetchingPercentileForMultipleMessages_ThenEachMessageHasIndependentPercentile() { + let store = RemoteMessagingPercentileUserDefaultsStore(userDefaults: userDefaults) + let percentile1 = store.percentile(forMessageId: "message-1") + let percentile2 = store.percentile(forMessageId: "message-2") + let percentile3 = store.percentile(forMessageId: "message-3") + + let percentileDictionary = userDefaults.dictionary( + forKey: RemoteMessagingPercentileUserDefaultsStore.Constants.remoteMessagingPercentileMapping + ) + + XCTAssertEqual(percentileDictionary?.count, 3) + } + +} From d218ef29678f480828072b6d2bdb695902b34020 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 5 May 2024 20:12:50 -0700 Subject: [PATCH 09/10] Fix compiler warnings and private init. --- .../RemoteMessaging/RemoteMessagingPercentileStoring.swift | 2 +- .../RemoteMessagingPercentileUserDefaultsStoreTests.swift | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift b/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift index 1bb2f092c..531ebab8b 100644 --- a/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift +++ b/Sources/RemoteMessaging/RemoteMessagingPercentileStoring.swift @@ -30,7 +30,7 @@ public class RemoteMessagingPercentileUserDefaultsStore: RemoteMessagingPercenti private let userDefaults: UserDefaults - init(userDefaults: UserDefaults) { + public init(userDefaults: UserDefaults) { self.userDefaults = userDefaults } diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift index 9f62573af..6801fe72c 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift @@ -51,9 +51,9 @@ class RemoteMessagingPercentileUserDefaultsStoreTests: XCTestCase { func testWhenFetchingPercentileForMultipleMessages_ThenEachMessageHasIndependentPercentile() { let store = RemoteMessagingPercentileUserDefaultsStore(userDefaults: userDefaults) - let percentile1 = store.percentile(forMessageId: "message-1") - let percentile2 = store.percentile(forMessageId: "message-2") - let percentile3 = store.percentile(forMessageId: "message-3") + _ = store.percentile(forMessageId: "message-1") + _ = store.percentile(forMessageId: "message-2") + _ = store.percentile(forMessageId: "message-3") let percentileDictionary = userDefaults.dictionary( forKey: RemoteMessagingPercentileUserDefaultsStore.Constants.remoteMessagingPercentileMapping From de941ca1118251405869ba9927b369e43dd66da0 Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Sun, 5 May 2024 20:19:34 -0700 Subject: [PATCH 10/10] Fix SwiftLint again. --- .../RemoteMessagingPercentileUserDefaultsStoreTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift index 6801fe72c..6e993abb8 100644 --- a/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift +++ b/Tests/BrowserServicesKitTests/RemoteMessaging/RemoteMessagingPercentileUserDefaultsStoreTests.swift @@ -1,6 +1,5 @@ // // RemoteMessagingPercentileUserDefaultsStoreTests.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. //