Skip to content

Commit d9e14b1

Browse files
yasirfolio3jaeopt
authored andcommitted
fix: Targeting rollout should move to fallback rule when bucketing fails. (#323)
1 parent e27df94 commit d9e14b1

File tree

2 files changed

+157
-30
lines changed

2 files changed

+157
-30
lines changed

Sources/Implementation/DefaultDecisionService.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2019, Optimizely, Inc. and contributors *
2+
* Copyright 2019-2020, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -200,9 +200,9 @@ class DefaultDecisionService: OPTDecisionService {
200200
logger.d(.userBucketedIntoTargetingRule(userId, index + 1))
201201

202202
return variation
203-
} else {
204-
logger.d(.userNotBucketedIntoTargetingRule(userId, index + 1))
205203
}
204+
logger.d(.userNotBucketedIntoTargetingRule(userId, index + 1))
205+
break
206206
} else {
207207
logger.d(.userDoesntMeetConditionsForTargetingRule(userId, index + 1))
208208
}

Tests/OptimizelyTests-Common/DecisionServiceTests_Features.swift

Lines changed: 154 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2019, Optimizely, Inc. and contributors *
2+
* Copyright 2019-2020, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -27,7 +27,9 @@ class DecisionServiceTests_Features: XCTestCase {
2727
var kExperimentId = "country11"
2828
var kRolloutId = "rollout11"
2929
var kRolloutExperimentId = "rolloutExp11"
30-
30+
var kRolloutExperimentId2 = "rolloutExp12"
31+
var kRolloutExperimentId3 = "rolloutExp13"
32+
3133
var kVariationKeyA = "a"
3234
var kVariationKeyB = "b"
3335
var kVariationKeyC = "c"
@@ -37,11 +39,21 @@ class DecisionServiceTests_Features: XCTestCase {
3739
var kAudienceIdAge = "20"
3840
var kAudienceIdInvalid = "9999999"
3941

42+
var kRolloutVariationKeyA = "rolloutA"
43+
var kRolloutVariationKeyB = "rolloutB"
44+
var kRolloutVariationKeyC = "rolloutC"
45+
46+
var kRolloutAudienceIdAge1 = "30"
47+
var kRolloutAudienceIdAge2 = "40"
48+
4049
var kAttributesCountryMatch: [String: Any] = ["country": "us"]
4150
var kAttributesCountryNotMatch: [String: Any] = ["country": "ca"]
4251
var kAttributesAgeMatch: [String: Any] = ["age": 30]
4352
var kAttributesAgeNotMatch: [String: Any] = ["age": 10]
4453
var kAttributesEmpty: [String: Any] = [:]
54+
var kAttributesRolloutAge1Match: [String: Any] = ["age": 20]
55+
var kAttributesRolloutAge2Match: [String: Any] = ["age": 30]
56+
var kAttributesRolloutNotMatch: [String: Any] = ["age": 40]
4557

4658
var experiment: Experiment!
4759
var variation: Variation!
@@ -127,32 +139,93 @@ class DecisionServiceTests_Features: XCTestCase {
127139
var sampleRolloutData: [String: Any] { return
128140
[
129141
"id": kRolloutId,
130-
"experiments": [sampleRolloutExperimentData]
142+
"experiments": sampleRolloutExperimentData
131143
]
132144
}
133145

134-
var sampleRolloutExperimentData: [String: Any] { return
146+
var sampleRolloutExperimentData: [[String: Any]] { return
135147
[
136-
"status": "Running",
137-
"id": kRolloutExperimentId,
138-
"key": "rolloutExp",
139-
"layerId": "10420273888",
140-
"trafficAllocation": [
141-
[
142-
"entityId": "10389700000",
143-
"endOfRange": 10000
144-
]
148+
[
149+
"status": "Running",
150+
"id": kRolloutExperimentId,
151+
"key": "rolloutExp",
152+
"layerId": "10420273888",
153+
"trafficAllocation": [
154+
[
155+
"entityId": "10389700000",
156+
"endOfRange": 10000
157+
]
158+
],
159+
"audienceIds": [kRolloutAudienceIdAge1],
160+
"variations": [
161+
[
162+
"variables": [],
163+
"id": "10389700000",
164+
"key": kRolloutVariationKeyA,
165+
"featureEnabled": true
166+
]
167+
],
168+
"forcedVariations": [:]
145169
],
146-
"audienceIds": [],
147-
"variations": [
148-
[
149-
"variables": [],
150-
"id": "10389700000",
151-
"key": kVariationKeyA,
152-
"featureEnabled": true
153-
]
170+
[
171+
"status": "Running",
172+
"id": kRolloutExperimentId2,
173+
"key": "rolloutExp1",
174+
"layerId": "10420273889",
175+
"trafficAllocation": [
176+
[
177+
"entityId": "10389700000",
178+
"endOfRange": 10000
179+
]
180+
],
181+
"audienceIds": [kRolloutAudienceIdAge2],
182+
"variations": [
183+
[
184+
"variables": [],
185+
"id": "10389700000",
186+
"key": kRolloutVariationKeyB,
187+
"featureEnabled": true
188+
]
189+
],
190+
"forcedVariations": [:]
154191
],
155-
"forcedVariations": [:]
192+
[
193+
"status": "Running",
194+
"id": kRolloutExperimentId3,
195+
"key": "rolloutExp2",
196+
"layerId": "10420273890",
197+
"trafficAllocation": [
198+
[
199+
"entityId": "10389700000",
200+
"endOfRange": 10000
201+
]
202+
],
203+
"audienceIds": [],
204+
"variations": [
205+
[
206+
"variables": [],
207+
"id": "10389700000",
208+
"key": kRolloutVariationKeyC,
209+
"featureEnabled": true
210+
]
211+
],
212+
"forcedVariations": [:]
213+
]
214+
]
215+
}
216+
217+
var sampleRolloutTypedAudiencesData: [[String: Any]] { return
218+
[
219+
[
220+
"id": kRolloutAudienceIdAge1,
221+
"conditions": [ "type": "custom_attribute", "name": "age", "match": "lt", "value": 30 ],
222+
"name": "age"
223+
],
224+
[
225+
"id": kRolloutAudienceIdAge2,
226+
"conditions": [ "type": "custom_attribute", "name": "age", "match": "lt", "value": 40 ],
227+
"name": "age"
228+
]
156229
]
157230
}
158231

@@ -222,14 +295,15 @@ extension DecisionServiceTests_Features {
222295
func testGetVariationForFeatureRollout() {
223296
// rollout set
224297
self.config.project.rollouts = [try! OTUtils.model(from: sampleRolloutData)]
298+
self.config.project.typedAudiences = try! OTUtils.model(from: sampleRolloutTypedAudiencesData)
225299
featureFlag.rolloutId = kRolloutId
226300
self.config.project.featureFlags = [featureFlag]
227301

228302
let variation = self.decisionService.getVariationForFeatureRollout(config: config,
229303
featureFlag: featureFlag,
230304
userId: kUserId,
231-
attributes: kAttributesEmpty)
232-
XCTAssert(variation!.key == kVariationKeyA)
305+
attributes: kAttributesRolloutAge1Match)
306+
XCTAssert(variation!.key == kRolloutVariationKeyA)
233307
}
234308

235309
func testGetVariationForFeatureRolloutEmpty() {
@@ -253,10 +327,63 @@ extension DecisionServiceTests_Features {
253327
XCTAssertNil(variation)
254328
}
255329

256-
func testGetVariationForFeatureRolloutMultiple() {
257-
// add tests for last rollout handling
330+
func testGetVariationForFeatureRolloutFallbackRule() {
331+
self.config.project.rollouts = [try! OTUtils.model(from: sampleRolloutData)]
332+
self.config.project.typedAudiences = try! OTUtils.model(from: sampleRolloutTypedAudiencesData)
333+
self.config.project.rollouts[0].experiments[0].trafficAllocation[0].endOfRange = 0
334+
featureFlag.rolloutId = kRolloutId
335+
self.config.project.featureFlags = [featureFlag]
336+
337+
let variation = self.decisionService.getVariationForFeatureRollout(config: config,
338+
featureFlag: featureFlag,
339+
userId: kUserId,
340+
attributes: kAttributesRolloutAge1Match)
341+
XCTAssert(variation!.key == kRolloutVariationKeyC)
342+
}
343+
344+
func testGetVariationForFeatureRolloutEvaluatesNextIfAudienceEvaluationFails() {
345+
self.config.project.rollouts = [try! OTUtils.model(from: sampleRolloutData)]
346+
self.config.project.typedAudiences = try! OTUtils.model(from: sampleRolloutTypedAudiencesData)
347+
featureFlag.rolloutId = kRolloutId
348+
self.config.project.featureFlags = [featureFlag]
349+
350+
let variation = self.decisionService.getVariationForFeatureRollout(config: config,
351+
featureFlag: featureFlag,
352+
userId: kUserId,
353+
attributes: kAttributesRolloutAge2Match)
354+
XCTAssert(variation!.key == kRolloutVariationKeyB)
258355
}
259356

357+
func testGetVariationForFeatureRolloutReturnsNilIfAudienceEvaluationFailsForFallback() {
358+
self.config.project.rollouts = [try! OTUtils.model(from: sampleRolloutData)]
359+
self.config.project.typedAudiences = try! OTUtils.model(from: sampleRolloutTypedAudiencesData)
360+
featureFlag.rolloutId = kRolloutId
361+
self.config.project.featureFlags = [featureFlag]
362+
363+
self.config.project.rollouts[0].experiments[0].audienceIds = [kRolloutAudienceIdAge2]
364+
self.config.project.rollouts[0].experiments[0].trafficAllocation[0].endOfRange = 0
365+
self.config.project.rollouts[0].experiments[2].audienceIds = [kRolloutAudienceIdAge1]
366+
let variation = self.decisionService.getVariationForFeatureRollout(config: config,
367+
featureFlag: featureFlag,
368+
userId: kUserId,
369+
attributes: kAttributesRolloutAge2Match)
370+
XCTAssertNil(variation)
371+
}
372+
373+
func testGetVariationForFeatureRolloutReturnsNilIfBucketingFailsForFallback() {
374+
self.config.project.rollouts = [try! OTUtils.model(from: sampleRolloutData)]
375+
self.config.project.typedAudiences = try! OTUtils.model(from: sampleRolloutTypedAudiencesData)
376+
featureFlag.rolloutId = kRolloutId
377+
self.config.project.featureFlags = [featureFlag]
378+
379+
self.config.project.rollouts[0].experiments[0].trafficAllocation[0].endOfRange = 0
380+
self.config.project.rollouts[0].experiments[2].trafficAllocation[0].endOfRange = 0
381+
let variation = self.decisionService.getVariationForFeatureRollout(config: config,
382+
featureFlag: featureFlag,
383+
userId: kUserId,
384+
attributes: kAttributesRolloutAge1Match)
385+
XCTAssertNil(variation)
386+
}
260387
}
261388

262389
// MARK: - Test getVariationForFeatureERollout()
@@ -293,7 +420,7 @@ extension DecisionServiceTests_Features {
293420
attributes: kAttributesCountryNotMatch)
294421
XCTAssertNotNil(pair)
295422
XCTAssertNil(pair!.experiment)
296-
XCTAssert(pair!.variation!.key == kVariationKeyA)
423+
XCTAssert(pair!.variation!.key == kRolloutVariationKeyC)
297424
}
298425

299426
}

0 commit comments

Comments
 (0)