From b681aebe225c2cbdca2522fc3c63e009541f4b18 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 17 Jul 2024 17:16:56 +0200 Subject: [PATCH 1/3] Add support for sendPixels flag --- .../BrowserServicesKit-Package.xcscheme | 16 ++++++- .../xcschemes/SubscriptionTests.xcscheme | 2 +- .../JsonToRemoteMessageModelMapper.swift | 11 +++-- .../Model/JsonRemoteMessagingConfig.swift | 1 + .../Model/RemoteMessageModel.swift | 42 ++++++++++++------- .../RemoteMessagingStore.swift | 16 ++++++- 6 files changed, 65 insertions(+), 23 deletions(-) diff --git a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme index 5d23935cc..a4a34b64d 100644 --- a/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme +++ b/.swiftpm/xcode/xcshareddata/xcschemes/BrowserServicesKit-Package.xcscheme @@ -1,7 +1,7 @@ + version = "1.8"> + + + + + version = "1.8"> Bool { return lhs.id == rhs.id diff --git a/Sources/RemoteMessaging/Model/RemoteMessageModel.swift b/Sources/RemoteMessaging/Model/RemoteMessageModel.swift index 34c6e2eca..68a6808ad 100644 --- a/Sources/RemoteMessaging/Model/RemoteMessageModel.swift +++ b/Sources/RemoteMessaging/Model/RemoteMessageModel.swift @@ -24,28 +24,40 @@ public struct RemoteMessageModel: Equatable, Codable { public var content: RemoteMessageModelType? public let matchingRules: [Int] public let exclusionRules: [Int] + public let sendPixels: Bool - public init(id: String, content: RemoteMessageModelType?, matchingRules: [Int], exclusionRules: [Int]) { + public init(id: String, content: RemoteMessageModelType?, matchingRules: [Int], exclusionRules: [Int], sendPixels: Bool) { self.id = id self.content = content self.matchingRules = matchingRules self.exclusionRules = exclusionRules + self.sendPixels = sendPixels } - public static func == (lhs: RemoteMessageModel, rhs: RemoteMessageModel) -> Bool { - if lhs.id != rhs.id { - return false - } - if lhs.content != rhs.content { - return false - } - if lhs.matchingRules != rhs.matchingRules { - return false - } - if lhs.exclusionRules != rhs.exclusionRules { - return false - } - return true + enum CodingKeys: CodingKey { + case id + case content + case matchingRules + case exclusionRules + case sendPixels + } + + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.id = try container.decode(String.self, forKey: .id) + self.content = try container.decodeIfPresent(RemoteMessageModelType.self, forKey: .content) + self.matchingRules = try container.decode([Int].self, forKey: .matchingRules) + self.exclusionRules = try container.decode([Int].self, forKey: .exclusionRules) + self.sendPixels = try container.decodeIfPresent(Bool.self, forKey: .sendPixels) ?? true + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.container(keyedBy: CodingKeys.self) + try container.encode(self.id, forKey: .id) + try container.encodeIfPresent(self.content, forKey: .content) + try container.encode(self.matchingRules, forKey: .matchingRules) + try container.encode(self.exclusionRules, forKey: .exclusionRules) + try container.encode(self.sendPixels, forKey: .sendPixels) } mutating func localizeContent(translation: RemoteMessageResponse.JsonContentTranslation) { diff --git a/Sources/RemoteMessaging/RemoteMessagingStore.swift b/Sources/RemoteMessaging/RemoteMessagingStore.swift index 83b46cb4a..a4c60cc44 100644 --- a/Sources/RemoteMessaging/RemoteMessagingStore.swift +++ b/Sources/RemoteMessaging/RemoteMessagingStore.swift @@ -226,7 +226,13 @@ extension RemoteMessagingStore { continue } - scheduledRemoteMessage = RemoteMessageModel(id: id, content: remoteMessage.content, matchingRules: [], exclusionRules: []) + scheduledRemoteMessage = RemoteMessageModel( + id: id, + content: remoteMessage.content, + matchingRules: [], + exclusionRules: [], + sendPixels: remoteMessage.sendPixels + ) break } } @@ -255,7 +261,13 @@ extension RemoteMessagingStore { continue } - remoteMessage = RemoteMessageModel(id: id, content: remoteMessageMapped.content, matchingRules: [], exclusionRules: []) + remoteMessage = RemoteMessageModel( + id: id, + content: remoteMessageMapped.content, + matchingRules: [], + exclusionRules: [], + sendPixels: remoteMessageMapped.sendPixels + ) break } } From 449b010ba5f285130d1aaccfbba9ac3c7dcfe360 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 18 Jul 2024 14:32:28 +0200 Subject: [PATCH 2/3] Replace sendPixels attribute with a per-message metrics object --- Package.swift | 1 + .../JsonToRemoteMessageModelMapper.swift | 2 +- .../Model/JsonRemoteMessagingConfig.swift | 15 ++++- .../Model/RemoteMessageModel.swift | 12 ++-- .../RemoteMessagingStore.swift | 4 +- .../JsonToRemoteConfigModelMapperTests.swift | 61 ++++++++++++++++--- .../RemoteMessagingConfigMatcherTests.swift | 7 ++- .../RemoteMessagingStoreTests.swift | 2 +- .../remote-messaging-config-metrics.json | 47 ++++++++++++++ 9 files changed, 129 insertions(+), 22 deletions(-) create mode 100644 Tests/RemoteMessagingTests/Resources/remote-messaging-config-metrics.json diff --git a/Package.swift b/Package.swift index 026d16f8c..57ab2bfca 100644 --- a/Package.swift +++ b/Package.swift @@ -512,6 +512,7 @@ let package = Package( resources: [ .copy("Resources/remote-messaging-config-example.json"), .copy("Resources/remote-messaging-config-malformed.json"), + .copy("Resources/remote-messaging-config-metrics.json"), .copy("Resources/remote-messaging-config-unsupported-items.json"), .copy("Resources/remote-messaging-config.json"), ] diff --git a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift index 6a6608cef..725d1b979 100644 --- a/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift +++ b/Sources/RemoteMessaging/Mappers/JsonToRemoteMessageModelMapper.swift @@ -98,7 +98,7 @@ struct JsonToRemoteMessageModelMapper { content: content, matchingRules: message.matchingRules ?? [], exclusionRules: message.exclusionRules ?? [], - sendPixels: message.sendPixels ?? true + isMetricsEnabled: message.isMetricsEnabled ) if let translation = getTranslation(from: message.translations, for: Locale.current) { diff --git a/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift b/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift index 6cf8c02ac..5e0995e83 100644 --- a/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift +++ b/Sources/RemoteMessaging/Model/JsonRemoteMessagingConfig.swift @@ -31,11 +31,24 @@ public enum RemoteMessageResponse { let content: JsonContent let translations: [String: JsonContentTranslation]? let matchingRules, exclusionRules: [Int]? - let sendPixels: Bool? + let metrics: JsonMetrics? static func == (lhs: JsonRemoteMessage, rhs: JsonRemoteMessage) -> Bool { return lhs.id == rhs.id } + + var isMetricsEnabled: Bool { + metrics?.state.flatMap(JsonMetrics.MetricsState.init) != .disabled + } + } + + struct JsonMetrics: Decodable { + let state: String? + + enum MetricsState: String, Decodable { + case disabled + case enabled + } } struct JsonContent: Decodable { diff --git a/Sources/RemoteMessaging/Model/RemoteMessageModel.swift b/Sources/RemoteMessaging/Model/RemoteMessageModel.swift index 68a6808ad..bfebb812d 100644 --- a/Sources/RemoteMessaging/Model/RemoteMessageModel.swift +++ b/Sources/RemoteMessaging/Model/RemoteMessageModel.swift @@ -24,14 +24,14 @@ public struct RemoteMessageModel: Equatable, Codable { public var content: RemoteMessageModelType? public let matchingRules: [Int] public let exclusionRules: [Int] - public let sendPixels: Bool + public let isMetricsEnabled: Bool - public init(id: String, content: RemoteMessageModelType?, matchingRules: [Int], exclusionRules: [Int], sendPixels: Bool) { + public init(id: String, content: RemoteMessageModelType?, matchingRules: [Int], exclusionRules: [Int], isMetricsEnabled: Bool) { self.id = id self.content = content self.matchingRules = matchingRules self.exclusionRules = exclusionRules - self.sendPixels = sendPixels + self.isMetricsEnabled = isMetricsEnabled } enum CodingKeys: CodingKey { @@ -39,7 +39,7 @@ public struct RemoteMessageModel: Equatable, Codable { case content case matchingRules case exclusionRules - case sendPixels + case isMetricsEnabled } public init(from decoder: Decoder) throws { @@ -48,7 +48,7 @@ public struct RemoteMessageModel: Equatable, Codable { self.content = try container.decodeIfPresent(RemoteMessageModelType.self, forKey: .content) self.matchingRules = try container.decode([Int].self, forKey: .matchingRules) self.exclusionRules = try container.decode([Int].self, forKey: .exclusionRules) - self.sendPixels = try container.decodeIfPresent(Bool.self, forKey: .sendPixels) ?? true + self.isMetricsEnabled = try container.decodeIfPresent(Bool.self, forKey: .isMetricsEnabled) ?? true } public func encode(to encoder: Encoder) throws { @@ -57,7 +57,7 @@ public struct RemoteMessageModel: Equatable, Codable { try container.encodeIfPresent(self.content, forKey: .content) try container.encode(self.matchingRules, forKey: .matchingRules) try container.encode(self.exclusionRules, forKey: .exclusionRules) - try container.encode(self.sendPixels, forKey: .sendPixels) + try container.encode(self.isMetricsEnabled, forKey: .isMetricsEnabled) } mutating func localizeContent(translation: RemoteMessageResponse.JsonContentTranslation) { diff --git a/Sources/RemoteMessaging/RemoteMessagingStore.swift b/Sources/RemoteMessaging/RemoteMessagingStore.swift index a4c60cc44..7181a5fbe 100644 --- a/Sources/RemoteMessaging/RemoteMessagingStore.swift +++ b/Sources/RemoteMessaging/RemoteMessagingStore.swift @@ -231,7 +231,7 @@ extension RemoteMessagingStore { content: remoteMessage.content, matchingRules: [], exclusionRules: [], - sendPixels: remoteMessage.sendPixels + isMetricsEnabled: remoteMessage.isMetricsEnabled ) break } @@ -266,7 +266,7 @@ extension RemoteMessagingStore { content: remoteMessageMapped.content, matchingRules: [], exclusionRules: [], - sendPixels: remoteMessageMapped.sendPixels + isMetricsEnabled: remoteMessageMapped.isMetricsEnabled ) break } diff --git a/Tests/RemoteMessagingTests/Mappers/JsonToRemoteConfigModelMapperTests.swift b/Tests/RemoteMessagingTests/Mappers/JsonToRemoteConfigModelMapperTests.swift index 3a87e411d..10acb5227 100644 --- a/Tests/RemoteMessagingTests/Mappers/JsonToRemoteConfigModelMapperTests.swift +++ b/Tests/RemoteMessagingTests/Mappers/JsonToRemoteConfigModelMapperTests.swift @@ -31,7 +31,8 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { content: .bigSingleAction(titleText: "title", descriptionText: "description", placeholder: .announce, primaryActionText: "Ok", primaryAction: .url(value: "https://duckduckgo.com")), matchingRules: [], - exclusionRules: []) + exclusionRules: [], + isMetricsEnabled: true) ) XCTAssertEqual(config.messages[1], RemoteMessageModel( @@ -39,21 +40,24 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { content: .bigSingleAction(titleText: "Kedvenc hozzáadása", descriptionText: "Kedvenc eltávolítása", placeholder: .ddgAnnounce, primaryActionText: "Ok", primaryAction: .url(value: "https://duckduckgo.com")), matchingRules: [], - exclusionRules: []) + exclusionRules: [], + isMetricsEnabled: true) ) XCTAssertEqual(config.messages[2], RemoteMessageModel( id: "26780792-49fe-4e25-ae27-aa6a2e6f013b", content: .small(titleText: "Here goes a title", descriptionText: "description"), matchingRules: [5, 6], - exclusionRules: [7, 8, 9]) + exclusionRules: [7, 8, 9], + isMetricsEnabled: true) ) XCTAssertEqual(config.messages[3], RemoteMessageModel( id: "c3549d64-b388-41d8-9649-33e6e2674e8e", content: .medium(titleText: "Here goes a title", descriptionText: "description", placeholder: .criticalUpdate), matchingRules: [], - exclusionRules: []) + exclusionRules: [], + isMetricsEnabled: true) ) XCTAssertEqual(config.messages[4], RemoteMessageModel( @@ -62,7 +66,8 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { primaryActionText: "Ok", primaryAction: .appStore, secondaryActionText: "Cancel", secondaryAction: .dismiss), matchingRules: [], - exclusionRules: []) + exclusionRules: [], + isMetricsEnabled: true) ) XCTAssertEqual(config.messages[5], RemoteMessageModel( @@ -72,7 +77,8 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { title: "Share Title"), secondaryActionText: "Cancel", secondaryAction: .dismiss), matchingRules: [], - exclusionRules: []) + exclusionRules: [], + isMetricsEnabled: true) ) XCTAssertEqual(config.messages[6], RemoteMessageModel( @@ -80,7 +86,8 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { content: .promoSingleAction(titleText: "Promo Title", descriptionText: "Promo Description", placeholder: .newForMacAndWindows, actionText: "Promo Action", action: .dismiss), matchingRules: [], - exclusionRules: []) + exclusionRules: [], + isMetricsEnabled: true) ) XCTAssertEqual(config.messages[7], RemoteMessageModel( @@ -93,7 +100,8 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { action: .survey(value: "https://duckduckgo.com/survey") ), matchingRules: [8], - exclusionRules: []) + exclusionRules: [], + isMetricsEnabled: true) ) } @@ -225,6 +233,43 @@ class JsonToRemoteConfigModelMapperTests: XCTestCase { XCTAssertEqual(rule6?.attributes.filter { $0 is UnknownMatchingAttribute }.count, 1) } + func testThatMetricsAreEnabledWhenStatedInJSONOrMissing() throws { + let config = try decodeAndMapJson(fileName: "remote-messaging-config-metrics.json") + XCTAssertEqual(config.messages.count, 4) + + XCTAssertEqual(config.messages[0], RemoteMessageModel( + id: "1", + content: .small(titleText: "title", descriptionText: "description"), + matchingRules: [], + exclusionRules: [], + isMetricsEnabled: true) + ) + + XCTAssertEqual(config.messages[1], RemoteMessageModel( + id: "2", + content: .small(titleText: "title", descriptionText: "description"), + matchingRules: [], + exclusionRules: [], + isMetricsEnabled: true) + ) + + XCTAssertEqual(config.messages[2], RemoteMessageModel( + id: "3", + content: .small(titleText: "title", descriptionText: "description"), + matchingRules: [], + exclusionRules: [], + isMetricsEnabled: false) + ) + + XCTAssertEqual(config.messages[3], RemoteMessageModel( + id: "4", + content: .small(titleText: "title", descriptionText: "description"), + matchingRules: [], + exclusionRules: [], + isMetricsEnabled: true) + ) + } + func decodeAndMapJson(fileName: String) throws -> RemoteConfigModel { let resourceURL = Bundle.module.resourceURL!.appendingPathComponent(fileName, conformingTo: .json) let validJson = try Data(contentsOf: resourceURL) diff --git a/Tests/RemoteMessagingTests/RemoteMessagingConfigMatcherTests.swift b/Tests/RemoteMessagingTests/RemoteMessagingConfigMatcherTests.swift index 8aeead797..bfba9bab5 100644 --- a/Tests/RemoteMessagingTests/RemoteMessagingConfigMatcherTests.swift +++ b/Tests/RemoteMessagingTests/RemoteMessagingConfigMatcherTests.swift @@ -515,9 +515,10 @@ class RemoteMessagingConfigMatcherTests: XCTestCase { func mediumMessage(id: String = "1", matchingRules: [Int], exclusionRules: [Int]) -> RemoteMessageModel { return RemoteMessageModel(id: id, - content: .medium(titleText: "title", descriptionText: "description", placeholder: .announce), - matchingRules: matchingRules, - exclusionRules: exclusionRules + content: .medium(titleText: "title", descriptionText: "description", placeholder: .announce), + matchingRules: matchingRules, + exclusionRules: exclusionRules, + isMetricsEnabled: true ) } } diff --git a/Tests/RemoteMessagingTests/RemoteMessagingStoreTests.swift b/Tests/RemoteMessagingTests/RemoteMessagingStoreTests.swift index ffede3d3a..8c1c13b8e 100644 --- a/Tests/RemoteMessagingTests/RemoteMessagingStoreTests.swift +++ b/Tests/RemoteMessagingTests/RemoteMessagingStoreTests.swift @@ -93,7 +93,7 @@ class RemoteMessagingStoreTests: XCTestCase { XCTAssertEqual(config?.version, processorResult.version) guard let remoteMessage = store.fetchScheduledRemoteMessage() else { XCTFail("No remote message found") - return RemoteMessageModel(id: "", content: nil, matchingRules: [], exclusionRules: []) + return RemoteMessageModel(id: "", content: nil, matchingRules: [], exclusionRules: [], isMetricsEnabled: true) } XCTAssertNotNil(remoteMessage) diff --git a/Tests/RemoteMessagingTests/Resources/remote-messaging-config-metrics.json b/Tests/RemoteMessagingTests/Resources/remote-messaging-config-metrics.json new file mode 100644 index 000000000..a2570fad9 --- /dev/null +++ b/Tests/RemoteMessagingTests/Resources/remote-messaging-config-metrics.json @@ -0,0 +1,47 @@ +{ + "version": 1, + "messages": [ + { + "id": "1", + "content": { + "messageType": "small", + "titleText": "title", + "descriptionText": "description" + } + }, + { + "id": "2", + "content": { + "messageType": "small", + "titleText": "title", + "descriptionText": "description" + }, + "metrics": { + "state": "enabled" + } + }, + { + "id": "3", + "content": { + "messageType": "small", + "titleText": "title", + "descriptionText": "description" + }, + "metrics": { + "state": "disabled" + } + }, + { + "id": "4", + "content": { + "messageType": "small", + "titleText": "title", + "descriptionText": "description" + }, + "metrics": { + "state": "unsupported-value" + } + } + ], + "rules": [] +} From d199e9d24dd5f67523ee68130cc0768e6c1d3d6f Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 18 Jul 2024 18:45:40 +0200 Subject: [PATCH 3/3] Remove unneeded custom encode function --- Sources/RemoteMessaging/Model/RemoteMessageModel.swift | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Sources/RemoteMessaging/Model/RemoteMessageModel.swift b/Sources/RemoteMessaging/Model/RemoteMessageModel.swift index bfebb812d..d6a35c4aa 100644 --- a/Sources/RemoteMessaging/Model/RemoteMessageModel.swift +++ b/Sources/RemoteMessaging/Model/RemoteMessageModel.swift @@ -51,15 +51,6 @@ public struct RemoteMessageModel: Equatable, Codable { self.isMetricsEnabled = try container.decodeIfPresent(Bool.self, forKey: .isMetricsEnabled) ?? true } - public func encode(to encoder: Encoder) throws { - var container = encoder.container(keyedBy: CodingKeys.self) - try container.encode(self.id, forKey: .id) - try container.encodeIfPresent(self.content, forKey: .content) - try container.encode(self.matchingRules, forKey: .matchingRules) - try container.encode(self.exclusionRules, forKey: .exclusionRules) - try container.encode(self.isMetricsEnabled, forKey: .isMetricsEnabled) - } - mutating func localizeContent(translation: RemoteMessageResponse.JsonContentTranslation) { guard let content = content else { return