From ad28b07d2df9f2ec45ab0306053126e2c8b237ca Mon Sep 17 00:00:00 2001 From: Sam Symons Date: Fri, 6 Dec 2024 12:52:01 -0800 Subject: [PATCH] Update RMF version matching to ignore build number (#1118) Required: Task/Issue URL: https://app.asana.com/0/1207619243206445/1208908529122506/f iOS PR: duckduckgo/iOS#3686 macOS PR: duckduckgo/macos-browser#3635 What kind of version bump will this require?: Patch Description: This PR updates the RMF version matching logic to ignore build number. Previously, when you tried to match a version like 1.110.0, it would fail because you needed to know the build number. --- .../Matchers/AppAttributeMatcher.swift | 2 +- .../Model/MatchingAttributes.swift | 37 +++++++++++++++++-- .../CommonAppAttributeMatcherTests.swift | 25 ++++++++----- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/Sources/RemoteMessaging/Matchers/AppAttributeMatcher.swift b/Sources/RemoteMessaging/Matchers/AppAttributeMatcher.swift index 1805ebbd0..677ca9f3a 100644 --- a/Sources/RemoteMessaging/Matchers/AppAttributeMatcher.swift +++ b/Sources/RemoteMessaging/Matchers/AppAttributeMatcher.swift @@ -81,7 +81,7 @@ public struct CommonAppAttributeMatcher: AttributeMatching { assertionFailure("BundleIdentifier should not be empty") } self.init(bundleId: AppVersion.shared.identifier, - appVersion: AppVersion.shared.versionAndBuildNumber, + appVersion: AppVersion.shared.versionNumber, isInternalUser: isInternalUser, statisticsStore: statisticsStore, variantManager: variantManager) diff --git a/Sources/RemoteMessaging/Model/MatchingAttributes.swift b/Sources/RemoteMessaging/Model/MatchingAttributes.swift index becc8280e..4e2cf04a3 100644 --- a/Sources/RemoteMessaging/Model/MatchingAttributes.swift +++ b/Sources/RemoteMessaging/Model/MatchingAttributes.swift @@ -49,12 +49,27 @@ struct AppIdMatchingAttribute: SingleValueMatching { } struct AppVersionMatchingAttribute: StringRangeMatching { - static let defaultMaxValue: String = AppVersion.shared.versionAndBuildNumber - var min: String = MatchingAttributeDefaults.stringDefaultValue - var max: String = AppVersion.shared.versionAndBuildNumber - var value: String = MatchingAttributeDefaults.stringDefaultValue + static let defaultMaxValue: String = AppVersion.shared.versionNumber + + var min: String + var max: String + var value: String var fallback: Bool? + + // Legacy versions of the app require a build number in the version string in order to match correctly. + // To allow message authors to include a build number for backwards compatibility, while also allowing new clients to use the simpler version + // string, this initializer trims the build number before storing it. + init(min: String = MatchingAttributeDefaults.stringDefaultValue, + max: String = AppVersion.shared.versionNumber, + value: String = MatchingAttributeDefaults.stringDefaultValue, + fallback: Bool?) { + self.min = min.trimmingBuildNumber + self.max = max.trimmingBuildNumber + self.value = value.trimmingBuildNumber + self.fallback = fallback + } + } struct AtbMatchingAttribute: SingleValueMatching { @@ -306,3 +321,17 @@ struct RangeStringNumericMatchingAttribute: Equatable { return version + String(repeating: ".0", count: matchComponents.count - versionComponents.count) } } + +private extension String { + + var trimmingBuildNumber: String { + let components = self.split(separator: ".") + + if components.count == 4 { + return components.dropLast().joined(separator: ".") + } else { + return self + } + } + +} diff --git a/Tests/RemoteMessagingTests/Matchers/CommonAppAttributeMatcherTests.swift b/Tests/RemoteMessagingTests/Matchers/CommonAppAttributeMatcherTests.swift index 7c9d87672..b792cae83 100644 --- a/Tests/RemoteMessagingTests/Matchers/CommonAppAttributeMatcherTests.swift +++ b/Tests/RemoteMessagingTests/Matchers/CommonAppAttributeMatcherTests.swift @@ -26,6 +26,7 @@ import XCTest class CommonAppAttributeMatcherTests: XCTestCase { private var matcher: CommonAppAttributeMatcher! + private let versionNumber = "3.2.1" override func setUpWithError() throws { try super.setUpWithError() @@ -36,7 +37,13 @@ class CommonAppAttributeMatcherTests: XCTestCase { mockStatisticsStore.searchRetentionAtb = "v105-88" let manager = MockVariantManager(isSupportedReturns: true, currentVariant: MockVariant(name: "zo", weight: 44, isIncluded: { return true }, features: [.dummy])) - matcher = CommonAppAttributeMatcher(statisticsStore: mockStatisticsStore, variantManager: manager) + matcher = CommonAppAttributeMatcher( + bundleId: AppVersion.shared.identifier, + appVersion: versionNumber, + isInternalUser: true, + statisticsStore: mockStatisticsStore, + variantManager: manager + ) } override func tearDownWithError() throws { @@ -66,7 +73,7 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionEqualOrLowerThanMaxThenReturnMatch() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let appMajorVersion = appVersionComponents[0] let appMinorVersion = appVersionComponents.suffix(from: 1).joined(separator: ".") @@ -82,7 +89,7 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionGreaterThanMaxThenReturnFail() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let appMajorVersion = appVersionComponents[0] let lessThanMax = String(Int(appMajorVersion)! - 1) @@ -91,12 +98,12 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionEqualOrGreaterThanMinThenReturnMatch() throws { - XCTAssertEqual(matcher.evaluate(matchingAttribute: AppVersionMatchingAttribute(min: AppVersion.shared.versionAndBuildNumber, fallback: nil)), + XCTAssertEqual(matcher.evaluate(matchingAttribute: AppVersionMatchingAttribute(min: versionNumber, fallback: nil)), .match) } func testWhenAppVersionLowerThanMinThenReturnFail() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let (major, minor, patch) = (appVersionComponents[0], appVersionComponents[1], appVersionComponents[2]) let majorBumped = String(Int(major)! + 1) let patchBumped = [major, minor, String(Int(patch)! + 1)].joined(separator: ".") @@ -105,7 +112,7 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionInRangeThenReturnMatch() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let (major, minor, patch) = (appVersionComponents[0], appVersionComponents[1], appVersionComponents[2]) let majorBumped = String(Int(major)! + 1) let patchDecremented = [major, minor, String(Int(patch)! - 1)].joined(separator: ".") @@ -115,7 +122,7 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionNotInRangeThenReturnFail() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let appMajorVersion = appVersionComponents[0] let greaterThanMax = String(Int(appMajorVersion)! + 1) @@ -124,12 +131,12 @@ class CommonAppAttributeMatcherTests: XCTestCase { } func testWhenAppVersionSameAsDeviceThenReturnMatch() throws { - XCTAssertEqual(matcher.evaluate(matchingAttribute: AppVersionMatchingAttribute(min: AppVersion.shared.versionAndBuildNumber, max: AppVersion.shared.versionAndBuildNumber, fallback: nil)), + XCTAssertEqual(matcher.evaluate(matchingAttribute: AppVersionMatchingAttribute(min: versionNumber, max: versionNumber, fallback: nil)), .match) } func testWhenAppVersionDifferentToDeviceThenReturnFail() throws { - let appVersionComponents = AppVersion.shared.versionAndBuildNumber.components(separatedBy: ".").map { $0 } + let appVersionComponents = versionNumber.components(separatedBy: ".").map { $0 } let (major, minor, patch) = (appVersionComponents[0], appVersionComponents[1], appVersionComponents[2]) let patchDecremented = [major, minor, String(Int(patch)! - 1)].joined(separator: ".")