From f8e381771a33287e317da84d5676a5e2a271bca3 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Fri, 19 Jul 2024 08:27:31 +0200 Subject: [PATCH 1/2] Add support for skipping sending usage pixels for remote messages (#902) Task/Issue URL: https://app.asana.com/0/1201621708115095/1207841204698435/f Description: This change allows to skip sending usage pixels for a given remote message if that's stated in the RMF config. --- .../BrowserServicesKit-Package.xcscheme | 16 ++++- .../xcschemes/SubscriptionTests.xcscheme | 2 +- Package.swift | 1 + .../JsonToRemoteMessageModelMapper.swift | 11 ++-- .../Model/JsonRemoteMessagingConfig.swift | 14 +++++ .../Model/RemoteMessageModel.swift | 33 +++++----- .../RemoteMessagingStore.swift | 16 ++++- .../JsonToRemoteConfigModelMapperTests.swift | 61 ++++++++++++++++--- .../RemoteMessagingConfigMatcherTests.swift | 7 ++- .../RemoteMessagingStoreTests.swift | 2 +- .../remote-messaging-config-metrics.json | 47 ++++++++++++++ 11 files changed, 175 insertions(+), 35 deletions(-) create mode 100644 Tests/RemoteMessagingTests/Resources/remote-messaging-config-metrics.json 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 } + + 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 34c6e2eca..d6a35c4aa 100644 --- a/Sources/RemoteMessaging/Model/RemoteMessageModel.swift +++ b/Sources/RemoteMessaging/Model/RemoteMessageModel.swift @@ -24,28 +24,31 @@ public struct RemoteMessageModel: Equatable, Codable { public var content: RemoteMessageModelType? public let matchingRules: [Int] public let exclusionRules: [Int] + public let isMetricsEnabled: Bool - public init(id: String, content: RemoteMessageModelType?, matchingRules: [Int], exclusionRules: [Int]) { + 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.isMetricsEnabled = isMetricsEnabled } - 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 isMetricsEnabled + } + + 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.isMetricsEnabled = try container.decodeIfPresent(Bool.self, forKey: .isMetricsEnabled) ?? true } mutating func localizeContent(translation: RemoteMessageResponse.JsonContentTranslation) { diff --git a/Sources/RemoteMessaging/RemoteMessagingStore.swift b/Sources/RemoteMessaging/RemoteMessagingStore.swift index 83b46cb4a..7181a5fbe 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: [], + isMetricsEnabled: remoteMessage.isMetricsEnabled + ) 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: [], + 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 3274feb8d84fda5f27541c13f2ab428b4e77a5e2 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Fri, 19 Jul 2024 12:54:37 +0200 Subject: [PATCH 2/2] Resetting all state for the VPN will cancel the tunnel and stop the monitors (#900) Task/Issue URL: https://app.asana.com/0/1207603085593419/1207832283330964/f iOS PR: https://github.com/duckduckgo/iOS/pull/3099 macOS PR: https://github.com/duckduckgo/macos-browser/pull/2991 What kind of version bump will this require?: Patch **Optional**: Tech Design URL: CC: ## Description: Fix the VPN configuration removal handling in the extension to stop the tunnel. --- .../PacketTunnelProvider.swift | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index fb6496359..4b1069544 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -83,6 +83,9 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { // Subscription Errors - 100+ case vpnAccessRevoked + // State Reset - 200+ + case appRequestedCancellation + public var errorDescription: String? { switch self { case .startingTunnelWithoutAuthToken: @@ -93,6 +96,8 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { return "Failed to generate a tunnel configuration: \(internalError.localizedDescription)" case .simulateTunnelFailureError: return "Simulated a tunnel error as requested" + case .appRequestedCancellation: + return nil } } @@ -104,6 +109,8 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { case .simulateTunnelFailureError: return 2 // Subscription Errors - 100+ case .vpnAccessRevoked: return 100 + // State Reset - 200+ + case .appRequestedCancellation: return 200 } } @@ -111,7 +118,8 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { switch self { case .startingTunnelWithoutAuthToken, .simulateTunnelFailureError, - .vpnAccessRevoked: + .vpnAccessRevoked, + .appRequestedCancellation: return [:] case .couldNotGenerateTunnelConfiguration(let underlyingError): return [NSUnderlyingErrorKey: underlyingError] @@ -1147,9 +1155,10 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { try? tokenStore.deleteToken() #endif - // This is not really an error, we received a command to reset the connection - cancelTunnelWithError(nil) - completionHandler?(nil) + Task { + completionHandler?(nil) + await cancelTunnel(with: TunnelError.appRequestedCancellation) + } } private func handleGetLastErrorMessage(completionHandler: ((Data?) -> Void)? = nil) {