Skip to content

Commit a9742c9

Browse files
jaeoptjaeopt
committed
fix bucket boundary bug
1 parent 5d0cf20 commit a9742c9

File tree

7 files changed

+182
-87
lines changed

7 files changed

+182
-87
lines changed

‎OptimizelySwiftSDK.xcodeproj/project.pbxproj

Lines changed: 96 additions & 72 deletions
Large diffs are not rendered by default.

‎Sources/Implementation/DefaultBucketer.swift

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,7 @@ class DefaultBucketer: OPTBucketer {
7777
return nil
7878
}
7979

80-
for trafficAllocation in group.trafficAllocation where bucketValue <= trafficAllocation.endOfRange {
81-
let experimentId = trafficAllocation.entityId
82-
83-
// propagate errors and logs for unknown experiment
80+
if let experimentId = allocateTraffic(trafficAllocation: group.trafficAllocation, bucketValue: bucketValue) {
8481
if let experiment = config.getExperiment(id: experimentId) {
8582
return experiment
8683
} else {
@@ -91,7 +88,7 @@ class DefaultBucketer: OPTBucketer {
9188

9289
return nil
9390
}
94-
91+
9592
func bucketToVariation(experiment: Experiment, bucketingId: String) -> Variation? {
9693
let hashId = makeHashIdFromBucketingId(bucketingId: bucketingId, entityId: experiment.id)
9794
let bucketValue = generateBucketValue(bucketingId: hashId)
@@ -101,17 +98,24 @@ class DefaultBucketer: OPTBucketer {
10198
logger.e(.experimentHasNoTrafficAllocation(experiment.key))
10299
return nil
103100
}
104-
105-
for trafficAllocation in experiment.trafficAllocation where bucketValue <= trafficAllocation.endOfRange {
106-
let variationId = trafficAllocation.entityId
107-
108-
// propagate errors and logs for unknown variation
101+
102+
if let variationId = allocateTraffic(trafficAllocation: experiment.trafficAllocation, bucketValue: bucketValue) {
109103
if let variation = experiment.getVariation(id: variationId) {
110104
return variation
111105
} else {
112106
logger.e(.userBucketedIntoInvalidVariation(variationId))
113107
return nil
114108
}
109+
} else {
110+
return nil
111+
}
112+
}
113+
114+
func allocateTraffic(trafficAllocation: [TrafficAllocation], bucketValue: Int) -> String? {
115+
for bucket in trafficAllocation {
116+
if bucketValue < bucket.endOfRange {
117+
return bucket.entityId
118+
}
115119
}
116120

117121
return nil

‎Tests/OptimizelyTests-Common/BucketTests_Base.swift

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,59 @@ class BucketTests_Base: XCTestCase {
4141
}
4242
}
4343

44+
func testAllocateExperimentTraffic() {
45+
var experimentData: [String: Any] { return
46+
[
47+
"status": "Running",
48+
"id": "12345",
49+
"key": "experimentA",
50+
"layerId": "10420273888",
51+
"trafficAllocation": [
52+
[
53+
"entityId": "1000",
54+
"endOfRange": 0
55+
],
56+
[
57+
"entityId": "1001",
58+
"endOfRange": 3000
59+
],
60+
[
61+
"entityId": "1002",
62+
"endOfRange": 6000
63+
]
64+
],
65+
"audienceIds": [],
66+
"variations": [
67+
[
68+
"variables": [],
69+
"id": "1000",
70+
"key": "a"
71+
],
72+
[
73+
"variables": [],
74+
"id": "1001",
75+
"key": "b"
76+
],
77+
[
78+
"variables": [],
79+
"id": "1002",
80+
"key": "c"
81+
]
82+
],
83+
"forcedVariations": [:]
84+
]
85+
}
86+
87+
let bucketer = DefaultBucketer()
88+
let experiment: Experiment = try! OTUtils.model(from: experimentData)
89+
let trafficAllocation = experiment.trafficAllocation
90+
91+
XCTAssert(bucketer.allocateTraffic(trafficAllocation: trafficAllocation, bucketValue: 0) == "1001")
92+
XCTAssert(bucketer.allocateTraffic(trafficAllocation: trafficAllocation, bucketValue: 2999) == "1001")
93+
XCTAssert(bucketer.allocateTraffic(trafficAllocation: trafficAllocation, bucketValue: 3000) == "1002")
94+
XCTAssert(bucketer.allocateTraffic(trafficAllocation: trafficAllocation, bucketValue: 5999) == "1002")
95+
XCTAssertNil(bucketer.allocateTraffic(trafficAllocation: trafficAllocation, bucketValue: 6000))
96+
XCTAssertNil(bucketer.allocateTraffic(trafficAllocation: trafficAllocation, bucketValue: 7000))
97+
}
98+
4499
}

‎Tests/OptimizelyTests-Common/BucketTests_Others.swift

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ extension BucketTests_Others {
176176
extension BucketTests_Others {
177177

178178
func testBucketExperimentInMutexGroup() {
179-
let optimizely = OTUtils.createOptimizely(datafileName: "BucketerTestsDatafile", clearUserProfileService: true)!
179+
let optimizely = OTUtils.createOptimizely(datafileName: "bucketer_test", clearUserProfileService: true)!
180180
let group = optimizely.config!.getGroup(id: "1886780721")!
181181

182182
let bucketer = DefaultBucketer()
@@ -200,7 +200,7 @@ extension BucketTests_Others {
200200
}
201201

202202
func testBucketReturnsNilWhenExperimentIsExcludedFromMutex() {
203-
let optimizely = OTUtils.createOptimizely(datafileName: "BucketerTestsDatafile", clearUserProfileService: true)!
203+
let optimizely = OTUtils.createOptimizely(datafileName: "bucketer_test", clearUserProfileService: true)!
204204
let config = optimizely.config!
205205
let bucketer = DefaultBucketer()
206206

@@ -237,7 +237,7 @@ extension BucketTests_Others {
237237
}
238238

239239
func testBucketExperimentWithMutexDoesNotChangeExperimentReference() {
240-
let optimizely = OTUtils.createOptimizely(datafileName: "BucketerTestsDatafile", clearUserProfileService: true)!
240+
let optimizely = OTUtils.createOptimizely(datafileName: "bucketer_test", clearUserProfileService: true)!
241241
let config = optimizely.config!
242242
let bucketer = DefaultBucketer()
243243

@@ -248,7 +248,7 @@ extension BucketTests_Others {
248248
}
249249

250250
func testBucketWithBucketingId() {
251-
let optimizely = OTUtils.createOptimizely(datafileName: "BucketerTestsDatafile2", clearUserProfileService: true)!
251+
let optimizely = OTUtils.createOptimizely(datafileName: "bucketer_test2", clearUserProfileService: true)!
252252
let config = optimizely.config!
253253
let bucketer = DefaultBucketer()
254254

@@ -269,7 +269,7 @@ extension BucketTests_Others {
269269
func testBucketVariationGroupedExperimentsWithBucketingId() {
270270
// make sure that bucketing works with experiments in group
271271

272-
let optimizely = OTUtils.createOptimizely(datafileName: "BucketerTestsDatafile2", clearUserProfileService: true)!
272+
let optimizely = OTUtils.createOptimizely(datafileName: "bucketer_test2", clearUserProfileService: true)!
273273
let config = optimizely.config!
274274
let bucketer = DefaultBucketer()
275275

@@ -296,4 +296,16 @@ extension BucketTests_Others {
296296
XCTAssertNil(variation)
297297
}
298298

299+
func testBucketVariationAtBoundaries() {
300+
// testing #OASIS-7150
301+
// userId(2113143589306368718) + experId(18513703488) = “211314358930636871818513703488” creates a hash value of 0
302+
303+
let optimizely = OTUtils.createOptimizely(datafileName: "bucketer_test3", clearUserProfileService: true)!
304+
305+
XCTAssertFalse(optimizely.isFeatureEnabled(featureKey: "async_payments", userId: "2113143589306368718"))
306+
XCTAssertFalse(optimizely.isFeatureEnabled(featureKey: "async_payments", userId: "2113143589306368719"))
307+
XCTAssertFalse(optimizely.isFeatureEnabled(featureKey: "async_payments", userId: "2113143589306368710"))
308+
XCTAssertFalse(optimizely.isFeatureEnabled(featureKey: "async_payments", userId: "2113143589306368711"))
309+
XCTAssertFalse(optimizely.isFeatureEnabled(featureKey: "async_payments", userId: "2113143589306368712"))
310+
}
299311
}

‎Tests/TestData/BucketerTestsDatafile.json renamed to ‎Tests/TestData/bucketer_test.json

File renamed without changes.

‎Tests/TestData/BucketerTestsDatafile2.json renamed to ‎Tests/TestData/bucketer_test2.json

File renamed without changes.

‎Tests/TestData/UnsupportedVersionDatafile.json renamed to ‎Tests/TestData/unsupported_version.json

File renamed without changes.

0 commit comments

Comments
 (0)