refactor decision service, bucketer, and user profile service. The decision service is the only one that needs to contain the others. So, the refactor is a great simplification

Author: thomaszurkan-optimizely

OptimizelySDK/Extensions/OptimizelyManager+Extension.swift

@@ -12,9 +12,7 @@ extension OptimizelyManager {
     func registerServices(sdkKey:String,
                           logger:OPTLogger,
                           eventDispatcher:OPTEventDispatcher,
-                          userProfileService:OPTUserProfileService,
                           datafileHandler:OPTDatafileHandler,
-                          bucketer:OPTBucketer,
                           decisionService:OPTDecisionService,
                           notificationCenter:OPTNotificationCenter) {
         // bind it as a non-singleton.  so, we will create an instance anytime injected.
@@ -26,10 +24,6 @@ extension OptimizelyManager {
         // this is bound a reusable singleton. so, if we re-initalize, we will keep this.
         HandlerRegistryService.shared.registerBinding(binder:Binder<OPTNotificationCenter>(service: OPTNotificationCenter.self).singetlon().reInitializeStrategy(strategy: .reUse).using(instance:notificationCenter).sdkKey(key: sdkKey))
 
-        // this is a singleton but it has a reIntializeStrategy of reCreate.  So, we create a new
-        // instance on re-initialize.
-        HandlerRegistryService.shared.registerBinding(binder:Binder<OPTBucketer>(service: OPTBucketer.self).singetlon().using(instance:bucketer).sdkKey(key: sdkKey))
-        
         // the decision service is also a singleton that will reCreate on re-initalize
         HandlerRegistryService.shared.registerBinding(binder:Binder<OPTDecisionService>(service: OPTDecisionService.self).singetlon().using(instance:decisionService).sdkKey(key: sdkKey))
 
@@ -39,9 +33,5 @@ extension OptimizelyManager {
         // This is a singleton and might be a good candidate for reuse.  The handler supports mulitple
         // sdk keys without having to be created for every key.
         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))
-        
-        // the user profile service is also a singleton using eh passed in version.
-        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))
-        
     }
 }

OptimizelySDK/Implementation/DefaultBucketer.swift

@@ -22,25 +22,14 @@ class DefaultBucketer : OPTBucketer {
     let MAX_HASH_SEED:UInt64 = 1
     var MAX_HASH_VALUE:UInt64?
 
-    private var config:ProjectConfig!
     private lazy var logger = HandlerRegistryService.shared.injectLogger()
 
-    internal required init(config:ProjectConfig) {
-        self.config = config
-
-        MAX_HASH_VALUE = MAX_HASH_SEED << 32
-    }
-    
     // [Jae]: let be configured after initialized (with custom DecisionHandler set up on OPTManger initialization)
     init() {
         MAX_HASH_VALUE = MAX_HASH_SEED << 32
     }
 
-    func initialize(config:ProjectConfig) {
-        self.config = config
-    }
-
-    func bucketToExperiment(group: Group, bucketingId: String) -> Experiment? {
+    func bucketToExperiment(config:ProjectConfig, group: Group, bucketingId: String) -> Experiment? {
         let hashId = makeHashIdFromBucketingId(bucketingId: bucketingId, entityId: group.id)
         let bucketValue = self.generateBucketValue(bucketingId: hashId)
 
@@ -73,7 +62,7 @@ class DefaultBucketer : OPTBucketer {
         return nil
     }
 
-    func bucketExperiment(experiment: Experiment, bucketingId: String) -> Variation? {
+    func bucketExperiment(config:ProjectConfig, experiment: Experiment, bucketingId: String) -> Variation? {
         var ok = true
         // check for mutex
         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 {
             case .overlapping:
                 break;
             case .random:
-                let mutexExperiment = bucketToExperiment(group: group, bucketingId: bucketingId)
+                let mutexExperiment = bucketToExperiment(config: config, group: group, bucketingId: bucketingId)
                 if let mutexExperiment = mutexExperiment, mutexExperiment.id == experiment.id {
                     ok = true
                 }

OptimizelySDK/Implementation/DefaultDecisionService.swift

@@ -17,26 +17,16 @@
 import Foundation
 
 class DefaultDecisionService : OPTDecisionService {
-    var config:ProjectConfig!
-    var bucketer:OPTBucketer!
-    var userProfileService:OPTUserProfileService!
-
-    internal required init(config:ProjectConfig, bucketer:OPTBucketer, userProfileService:OPTUserProfileService) {
-        self.config = config
-        self.bucketer = bucketer
-        self.userProfileService = userProfileService
-    }
 
-    // [Jae]: let be configured after initialized (with custom DecisionHandler set up on OPTManger initialization)
-    init() {}
+    var bucketer:OPTBucketer
+    var userProfileService:OPTUserProfileService
 
-    func initialize(config:ProjectConfig, bucketer:OPTBucketer, userProfileService:OPTUserProfileService) {
-        self.config = config
+    init(bucketer: OPTBucketer, userProfileService:OPTUserProfileService) {
         self.bucketer = bucketer
         self.userProfileService = userProfileService
     }
-    
-    func getVariation(userId:String, experiment: Experiment, attributes: OptimizelyAttributes) -> Variation? {
+
+    func getVariation(config:ProjectConfig, userId:String, experiment: Experiment, attributes: OptimizelyAttributes) -> Variation? {
         let experimentId = experiment.id;
 
         // Acquire bucketingId .
@@ -68,12 +58,13 @@ class DefaultDecisionService : OPTDecisionService {
         var bucketedVariation:Variation?
         // ---- check if the user passes audience targeting before bucketing ----
         if let result = isInExperiment(
+            config: config,
             experiment:experiment,
             userId:userId,
             attributes:attributes), result == true {
 
             // bucket user into a variation
-            bucketedVariation = bucketer.bucketExperiment(experiment: experiment, bucketingId:bucketingId)
+            bucketedVariation = bucketer.bucketExperiment(config: config, experiment: experiment, bucketingId:bucketingId)
 
             if let bucketedVariation = bucketedVariation {
                 // save to user profile
@@ -85,7 +76,7 @@ class DefaultDecisionService : OPTDecisionService {
 
     }
 
-    func isInExperiment(experiment:Experiment, userId:String, attributes: OptimizelyAttributes) -> Bool? {
+    func isInExperiment(config:ProjectConfig, experiment:Experiment, userId:String, attributes: OptimizelyAttributes) -> Bool? {
 
         if let conditions = experiment.audienceConditions {
             switch conditions {
@@ -119,45 +110,46 @@ class DefaultDecisionService : OPTDecisionService {
         return true
     }
 
-    func getExperimentInGroup(group:Group, bucketingId:String) -> Experiment? {
-        let experiment = bucketer.bucketToExperiment(group:group, bucketingId:bucketingId)
+    func getExperimentInGroup(config:ProjectConfig, group:Group, bucketingId:String) -> Experiment? {
+        let experiment = bucketer.bucketToExperiment(config: config, group:group, bucketingId:bucketingId)
         if let _ = experiment {
             // log
         }
 
         return experiment;
     }
 
-     func getVariationForFeature(featureFlag:FeatureFlag, userId:String, attributes: OptimizelyAttributes) -> (experiment:Experiment?, variation:Variation?)? {
+    func getVariationForFeature(config:ProjectConfig, featureFlag:FeatureFlag, userId:String, attributes: OptimizelyAttributes) -> (experiment:Experiment?, variation:Variation?)? {
         //Evaluate in this order:
 
         //1. Attempt to bucket user into experiment using feature flag.
         // Check if the feature flag is under an experiment and the the user is bucketed into one of these experiments
-        if let variation = getVariationForFeatureExperiment(featureFlag: featureFlag, userId:userId, attributes:attributes) {
+        if let variation = getVariationForFeatureExperiment(config: config, featureFlag: featureFlag, userId:userId, attributes:attributes) {
             return variation
         }
 
         //2. Attempt to bucket user into rollout using the feature flag.
         // Check if the feature flag has rollout and the user is bucketed into one of it's rules
-        if let variation = getVariationForFeatureRollout(featureFlag: featureFlag, userId:userId, attributes:attributes) {
+        if let variation = getVariationForFeatureRollout(config: config, featureFlag: featureFlag, userId:userId, attributes:attributes) {
             return (nil, variation)
         }
 
         return nil;
 
     }
 
-    func getVariationForFeatureGroup(featureFlag: FeatureFlag,
+    func getVariationForFeatureGroup(config:ProjectConfig,
+                                     featureFlag: FeatureFlag,
                                      groupId: String,
                                      userId: String,
                                      attributes: OptimizelyAttributes) -> (experiment:Experiment?, variation:Variation?)? {
 
         let bucketing_id = getBucketingId(userId:userId, attributes:attributes)
         if let group = config.project.groups.filter({$0.id == groupId}).first {
 
-            if let experiment = getExperimentInGroup(group: group, bucketingId:bucketing_id),
+            if let experiment = getExperimentInGroup(config: config, group: group, bucketingId:bucketing_id),
                 featureFlag.experimentIds.contains(experiment.id),
-                let variation = getVariation(userId:userId, experiment:experiment, attributes:attributes) {
+                let variation = getVariation(config: config, userId:userId, experiment:experiment, attributes:attributes) {
                   // log
                     return (experiment,variation)
             }
@@ -169,7 +161,8 @@ class DefaultDecisionService : OPTDecisionService {
         return nil
     }
 
-    func getVariationForFeatureExperiment(featureFlag: FeatureFlag,
+    func getVariationForFeatureExperiment(config:ProjectConfig,
+                                          featureFlag: FeatureFlag,
                                           userId: String,
                                           attributes: OptimizelyAttributes) -> (experiment:Experiment?, variation:Variation?)? {
 
@@ -178,14 +171,15 @@ class DefaultDecisionService : OPTDecisionService {
         // Evaluate each experiment ID and return the first bucketed experiment variation
         for experimentId in experimentIds {
             if let experiment = config.getExperiment(id: experimentId),
-                let variation = getVariation(userId: userId, experiment: experiment, attributes: attributes) {
+                let variation = getVariation(config: config, userId: userId, experiment: experiment, attributes: attributes) {
                     return (experiment,variation)
             }
         }
         return nil;
     }
 
-    func getVariationForFeatureRollout(featureFlag: FeatureFlag,
+    func getVariationForFeatureRollout(config:ProjectConfig,
+                                       featureFlag: FeatureFlag,
                                        userId: String,
                                        attributes: OptimizelyAttributes) -> Variation? {
 
@@ -200,16 +194,16 @@ class DefaultDecisionService : OPTDecisionService {
         let rolloutRules = rollout.experiments
         // Evaluate all rollout rules except for last one
         for experiment in rolloutRules[0..<rolloutRules.count.advanced(by: -1)] {
-            if isInExperiment(experiment: experiment, userId: userId, attributes: attributes) ?? false {
-                if let variation = bucketer.bucketExperiment(experiment: experiment, bucketingId: bucketingId) {
+            if isInExperiment(config: config, experiment: experiment, userId: userId, attributes: attributes) ?? false {
+                if let variation = bucketer.bucketExperiment(config:config, experiment: experiment, bucketingId: bucketingId) {
                     return variation
                 }
             }
         }
         // Evaluate fall back rule / last rule now
         let experiment = rolloutRules[rolloutRules.count - 1];
-        if isInExperiment(experiment: experiment, userId: userId, attributes: attributes) ?? false {
-            return bucketer.bucketExperiment(experiment: experiment, bucketingId: bucketingId)
+        if isInExperiment(config: config, experiment: experiment, userId: userId, attributes: attributes) ?? false {
+            return bucketer.bucketExperiment(config: config, experiment: experiment, bucketingId: bucketingId)
         }
 
         return nil

OptimizelySDK/Optimizely/OptimizelyManager.swift

@@ -30,20 +30,10 @@ open class OptimizelyManager: NSObject {
             return HandlerRegistryService.shared.injectEventDispatcher(sdkKey: self.sdkKey)!
         }
     }
-    var userProfileService: OPTUserProfileService {
-        get {
-            return HandlerRegistryService.shared.injectUserProfileService(sdkKey: self.sdkKey)!
-        }
-    }
     let periodicDownloadInterval: Int
 
     // MARK: - Default Services
 
-    var bucketer: OPTBucketer {
-        get {
-            return HandlerRegistryService.shared.injectBucketer(sdkKey: self.sdkKey)!
-        }
-    }
     var decisionService: OPTDecisionService {
         get {
             return HandlerRegistryService.shared.injectDecisionService(sdkKey: self.sdkKey)!
@@ -82,13 +72,13 @@ open class OptimizelyManager: NSObject {
 
         super.init()
 
+        let bucketer = DefaultBucketer()
+        let userProfileService = userProfileService ?? DefaultUserProfileService()
         self.registerServices(sdkKey: sdkKey,
                               logger: logger ?? DefaultLogger(),
                               eventDispatcher: eventDispatcher ?? DefaultEventDispatcher.sharedInstance,
-                              userProfileService: userProfileService ?? DefaultUserProfileService(),
                               datafileHandler: DefaultDatafileHandler(),
-                              bucketer: DefaultBucketer(),
-                              decisionService: DefaultDecisionService(),
+                              decisionService: DefaultDecisionService(bucketer: bucketer, userProfileService: userProfileService),
                               notificationCenter: DefaultNotificationCenter())
 
     }
@@ -158,11 +148,6 @@ open class OptimizelyManager: NSObject {
             // 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.
             guard let config = self.config else { throw OptimizelyError.dataFileInvalid }
 
-            // TODO: fix these to throw errors
-            bucketer.initialize(config: config)
-            decisionService.initialize(config: config,
-                                       bucketer: self.bucketer,
-                                       userProfileService: self.userProfileService)
             if periodicDownloadInterval > 0 {
                 datafileHandler.stopPeriodicUpdates(sdkKey: self.sdkKey)
                 datafileHandler.startPeriodicUpdates(sdkKey: self.sdkKey, updateInterval: periodicDownloadInterval) { data in
@@ -181,12 +166,6 @@ open class OptimizelyManager: NSObject {
                                 HandlerRegistryService.shared.reInitializeComponent(service: component, sdkKey: self.sdkKey)
                             }
 
-                            // now reinitialize with the new config.
-                            self.bucketer.initialize(config: config)
-                            self.decisionService.initialize(config: config,
-                                                       bucketer: self.bucketer,
-                                                       userProfileService: self.userProfileService)
-
                         }
 
                         self.notificationCenter.sendNotifications(type:
@@ -347,7 +326,7 @@ open class OptimizelyManager: NSObject {
         }
 
         // fix DecisionService to throw error
-        guard let variation = decisionService.getVariation(userId: userId, experiment: experiment, attributes: attributes ?? OptimizelyAttributes()) else {
+        guard let variation = decisionService.getVariation(config: config, userId: userId, experiment: experiment, attributes: attributes ?? OptimizelyAttributes()) else {
             throw OptimizelyError.variationUnknown
         }
 
@@ -417,7 +396,7 @@ open class OptimizelyManager: NSObject {
         }
 
         // fix DecisionService to throw error
-        let pair = decisionService.getVariationForFeature(featureFlag: featureFlag, userId: userId, attributes: attributes ?? OptimizelyAttributes())
+        let pair = decisionService.getVariationForFeature(config: config, featureFlag: featureFlag, userId: userId, attributes: attributes ?? OptimizelyAttributes())
 
         guard let variation = pair?.variation else {
             throw OptimizelyError.variationUnknown
@@ -561,7 +540,7 @@ open class OptimizelyManager: NSObject {
         if attributes != nil {
             _attributes = attributes!
         }
-        if let decision = self.decisionService.getVariationForFeature(featureFlag: featureFlag, userId: userId, attributes: _attributes) {
+        if let decision = self.decisionService.getVariationForFeature(config: config, featureFlag: featureFlag, userId: userId, attributes: _attributes) {
             if let featureVariableUsage = decision.variation?.variables?.filter({$0.id == variable.id}).first {
                 defaultValue = featureVariableUsage.value
             }