Skip to content

Commit 5d8039f

Browse files
jaeoptjaeopt
committed
fix: bucketing error at traffic allocation boundaries (#365)
1 parent dbfaa66 commit 5d8039f

File tree

8 files changed

+233
-87
lines changed

8 files changed

+233
-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
@@ -79,10 +79,7 @@ class DefaultBucketer: OPTBucketer {
7979
return nil
8080
}
8181

82-
for trafficAllocation in group.trafficAllocation where bucketValue <= trafficAllocation.endOfRange {
83-
let experimentId = trafficAllocation.entityId
84-
85-
// propagate errors and logs for unknown experiment
82+
if let experimentId = allocateTraffic(trafficAllocation: group.trafficAllocation, bucketValue: bucketValue) {
8683
if let experiment = config.getExperiment(id: experimentId) {
8784
return experiment
8885
} else {
@@ -93,7 +90,7 @@ class DefaultBucketer: OPTBucketer {
9390

9491
return nil
9592
}
96-
93+
9794
func bucketToVariation(experiment: Experiment, bucketingId: String) -> Variation? {
9895
let hashId = makeHashIdFromBucketingId(bucketingId: bucketingId, entityId: experiment.id)
9996
let bucketValue = generateBucketValue(bucketingId: hashId)
@@ -103,17 +100,24 @@ class DefaultBucketer: OPTBucketer {
103100
logger.e(.experimentHasNoTrafficAllocation(experiment.key))
104101
return nil
105102
}
106-
107-
for trafficAllocation in experiment.trafficAllocation where bucketValue <= trafficAllocation.endOfRange {
108-
let variationId = trafficAllocation.entityId
109-
110-
// propagate errors and logs for unknown variation
103+
104+
if let variationId = allocateTraffic(trafficAllocation: experiment.trafficAllocation, bucketValue: bucketValue) {
111105
if let variation = experiment.getVariation(id: variationId) {
112106
return variation
113107
} else {
114108
logger.e(.userBucketedIntoInvalidVariation(variationId))
115109
return nil
116110
}
111+
} else {
112+
return nil
113+
}
114+
}
115+
116+
func allocateTraffic(trafficAllocation: [TrafficAllocation], bucketValue: Int) -> String? {
117+
for bucket in trafficAllocation {
118+
if bucketValue < bucket.endOfRange {
119+
return bucket.entityId
120+
}
117121
}
118122

119123
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/bucketer_test3.json

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
{
2+
"version": "4",
3+
"rollouts": [
4+
{
5+
"experiments": [
6+
{
7+
"status": "Running",
8+
"audienceIds": [ ],
9+
"variations": [
10+
{
11+
"variables": [ ],
12+
"id": "18532971919",
13+
"key": "18532971919",
14+
"featureEnabled": true
15+
}
16+
],
17+
"id": "18513703488",
18+
"key": "18513703488",
19+
"layerId": "18513932701",
20+
"trafficAllocation": [
21+
{
22+
"entityId": "18532971919",
23+
"endOfRange": 0
24+
}
25+
],
26+
"forcedVariations": { }
27+
}
28+
],
29+
"id": "18513932701"
30+
}
31+
],
32+
"anonymizeIP": true,
33+
"projectId": "10431130345",
34+
"variables": [],
35+
"featureFlags": [
36+
{
37+
"experimentIds": [],
38+
"rolloutId": "18513932701",
39+
"variables": [],
40+
"id": "18533552281",
41+
"key": "async_payments"
42+
}
43+
],
44+
"experiments": [],
45+
"audiences": [],
46+
"groups": [],
47+
"attributes": [],
48+
"accountId": "10367498574",
49+
"events": [],
50+
"revision": "100"
51+
}

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

File renamed without changes.

0 commit comments

Comments
 (0)