Skip to content

Commit 51b0387

Browse files
muzahidul-optimuzahidul-opti
authored
[FSSDK-11373] add holdout support and refactor decision logic in DefaultDecisionService (#587)
1 parent 7c4fee7 commit 51b0387

20 files changed

+1971
-107
lines changed

‎OptimizelySwiftSDK.xcodeproj/project.pbxproj

+50
Large diffs are not rendered by default.

‎Sources/Data Model/FeatureFlag.swift

-2
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ struct FeatureFlag: Codable, Equatable, OptimizelyFeature {
3535
case variables
3636
}
3737

38-
// var holdoutIds: [String] = []
39-
4038
// MARK: - OptimizelyConfig
4139

4240
var experimentsMap: [String: OptimizelyExperiment] = [:]

‎Sources/Data Model/HoldoutConfig.swift

-1
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,3 @@ struct HoldoutConfig {
115115
return holdoutIdMap[id]
116116
}
117117
}
118-

‎Sources/Implementation/DecisionInfo.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ struct DecisionInfo {
2222
let decisionType: Constants.DecisionType
2323

2424
/// The experiment that the decision variation belongs to.
25-
var experiment: Experiment?
25+
var experiment: ExperimentCore?
2626

2727
/// The variation selected by the decision.
2828
var variation: Variation?
@@ -58,7 +58,7 @@ struct DecisionInfo {
5858
var decisionEventDispatched: Bool
5959

6060
init(decisionType: Constants.DecisionType,
61-
experiment: Experiment? = nil,
61+
experiment: ExperimentCore? = nil,
6262
variation: Variation? = nil,
6363
source: String? = nil,
6464
feature: FeatureFlag? = nil,

‎Sources/Implementation/DefaultBucketer.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class DefaultBucketer: OPTBucketer {
120120
return DecisionResponse(result: nil, reasons: reasons)
121121
}
122122

123-
func bucketToVariation(experiment: Experiment,
123+
func bucketToVariation(experiment: ExperimentCore,
124124
bucketingId: String) -> DecisionResponse<Variation> {
125125
let reasons = DecisionReasons()
126126

‎Sources/Implementation/DefaultDecisionService.swift

+272-85
Large diffs are not rendered by default.

‎Sources/Implementation/Events/BatchEventBuilder.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class BatchEventBuilder {
2222
// MARK: - Impression Event
2323

2424
static func createImpressionEvent(config: ProjectConfig,
25-
experiment: Experiment?,
25+
experiment: ExperimentCore?,
2626
variation: Variation?,
2727
userId: String,
2828
attributes: OptimizelyAttributes?,

‎Sources/Optimizely/OptimizelyClient.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ extension OptimizelyClient {
804804
return (source == Constants.DecisionSource.featureTest.rawValue && decision?.variation != nil) || config.sendFlagDecisions
805805
}
806806

807-
func sendImpressionEvent(experiment: Experiment?,
807+
func sendImpressionEvent(experiment: ExperimentCore?,
808808
variation: Variation?,
809809
userId: String,
810810
attributes: OptimizelyAttributes? = nil,
@@ -892,7 +892,7 @@ extension OptimizelyClient {
892892

893893
extension OptimizelyClient {
894894

895-
func sendActivateNotification(experiment: Experiment,
895+
func sendActivateNotification(experiment: ExperimentCore,
896896
variation: Variation,
897897
userId: String,
898898
attributes: OptimizelyAttributes?,

‎Sources/Protocols/OPTBucketer.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ protocol OPTBucketer {
3636
func bucketExperiment(config: ProjectConfig,
3737
experiment: Experiment,
3838
bucketingId: String) -> DecisionResponse<Variation>
39-
39+
4040
/**
4141
Hash the bucketing ID and map it to the range [0, 10000).
4242
- Parameter bucketingId: The ID for which to generate the hash and bucket values.

‎Sources/Utils/Constants.swift

+1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ struct Constants {
5757
case experiment = "experiment"
5858
case featureTest = "feature-test"
5959
case rollout = "rollout"
60+
case holdout = "holdout"
6061
}
6162

6263
struct DecisionInfoKeys {

‎Sources/Utils/LogMessage.swift

+14-4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import Foundation
1818

1919
enum LogMessage {
2020
case experimentNotRunning(_ key: String)
21+
case holdoutNotRunning(_ key: String)
2122
case featureEnabledForUser(_ key: String, _ userId: String)
2223
case featureNotEnabledForUser(_ key: String, _ userId: String)
2324
case featureHasNoExperiments(_ key: String)
@@ -34,7 +35,9 @@ enum LogMessage {
3435
case userAssignedToBucketValue(_ bucket: Int, _ userId: String)
3536
case userMappedToForcedVariation(_ userId: String, _ expId: String, _ varId: String)
3637
case userMeetsConditionsForTargetingRule(_ userId: String, _ rule: String)
38+
case userMeetsConditionsForHoldout(_ userId: String, _ holdoutKey: String)
3739
case userDoesntMeetConditionsForTargetingRule(_ userId: String, _ rule: String)
40+
case userDoesntMeetConditionsForHoldout(_ userId: String, _ holdoutKey: String)
3841
case userBucketedIntoTargetingRule(_ userId: String, _ rule: String)
3942
case userNotBucketedIntoTargetingRule(_ userId: String, _ rule: String)
4043
case userHasForcedDecision(_ userId: String, _ flagKey: String, _ ruleKey: String?, _ varKey: String)
@@ -44,8 +47,10 @@ enum LogMessage {
4447
case userHasNoForcedVariation(_ userId: String)
4548
case userHasNoForcedVariationForExperiment(_ userId: String, _ expKey: String)
4649
case userBucketedIntoVariationInExperiment(_ userId: String, _ expKey: String, _ varKey: String)
50+
case userBucketedIntoVariationInHoldout(_ userId: String, _ expKey: String, _ varKey: String)
4751
case userNotBucketedIntoVariation(_ userId: String)
4852
case userBucketedIntoInvalidVariation(_ id: String)
53+
case userNotBucketedIntoHoldoutVariation(_ userId: String)
4954
case userBucketedIntoExperimentInGroup(_ userId: String, _ expKey: String, _ group: String)
5055
case userNotBucketedIntoExperimentInGroup(_ userId: String, _ expKey: String, _ group: String)
5156
case userNotBucketedIntoAnyExperimentInGroup(_ userId: String, _ group: String)
@@ -76,6 +81,7 @@ extension LogMessage: CustomStringConvertible {
7681

7782
switch self {
7883
case .experimentNotRunning(let key): message = "Experiment (\(key)) is not running."
84+
case .holdoutNotRunning(let key): message = "Holdout (\(key)) is not running."
7985
case .featureEnabledForUser(let key, let userId): message = "Feature (\(key)) is enabled for user (\(userId))."
8086
case .featureNotEnabledForUser(let key, let userId): message = "Feature (\(key)) is not enabled for user (\(userId))."
8187
case .featureHasNoExperiments(let key): message = "Feature (\(key)) is not attached to any experiments."
@@ -91,10 +97,12 @@ extension LogMessage: CustomStringConvertible {
9197
case .savedVariationInUserProfile(let varId, let expId, let userId): message = "Saved variation (\(varId)) of experiment (\(expId)) for user (\(userId))."
9298
case .userAssignedToBucketValue(let bucket, let userId): message = "Assigned bucket (\(bucket)) to user with bucketing ID (\(userId))."
9399
case .userMappedToForcedVariation(let userId, let expId, let varId): message = "Set variation (\(varId)) for experiment (\(expId)) and user (\(userId)) in the forced variation map."
94-
case .userMeetsConditionsForTargetingRule(let userId, let rule): message = "User (\(userId)) meets conditions for targeting rule (\(rule))."
95-
case .userDoesntMeetConditionsForTargetingRule(let userId, let rule): message = "User (\(userId)) does not meet conditions for targeting rule (\(rule))."
96-
case .userBucketedIntoTargetingRule(let userId, let rule): message = "User (\(userId)) is in the traffic group of targeting rule (\(rule))."
97-
case .userNotBucketedIntoTargetingRule(let userId, let rule): message = "User (\(userId)) is not in the traffic group for targeting rule (\(rule)). Checking (Everyone Else) rule now."
100+
case .userMeetsConditionsForTargetingRule(let userId, let rule): message = "User (\(userId)) meets conditions for targeting rule (\(rule))."
101+
case .userMeetsConditionsForHoldout(let userId, let holdoutKey): message = "User (\(userId)) meets conditions for holdout(\(holdoutKey))."
102+
case .userDoesntMeetConditionsForTargetingRule(let userId, let rule): message = "User (\(userId)) does not meet conditions for targeting rule (\(rule))."
103+
case .userDoesntMeetConditionsForHoldout(let userId, let holdoutKey): message = "User (\(userId)) does not meet conditions for holdout (\(holdoutKey))."
104+
case .userBucketedIntoTargetingRule(let userId, let rule): message = "User (\(userId)) is in the traffic group of targeting rule (\(rule))."
105+
case .userNotBucketedIntoTargetingRule(let userId, let rule): message = "User (\(userId)) is not in the traffic group for targeting rule (\(rule)). Checking (Everyone Else) rule now."
98106
case .userHasForcedDecision(let userId, let flagKey, let ruleKey, let varKey):
99107
let target = (ruleKey != nil) ? "flag (\(flagKey)), rule (\(ruleKey!))" : "flag (\(flagKey))"
100108
message = "Variation (\(varKey)) is mapped to \(target) and user (\(userId)) in the forced decision map."
@@ -106,7 +114,9 @@ extension LogMessage: CustomStringConvertible {
106114
case .userHasNoForcedVariation(let userId): message = "User (\(userId)) is not in the forced variation map."
107115
case .userHasNoForcedVariationForExperiment(let userId, let expKey): message = "No experiment (\(expKey)) mapped to user (\(userId)) in the forced variation map."
108116
case .userBucketedIntoVariationInExperiment(let userId, let expKey, let varKey): message = "User (\(userId)) is in variation (\(varKey)) of experiment (\(expKey))"
117+
case .userBucketedIntoVariationInHoldout(let userId, let holdoutKey, let varKey): message = "User (\(userId)) is in variation (\(varKey)) of holdout (\(holdoutKey))"
109118
case .userNotBucketedIntoVariation(let userId): message = "User (\(userId)) is in no variation."
119+
case .userNotBucketedIntoHoldoutVariation(let userId): message = "User (\(userId)) is in no holdout variation."
110120
case .userBucketedIntoInvalidVariation(let id): message = "Bucketed into an invalid variation id (\(id))"
111121
case .userBucketedIntoExperimentInGroup(let userId, let expId, let group): message = "User (\(userId)) is in experiment (\(expId)) of group (\(group))."
112122
case .userNotBucketedIntoExperimentInGroup(let userId, let expKey, let group): message = "User (\(userId)) is not in experiment (\(expKey)) of group (\(group))."

‎Tests/OptimizelyTests-Common/BucketTests_HoldoutToVariation.swift

+143
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
//
2+
// Copyright 2019, 2021, Optimizely, Inc. and contributors
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
//
16+
17+
import XCTest
18+
19+
class BucketTests_HoldoutToVariation: XCTestCase {
20+
var optimizely: OptimizelyClient!
21+
var config: ProjectConfig!
22+
var bucketer: DefaultBucketer!
23+
24+
var kUserId = "123456"
25+
var kHoldoutId = "4444444"
26+
var kHoldoutKey = "holdout_key"
27+
28+
var kVariationKeyA = "a"
29+
var kVariationIdA = "a11"
30+
31+
var kAudienceIdCountry = "10"
32+
var kAudienceIdAge = "20"
33+
var kAudienceIdInvalid = "9999999"
34+
35+
var kAttributesCountryMatch: [String: Any] = ["country": "us"]
36+
var kAttributesCountryNotMatch: [String: Any] = ["country": "ca"]
37+
var kAttributesAgeMatch: [String: Any] = ["age": 30]
38+
var kAttributesAgeNotMatch: [String: Any] = ["age": 10]
39+
var kAttributesEmpty: [String: Any] = [:]
40+
41+
var holdout: Holdout!
42+
43+
// MARK: - Sample datafile data
44+
45+
var sampleHoldoutData: [String: Any] {
46+
return [
47+
"status": "Running",
48+
"id": kHoldoutId,
49+
"key": kHoldoutKey,
50+
"layerId": "10420273888",
51+
"trafficAllocation": [
52+
["entityId": kVariationIdA, "endOfRange": 1000] // 10% traffic allocation (0-1000 out of 10000)
53+
],
54+
"audienceIds": [kAudienceIdCountry],
55+
"variations": [
56+
["variables": [], "id": kVariationIdA, "key": kVariationKeyA]
57+
],
58+
]
59+
}
60+
61+
// MARK: - Setup
62+
63+
override func setUp() {
64+
super.setUp()
65+
66+
self.optimizely = OTUtils.createOptimizely(datafileName: "empty_datafile",
67+
clearUserProfileService: true)
68+
self.config = self.optimizely.config!
69+
self.bucketer = ((optimizely.decisionService as! DefaultDecisionService).bucketer as! DefaultBucketer)
70+
71+
// Initialize holdout
72+
holdout = try! OTUtils.model(from: sampleHoldoutData)
73+
}
74+
75+
// MARK: - Tests for bucketToVariation
76+
77+
func testBucketToVariation_ValidBucketingWithinAllocation() {
78+
// Test users that should bucket into the single variation (within 0-1000 range)
79+
let testCases = [
80+
["userId": "user1", "expectedVariation": kVariationKeyA], // Buckets to variation A
81+
["userId": "testuser", "expectedVariation": kVariationKeyA] // Buckets to variation A
82+
]
83+
84+
for (index, test) in testCases.enumerated() {
85+
// Mock bucket value to ensure it falls within 0-1000
86+
let mockBucketValue = 500 // Within 10% allocation
87+
let mockBucketer = MockBucketer(mockBucketValue: mockBucketValue)
88+
let response = mockBucketer.bucketToVariation(experiment: holdout, bucketingId: test["userId"]!)
89+
XCTAssertNotNil(response.result, "Variation should not be nil for test case \(index)")
90+
XCTAssertEqual(response.result?.key, test["expectedVariation"], "Wrong variation for test case \(index)")
91+
}
92+
}
93+
94+
func testBucketToVariation_BucketValueOutsideAllocation() {
95+
// Test users that fall outside the 10% allocation (bucket value > 1000)
96+
let testCases = [
97+
["userId": "user2"],
98+
["userId": "anotheruser"]
99+
]
100+
101+
for (index, test) in testCases.enumerated() {
102+
// Mock bucket value to ensure it falls outside 0-1000
103+
let mockBucketValue = 1500 // Outside 10% allocation
104+
let mockBucketer = MockBucketer(mockBucketValue: mockBucketValue)
105+
let response = mockBucketer.bucketToVariation(experiment: holdout, bucketingId: test["userId"]!)
106+
XCTAssertNil(response.result, "Variation should be nil for test case \(index) when outside allocation")
107+
}
108+
}
109+
110+
func testBucketToVariation_NoTrafficAllocation() {
111+
// Create a holdout with empty traffic allocation
112+
var modifiedHoldoutData = sampleHoldoutData
113+
modifiedHoldoutData["trafficAllocation"] = []
114+
let modifiedHoldout = try! OTUtils.model(from: modifiedHoldoutData) as Holdout
115+
116+
let response = bucketer.bucketToVariation(experiment: modifiedHoldout, bucketingId: kUserId)
117+
118+
XCTAssertNil(response.result, "Variation should be nil when no traffic allocation")
119+
}
120+
121+
func testBucketToVariation_InvalidVariationId() {
122+
// Create a holdout with invalid variation ID in traffic allocation
123+
var modifiedHoldoutData = sampleHoldoutData
124+
modifiedHoldoutData["trafficAllocation"] = [
125+
["entityId": "invalid_variation_id", "endOfRange": 1000]
126+
]
127+
let modifiedHoldout = try! OTUtils.model(from: modifiedHoldoutData) as Holdout
128+
129+
let response = bucketer.bucketToVariation(experiment: modifiedHoldout, bucketingId: kUserId)
130+
131+
XCTAssertNil(response.result, "Variation should be nil for invalid variation ID")
132+
}
133+
134+
func testBucketToVariation_EmptyBucketingId() {
135+
// Test with empty bucketing ID, still within allocation
136+
let mockBucketValue = 500
137+
let mockBucketer = MockBucketer(mockBucketValue: mockBucketValue)
138+
let response = mockBucketer.bucketToVariation(experiment: holdout, bucketingId: "")
139+
140+
XCTAssertNotNil(response.result, "Should still bucket with empty bucketing ID")
141+
XCTAssertEqual(response.result?.key, kVariationKeyA, "Should bucket to variation A")
142+
}
143+
}

‎Tests/OptimizelyTests-Common/DecisionListenerTests.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -1263,7 +1263,7 @@ class FakeDecisionService: DefaultDecisionService {
12631263
return DecisionResponse.responseNoReasons(result: featureDecision)
12641264
}
12651265

1266-
override func getVariationForFeatureExperiment(config: ProjectConfig, featureFlag: FeatureFlag, user: OptimizelyUserContext, userProfileTracker: UserProfileTracker? = nil, options: [OptimizelyDecideOption]? = nil) -> DecisionResponse<FeatureDecision> {
1266+
override func getVariationForFeatureExperiments(config: ProjectConfig, featureFlag: FeatureFlag, user: OptimizelyUserContext, userProfileTracker: UserProfileTracker? = nil, options: [OptimizelyDecideOption]? = nil) -> DecisionResponse<FeatureDecision> {
12671267
guard let experiment = self.experiment, let tmpVariation = self.variation else {
12681268
return DecisionResponse.nilNoReasons()
12691269
}

‎Tests/OptimizelyTests-Common/DecisionServiceTests_Features.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ class DecisionServiceTests_Features: XCTestCase {
258258
extension DecisionServiceTests_Features {
259259

260260
func testGetVariationForFeatureExperimentWhenMatched() {
261-
let pair = self.decisionService.getVariationForFeatureExperiment(config: config,
261+
let pair = self.decisionService.getVariationForFeatureExperiments(config: config,
262262
featureFlag: featureFlag,
263263
user: optimizely.createUserContext(userId: kUserId,
264264
attributes: kAttributesCountryMatch)).result
@@ -268,7 +268,7 @@ extension DecisionServiceTests_Features {
268268
}
269269

270270
func testGetVariationForFeatureExperimentWhenNotMatched() {
271-
let pair = self.decisionService.getVariationForFeatureExperiment(config: config,
271+
let pair = self.decisionService.getVariationForFeatureExperiments(config: config,
272272
featureFlag: featureFlag,
273273
user: optimizely.createUserContext(userId: kUserId,
274274
attributes: kAttributesCountryNotMatch)).result
@@ -280,7 +280,7 @@ extension DecisionServiceTests_Features {
280280
featureFlag.experimentIds = ["99999"] // not-existing experiment
281281
self.config.project.featureFlags = [featureFlag]
282282

283-
let pair = self.decisionService.getVariationForFeatureExperiment(config: config,
283+
let pair = self.decisionService.getVariationForFeatureExperiments(config: config,
284284
featureFlag: featureFlag,
285285
user: optimizely.createUserContext(userId: kUserId,
286286
attributes: kAttributesCountryMatch)).result

0 commit comments

Comments
 (0)