(chore): cleanup (#173)

* cleanup * more cleanup * remove bang that could cause crash * cleanup evaluate to make it faster * fix condition holder tests and add back into project

Author: thomaszurkan-optimizely

DemoSwiftApp/AppDelegate.swift

@@ -51,7 +51,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
  // - initialize immediately with the given JSON datafile or its cached copy
  // - no network delay, but the local copy is not guaranteed to be in sync with the server experiment settings
 
- initializeOptimizelySDKAsynchronous()
+ initializeOptimizelySDKWithCustomization()
  }
 
  // MARK: - Initialization Examples

OptimizelySDK/Customization/DefaultLogger.swift

@@ -23,8 +23,9 @@ open class DefaultLogger : OPTLogger {
  return _logLevel ?? .info
  }
  set (newLevel){
- if _logLevel == nil {
+ guard let _ = _logLevel else {
  _logLevel = newLevel
+ return
  }
  }
  }

OptimizelySDK/Data Model/Audience/Audience.swift

@@ -63,7 +63,7 @@ struct Audience: Codable, Equatable {
  try container.encode(conditions, forKey: .conditions)
  }
 
- func evaluate(project: ProjectProtocol?, attributes: OptimizelyAttributes?) throws -> Bool {
+ func evaluate(project: Project?, attributes: OptimizelyAttributes?) throws -> Bool {
  return try conditions.evaluate(project: project, attributes: attributes)
  }
 }

OptimizelySDK/Data Model/Audience/ConditionHolder.swift

@@ -61,7 +61,7 @@ enum ConditionHolder: Codable, Equatable {
  }
  }
 
- func evaluate(project: ProjectProtocol?, attributes: OptimizelyAttributes?) throws -> Bool {
+ func evaluate(project: Project?, attributes: OptimizelyAttributes?) throws -> Bool {
  switch self {
  case .logicalOp:
  throw OptimizelyError.conditionInvalidFormat("Logical operation not evaluated")
@@ -77,7 +77,7 @@ enum ConditionHolder: Codable, Equatable {
 
 extension Array where Element == ConditionHolder {
 
- func evaluate(project: ProjectProtocol?, attributes: OptimizelyAttributes?) throws -> Bool {
+ func evaluate(project: Project?, attributes: OptimizelyAttributes?) throws -> Bool {
  guard let firstItem = self.first else {
  throw OptimizelyError.conditionInvalidFormat("Empty condition array")
  }
@@ -94,7 +94,7 @@ extension Array where Element == ConditionHolder {
  }
  }
 
- func evaluate(op: LogicalOp, project: ProjectProtocol?, attributes: OptimizelyAttributes?) throws -> Bool {
+ func evaluate(op: LogicalOp, project: Project?, attributes: OptimizelyAttributes?) throws -> Bool {
  guard self.count > 0 else {
  throw OptimizelyError.conditionInvalidFormat("Empty condition array")
  }

OptimizelySDK/Data Model/Audience/ConditionLeaf.swift

@@ -50,14 +50,14 @@ enum ConditionLeaf: Codable, Equatable {
  }
  }
 
- func evaluate(project: ProjectProtocol?, attributes: OptimizelyAttributes?) throws -> Bool {
+ func evaluate(project: Project?, attributes: OptimizelyAttributes?) throws -> Bool {
  switch self {
  case .audienceId(let id):
- guard let project = project else {
+ guard var project = project else {
  throw OptimizelyError.conditionCannotBeEvaluated("audienceId: \(id)")
  }
 
- return try project.evaluateAudience(audienceId: id, attributes: attributes)
+ return try project.getAudience(id: id)?.evaluate(project: project, attributes: attributes) ?? false
  case .attribute(let userAttribute):
  return try userAttribute.evaluate(attributes: attributes)
  }

OptimizelySDK/Data Model/Group.swift

@@ -26,14 +26,27 @@ struct Group: Codable, Equatable {
  var policy: Policy
  var trafficAllocation: [TrafficAllocation]
  var experiments: [Experiment]
+ 
+ private enum CodingKeys: String, CodingKey {
+ case id
+ case policy
+ case trafficAllocation
+ case experiments
+ }
+ 
+ lazy var experimentMap:[String:Experiment] = {
+ var map:[String:Experiment] = [:]
+ experiments.forEach({map[$0.id] = $0 })
+ return map
+ }()
 }
 
 // MARK: - Utils
 
 extension Group {
 
- func getExperiemnt(id: String) -> Experiment? {
- return experiments.filter { $0.id == id }.first
+ mutating func getExperiment(id: String) -> Experiment? {
+ return self.experimentMap[id]
  }
 
 }

OptimizelySDK/Data Model/Project.swift

@@ -16,10 +16,6 @@
 
 import Foundation
 
-protocol ProjectProtocol {
- func evaluateAudience(audienceId: String, attributes: OptimizelyAttributes?) throws -> Bool
-}
-
 //[REF]: datafile schema
 // https://github.com/optimizely/optimizely/blob/43454b726a2a8aab7dcd953999cf8e1902b09d4d/src/www/services/datafile_generator/schema.json
 
@@ -42,27 +38,40 @@ struct Project: Codable, Equatable {
  var typedAudiences: [Audience]?
  var featureFlags: [FeatureFlag]
  var botFiltering: Bool?
-}
-
-extension Project: ProjectProtocol {
 
- func evaluateAudience(audienceId: String, attributes: OptimizelyAttributes?) throws -> Bool {
- guard let audience = getAudience(id: audienceId) else {
- throw OptimizelyError.conditionNoMatchingAudience(audienceId)
- }
- 
- return try audience.evaluate(project: self, attributes: attributes)
+ private enum CodingKeys: String, CodingKey {
+ case version
+ case projectId
+ case experiments
+ case audiences
+ case groups
+ case attributes
+ case accountId
+ case events
+ case revision
+ case anonymizeIP
+ case rollouts
+ case typedAudiences
+ case featureFlags
+ case botFiltering
  }
 
+ lazy var audienceMap:[String:Audience] = {
+ var map:[String:Audience] = [:]
+ audiences.forEach({map[$0.id] = $0 })
+ typedAudiences?.forEach({map[$0.id] = $0})
+ return map
+ }()
+
+
 }
 
 // MARK: - Utils
 
 extension Project {
 
- func getAudience(id: String) -> Audience? {
- return typedAudiences?.filter{ $0.id == id }.first ??
- audiences.filter{ $0.id == id }.first
+ mutating func getAudience(id: String) -> Audience? {
+ return audienceMap[id]
  }
 
 }

OptimizelySDK/Implementation/DefaultBucketer.swift

@@ -33,7 +33,13 @@ class DefaultBucketer : OPTBucketer {
 
  // check for mutex
 
- let group = config.project.groups.filter{ $0.getExperiemnt(id: experiment.id) != nil }.first
+ let group = config.project.groups.filter({
+ var g = $0
+ guard let _ = g.experimentMap[experiment.id] else {
+ return false
+ }
+ return true
+ }).first
 
  if let group = group {
  switch group.policy {
@@ -79,17 +85,12 @@ class DefaultBucketer : OPTBucketer {
  return nil;
  }
 
- for trafficAllocation in group.trafficAllocation {
- if bucketValue <= trafficAllocation.endOfRange {
- let experimentId = trafficAllocation.entityId;
- 
- // propagate errors and logs for unknown experiment
- if let experiment = config.getExperiment(id: experimentId) {
- return experiment
- } else {
- logger?.e(.userBucketedIntoInvalidExperiment(experimentId))
- return nil
- }
+ for trafficAllocation in group.trafficAllocation where bucketValue <= trafficAllocation.endOfRange {
+ if let experiment = config.getExperiment(id: trafficAllocation.entityId) {
+ return experiment
+ } else {
+ logger?.e(.userBucketedIntoInvalidExperiment(trafficAllocation.entityId))
+ return nil
  }
  }
 
@@ -106,18 +107,14 @@ class DefaultBucketer : OPTBucketer {
  return nil
  }
 
- for trafficAllocation in experiment.trafficAllocation {
- if (bucketValue <= trafficAllocation.endOfRange) {
- let variationId = trafficAllocation.entityId;
-
+ for trafficAllocation in experiment.trafficAllocation where (bucketValue <= trafficAllocation.endOfRange) {
  // propagate errors and logs for unknown variation
- if let variation = experiment.getVariation(id: variationId) {
+ if let variation = experiment.getVariation(id: trafficAllocation.entityId) {
  return variation
  } else {
- logger?.e(.userBucketedIntoInvalidVariation(variationId))
+ logger?.e(.userBucketedIntoInvalidVariation(trafficAllocation.entityId))
  return nil
  }
- }
  }
 
  return nil;

OptimizelySDK/Implementation/DefaultDecisionService.swift

@@ -63,50 +63,50 @@ class DefaultDecisionService : OPTDecisionService {
  return variation
  }
 
- var bucketedVariation:Variation?
  // ---- check if the user passes audience targeting before bucketing ----
- if isInExperiment(config: config, experiment:experiment, userId:userId, attributes:attributes) {
- // bucket user into a variation
- bucketedVariation = bucketer.bucketExperiment(config: config, experiment: experiment, bucketingId: bucketingId)
- 
- if let bucketedVariation = bucketedVariation {
+ if isInExperiment(config: config, experiment:experiment, userId:userId, attributes:attributes), let bucketedVariation = bucketer.bucketExperiment(config: config, experiment: experiment, bucketingId: bucketingId) {
  // save to user profile
-  self.saveProfile(userId: userId, experimentId: experimentId, variationId: bucketedVariation.id)
- }
+ self.saveProfile(userId: userId, experimentId: experimentId, variationId: bucketedVariation.id)
+ return bucketedVariation
  } else {
  logger?.i(.userNotInExperiment(userId, experiment.key))
  }
 
- return bucketedVariation;
+ return nil
  }
 
  func isInExperiment(config:ProjectConfig, experiment:Experiment, userId:String, attributes: OptimizelyAttributes) -> Bool {
- 
+ 
+ func innerEvaluate(conditions:ConditionHolder) throws -> Bool {
+ var result = false
+ switch conditions {
+ case .array(let arrConditions):
+ if arrConditions.count > 0 {
+ result = try conditions.evaluate(project: config.project, attributes: attributes)
+ } else {
+ // empty conditions (backward compatibility with "audienceIds" is ignored if exists even though empty
+ result = true
+ }
+ case .leaf:
+ result = try conditions.evaluate(project: config.project, attributes: attributes)
+ default:
+ result = true
+ }
+ return result
+ }
  var result = true // success as default (no condition, etc)
 
  do {
  if let conditions = experiment.audienceConditions {
- switch conditions {
- case .array(let arrConditions):
- if arrConditions.count > 0 {
- result = try conditions.evaluate(project: config.project, attributes: attributes)
- } else {
- // empty conditions (backward compatibility with "audienceIds" is ignored if exists even though empty
- result = true
- }
- case .leaf:
- result = try conditions.evaluate(project: config.project, attributes: attributes)
- default:
- result = true
- }
+ result = try innerEvaluate(conditions: conditions)
  }
  // backward compatibility with audiencIds list
  else if experiment.audienceIds.count > 0 {
  var holder = [ConditionHolder]()
  holder.append(.logicalOp(.or))
- for id in experiment.audienceIds {
- holder.append(.leaf(.audienceId(id)))
- }
+ experiment.audienceIds.forEach({
+ holder.append(.leaf(.audienceId($0)))
+ })
 
  result = try holder.evaluate(project: config.project, attributes: attributes)
  }