Skip to content

Commit

Permalink
Update RMF version matching to ignore build number (#1118)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
samsymons authored Dec 6, 2024
1 parent ff606d7 commit ad28b07
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Sources/RemoteMessaging/Matchers/AppAttributeMatcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
37 changes: 33 additions & 4 deletions Sources/RemoteMessaging/Model/MatchingAttributes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand Down Expand Up @@ -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: ".")

Expand All @@ -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)

Expand All @@ -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: ".")
Expand All @@ -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: ".")
Expand All @@ -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)

Expand All @@ -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: ".")

Expand Down

0 comments on commit ad28b07

Please sign in to comment.