Skip to content

Commit f11e5bb

Browse files
jaeoptjaeopt
authored
Merge pull request #119 from optimizely/refactorDecisionService
(chore):refactor decision service, bucketer, and user profile service. The d…
2 parents 8e7eb23 + a883ac6 commit f11e5bb

File tree

10 files changed

+69
-145
lines changed

10 files changed

+69
-145
lines changed

‎OptimizelySDK/Extensions/OptimizelyManager+Extension.swift

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ extension OptimizelyManager {
1212
func registerServices(sdkKey:String,
1313
logger:OPTLogger,
1414
eventDispatcher:OPTEventDispatcher,
15-
userProfileService:OPTUserProfileService,
1615
datafileHandler:OPTDatafileHandler,
17-
bucketer:OPTBucketer,
1816
decisionService:OPTDecisionService,
1917
notificationCenter:OPTNotificationCenter) {
2018
// bind it as a non-singleton. so, we will create an instance anytime injected.
@@ -26,10 +24,6 @@ extension OptimizelyManager {
2624
// this is bound a reusable singleton. so, if we re-initalize, we will keep this.
2725
HandlerRegistryService.shared.registerBinding(binder:Binder<OPTNotificationCenter>(service: OPTNotificationCenter.self).singetlon().reInitializeStrategy(strategy: .reUse).using(instance:notificationCenter).sdkKey(key: sdkKey))
2826

29-
// this is a singleton but it has a reIntializeStrategy of reCreate. So, we create a new
30-
// instance on re-initialize.
31-
HandlerRegistryService.shared.registerBinding(binder:Binder<OPTBucketer>(service: OPTBucketer.self).singetlon().using(instance:bucketer).sdkKey(key: sdkKey))
32-
3327
// the decision service is also a singleton that will reCreate on re-initalize
3428
HandlerRegistryService.shared.registerBinding(binder:Binder<OPTDecisionService>(service: OPTDecisionService.self).singetlon().using(instance:decisionService).sdkKey(key: sdkKey))
3529

@@ -39,9 +33,5 @@ extension OptimizelyManager {
3933
// This is a singleton and might be a good candidate for reuse. The handler supports mulitple
4034
// sdk keys without having to be created for every key.
4135
HandlerRegistryService.shared.registerBinding(binder:Binder<OPTDatafileHandler>(service: OPTDatafileHandler.self).singetlon().reInitializeStrategy(strategy: .reUse).to(factory: type(of:datafileHandler).init).using(instance: datafileHandler).sdkKey(key: sdkKey))
42-
43-
// the user profile service is also a singleton using eh passed in version.
44-
HandlerRegistryService.shared.registerBinding(binder:Binder<OPTUserProfileService>(service: OPTUserProfileService.self).singetlon().reInitializeStrategy(strategy:.reUse).using(instance:userProfileService).to(factory: type(of:userProfileService).init).sdkKey(key: sdkKey))
45-
4636
}
4737
}

‎OptimizelySDK/Implementation/DefaultBucketer.swift

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,14 @@ class DefaultBucketer : OPTBucketer {
2222
let MAX_HASH_SEED:UInt64 = 1
2323
var MAX_HASH_VALUE:UInt64?
2424

25-
private var config:ProjectConfig!
2625
private lazy var logger = HandlerRegistryService.shared.injectLogger()
2726

28-
internal required init(config:ProjectConfig) {
29-
self.config = config
30-
31-
MAX_HASH_VALUE = MAX_HASH_SEED << 32
32-
}
33-
3427
// [Jae]: let be configured after initialized (with custom DecisionHandler set up on OPTManger initialization)
3528
init() {
3629
MAX_HASH_VALUE = MAX_HASH_SEED << 32
3730
}
3831

39-
func initialize(config:ProjectConfig) {
40-
self.config = config
41-
}
42-
43-
func bucketToExperiment(group: Group, bucketingId: String) -> Experiment? {
32+
func bucketToExperiment(config:ProjectConfig, group: Group, bucketingId: String) -> Experiment? {
4433
let hashId = makeHashIdFromBucketingId(bucketingId: bucketingId, entityId: group.id)
4534
let bucketValue = self.generateBucketValue(bucketingId: hashId)
4635

@@ -73,7 +62,7 @@ class DefaultBucketer : OPTBucketer {
7362
return nil
7463
}
7564

76-
func bucketExperiment(experiment: Experiment, bucketingId: String) -> Variation? {
65+
func bucketExperiment(config:ProjectConfig, experiment: Experiment, bucketingId: String) -> Variation? {
7766
var ok = true
7867
// check for mutex
7968
let group = config.project.groups.filter({ if let _ = $0.experiments.filter({$0.id == experiment.id }).first { return true } else { return false }}).first
@@ -83,7 +72,7 @@ class DefaultBucketer : OPTBucketer {
8372
case .overlapping:
8473
break;
8574
case .random:
86-
let mutexExperiment = bucketToExperiment(group: group, bucketingId: bucketingId)
75+
let mutexExperiment = bucketToExperiment(config: config, group: group, bucketingId: bucketingId)
8776
if let mutexExperiment = mutexExperiment, mutexExperiment.id == experiment.id {
8877
ok = true
8978
}

‎OptimizelySDK/Implementation/DefaultDecisionService.swift

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,26 +17,16 @@
1717
import Foundation
1818

1919
class DefaultDecisionService : OPTDecisionService {
20-
var config:ProjectConfig!
21-
var bucketer:OPTBucketer!
22-
var userProfileService:OPTUserProfileService!
23-
24-
internal required init(config:ProjectConfig, bucketer:OPTBucketer, userProfileService:OPTUserProfileService) {
25-
self.config = config
26-
self.bucketer = bucketer
27-
self.userProfileService = userProfileService
28-
}
2920

30-
// [Jae]: let be configured after initialized (with custom DecisionHandler set up on OPTManger initialization)
31-
init() {}
21+
let bucketer:OPTBucketer
22+
let userProfileService:OPTUserProfileService
3223

33-
func initialize(config:ProjectConfig, bucketer:OPTBucketer, userProfileService:OPTUserProfileService) {
34-
self.config = config
35-
self.bucketer = bucketer
24+
init(userProfileService:OPTUserProfileService) {
25+
self.bucketer = DefaultBucketer()
3626
self.userProfileService = userProfileService
3727
}
38-
39-
func getVariation(userId:String, experiment: Experiment, attributes: OptimizelyAttributes) -> Variation? {
28+
29+
func getVariation(config:ProjectConfig, userId:String, experiment: Experiment, attributes: OptimizelyAttributes) -> Variation? {
4030
let experimentId = experiment.id;
4131

4232
// Acquire bucketingId .
@@ -68,12 +58,13 @@ class DefaultDecisionService : OPTDecisionService {
6858
var bucketedVariation:Variation?
6959
// ---- check if the user passes audience targeting before bucketing ----
7060
if let result = isInExperiment(
61+
config: config,
7162
experiment:experiment,
7263
userId:userId,
7364
attributes:attributes), result == true {
7465

7566
// bucket user into a variation
76-
bucketedVariation = bucketer.bucketExperiment(experiment: experiment, bucketingId:bucketingId)
67+
bucketedVariation = bucketer.bucketExperiment(config: config, experiment: experiment, bucketingId:bucketingId)
7768

7869
if let bucketedVariation = bucketedVariation {
7970
// save to user profile
@@ -85,7 +76,7 @@ class DefaultDecisionService : OPTDecisionService {
8576

8677
}
8778

88-
func isInExperiment(experiment:Experiment, userId:String, attributes: OptimizelyAttributes) -> Bool? {
79+
func isInExperiment(config:ProjectConfig, experiment:Experiment, userId:String, attributes: OptimizelyAttributes) -> Bool? {
8980

9081
if let conditions = experiment.audienceConditions {
9182
switch conditions {
@@ -119,45 +110,46 @@ class DefaultDecisionService : OPTDecisionService {
119110
return true
120111
}
121112

122-
func getExperimentInGroup(group:Group, bucketingId:String) -> Experiment? {
123-
let experiment = bucketer.bucketToExperiment(group:group, bucketingId:bucketingId)
113+
func getExperimentInGroup(config:ProjectConfig, group:Group, bucketingId:String) -> Experiment? {
114+
let experiment = bucketer.bucketToExperiment(config: config, group:group, bucketingId:bucketingId)
124115
if let _ = experiment {
125116
// log
126117
}
127118

128119
return experiment;
129120
}
130121

131-
func getVariationForFeature(featureFlag:FeatureFlag, userId:String, attributes: OptimizelyAttributes) -> (experiment:Experiment?, variation:Variation?)? {
122+
func getVariationForFeature(config:ProjectConfig, featureFlag:FeatureFlag, userId:String, attributes: OptimizelyAttributes) -> (experiment:Experiment?, variation:Variation?)? {
132123
//Evaluate in this order:
133124

134125
//1. Attempt to bucket user into experiment using feature flag.
135126
// Check if the feature flag is under an experiment and the the user is bucketed into one of these experiments
136-
if let variation = getVariationForFeatureExperiment(featureFlag: featureFlag, userId:userId, attributes:attributes) {
127+
if let variation = getVariationForFeatureExperiment(config: config, featureFlag: featureFlag, userId:userId, attributes:attributes) {
137128
return variation
138129
}
139130

140131
//2. Attempt to bucket user into rollout using the feature flag.
141132
// Check if the feature flag has rollout and the user is bucketed into one of it's rules
142-
if let variation = getVariationForFeatureRollout(featureFlag: featureFlag, userId:userId, attributes:attributes) {
133+
if let variation = getVariationForFeatureRollout(config: config, featureFlag: featureFlag, userId:userId, attributes:attributes) {
143134
return (nil, variation)
144135
}
145136

146137
return nil;
147138

148139
}
149140

150-
func getVariationForFeatureGroup(featureFlag: FeatureFlag,
141+
func getVariationForFeatureGroup(config:ProjectConfig,
142+
featureFlag: FeatureFlag,
151143
groupId: String,
152144
userId: String,
153145
attributes: OptimizelyAttributes) -> (experiment:Experiment?, variation:Variation?)? {
154146

155147
let bucketing_id = getBucketingId(userId:userId, attributes:attributes)
156148
if let group = config.project.groups.filter({$0.id == groupId}).first {
157149

158-
if let experiment = getExperimentInGroup(group: group, bucketingId:bucketing_id),
150+
if let experiment = getExperimentInGroup(config: config, group: group, bucketingId:bucketing_id),
159151
featureFlag.experimentIds.contains(experiment.id),
160-
let variation = getVariation(userId:userId, experiment:experiment, attributes:attributes) {
152+
let variation = getVariation(config: config, userId:userId, experiment:experiment, attributes:attributes) {
161153
// log
162154
return (experiment,variation)
163155
}
@@ -169,7 +161,8 @@ class DefaultDecisionService : OPTDecisionService {
169161
return nil
170162
}
171163

172-
func getVariationForFeatureExperiment(featureFlag: FeatureFlag,
164+
func getVariationForFeatureExperiment(config:ProjectConfig,
165+
featureFlag: FeatureFlag,
173166
userId: String,
174167
attributes: OptimizelyAttributes) -> (experiment:Experiment?, variation:Variation?)? {
175168

@@ -178,14 +171,15 @@ class DefaultDecisionService : OPTDecisionService {
178171
// Evaluate each experiment ID and return the first bucketed experiment variation
179172
for experimentId in experimentIds {
180173
if let experiment = config.getExperiment(id: experimentId),
181-
let variation = getVariation(userId: userId, experiment: experiment, attributes: attributes) {
174+
let variation = getVariation(config: config, userId: userId, experiment: experiment, attributes: attributes) {
182175
return (experiment,variation)
183176
}
184177
}
185178
return nil;
186179
}
187180

188-
func getVariationForFeatureRollout(featureFlag: FeatureFlag,
181+
func getVariationForFeatureRollout(config:ProjectConfig,
182+
featureFlag: FeatureFlag,
189183
userId: String,
190184
attributes: OptimizelyAttributes) -> Variation? {
191185

@@ -200,16 +194,16 @@ class DefaultDecisionService : OPTDecisionService {
200194
let rolloutRules = rollout.experiments
201195
// Evaluate all rollout rules except for last one
202196
for experiment in rolloutRules[0..<rolloutRules.count.advanced(by: -1)] {
203-
if isInExperiment(experiment: experiment, userId: userId, attributes: attributes) ?? false {
204-
if let variation = bucketer.bucketExperiment(experiment: experiment, bucketingId: bucketingId) {
197+
if isInExperiment(config: config, experiment: experiment, userId: userId, attributes: attributes) ?? false {
198+
if let variation = bucketer.bucketExperiment(config:config, experiment: experiment, bucketingId: bucketingId) {
205199
return variation
206200
}
207201
}
208202
}
209203
// Evaluate fall back rule / last rule now
210204
let experiment = rolloutRules[rolloutRules.count - 1];
211-
if isInExperiment(experiment: experiment, userId: userId, attributes: attributes) ?? false {
212-
return bucketer.bucketExperiment(experiment: experiment, bucketingId: bucketingId)
205+
if isInExperiment(config: config, experiment: experiment, userId: userId, attributes: attributes) ?? false {
206+
return bucketer.bucketExperiment(config: config, experiment: experiment, bucketingId: bucketingId)
213207
}
214208

215209
return nil

‎OptimizelySDK/Optimizely/OptimizelyManager.swift

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,10 @@ open class OptimizelyManager: NSObject {
3030
return HandlerRegistryService.shared.injectEventDispatcher(sdkKey: self.sdkKey)!
3131
}
3232
}
33-
var userProfileService: OPTUserProfileService {
34-
get {
35-
return HandlerRegistryService.shared.injectUserProfileService(sdkKey: self.sdkKey)!
36-
}
37-
}
3833
let periodicDownloadInterval: Int
3934

4035
// MARK: - Default Services
4136

42-
var bucketer: OPTBucketer {
43-
get {
44-
return HandlerRegistryService.shared.injectBucketer(sdkKey: self.sdkKey)!
45-
}
46-
}
4737
var decisionService: OPTDecisionService {
4838
get {
4939
return HandlerRegistryService.shared.injectDecisionService(sdkKey: self.sdkKey)!
@@ -82,13 +72,12 @@ open class OptimizelyManager: NSObject {
8272

8373
super.init()
8474

75+
let userProfileService = userProfileService ?? DefaultUserProfileService()
8576
self.registerServices(sdkKey: sdkKey,
8677
logger: logger ?? DefaultLogger(),
8778
eventDispatcher: eventDispatcher ?? DefaultEventDispatcher.sharedInstance,
88-
userProfileService: userProfileService ?? DefaultUserProfileService(),
8979
datafileHandler: DefaultDatafileHandler(),
90-
bucketer: DefaultBucketer(),
91-
decisionService: DefaultDecisionService(),
80+
decisionService: DefaultDecisionService(userProfileService: userProfileService),
9281
notificationCenter: DefaultNotificationCenter())
9382

9483
}
@@ -158,11 +147,6 @@ open class OptimizelyManager: NSObject {
158147
// this isn't really necessary because the try would throw if there is a problem. But, we want to avoid using bang so we do another let binding.
159148
guard let config = self.config else { throw OptimizelyError.dataFileInvalid }
160149

161-
// TODO: fix these to throw errors
162-
bucketer.initialize(config: config)
163-
decisionService.initialize(config: config,
164-
bucketer: self.bucketer,
165-
userProfileService: self.userProfileService)
166150
if periodicDownloadInterval > 0 {
167151
datafileHandler.stopPeriodicUpdates(sdkKey: self.sdkKey)
168152
datafileHandler.startPeriodicUpdates(sdkKey: self.sdkKey, updateInterval: periodicDownloadInterval) { data in
@@ -181,12 +165,6 @@ open class OptimizelyManager: NSObject {
181165
HandlerRegistryService.shared.reInitializeComponent(service: component, sdkKey: self.sdkKey)
182166
}
183167

184-
// now reinitialize with the new config.
185-
self.bucketer.initialize(config: config)
186-
self.decisionService.initialize(config: config,
187-
bucketer: self.bucketer,
188-
userProfileService: self.userProfileService)
189-
190168
}
191169

192170
self.notificationCenter.sendNotifications(type:
@@ -347,7 +325,7 @@ open class OptimizelyManager: NSObject {
347325
}
348326

349327
// fix DecisionService to throw error
350-
guard let variation = decisionService.getVariation(userId: userId, experiment: experiment, attributes: attributes ?? OptimizelyAttributes()) else {
328+
guard let variation = decisionService.getVariation(config: config, userId: userId, experiment: experiment, attributes: attributes ?? OptimizelyAttributes()) else {
351329
throw OptimizelyError.variationUnknown
352330
}
353331

@@ -417,7 +395,7 @@ open class OptimizelyManager: NSObject {
417395
}
418396

419397
// fix DecisionService to throw error
420-
let pair = decisionService.getVariationForFeature(featureFlag: featureFlag, userId: userId, attributes: attributes ?? OptimizelyAttributes())
398+
let pair = decisionService.getVariationForFeature(config: config, featureFlag: featureFlag, userId: userId, attributes: attributes ?? OptimizelyAttributes())
421399

422400
guard let variation = pair?.variation else {
423401
throw OptimizelyError.variationUnknown
@@ -561,7 +539,7 @@ open class OptimizelyManager: NSObject {
561539
if attributes != nil {
562540
_attributes = attributes!
563541
}
564-
if let decision = self.decisionService.getVariationForFeature(featureFlag: featureFlag, userId: userId, attributes: _attributes) {
542+
if let decision = self.decisionService.getVariationForFeature(config: config, featureFlag: featureFlag, userId: userId, attributes: _attributes) {
565543
if let featureVariableUsage = decision.variation?.variables?.filter({$0.id == variable.id}).first {
566544
defaultValue = featureVariableUsage.value
567545
}

0 commit comments

Comments
 (0)