cleanup: address review changes

Author: muzahidul-opti

Sources/Data Model/HoldoutConfig.swift

@@ -28,10 +28,6 @@ struct HoldoutConfig {
     private(set) var includedHoldouts: [String: [Holdout]] = [:]
     private(set) var excludedHoldouts: [String: [Holdout]] = [:]
 
-    var cacheWriteCount = 0 // for testing purpose only
-    
-    var logger: OPTLogger = OPTLoggerFactory.getLogger()
-    
     init(allholdouts: [Holdout] = []) {
         self.allHoldouts = allholdouts
         updateHoldoutMapping()
@@ -55,10 +51,7 @@ struct HoldoutConfig {
                 case (true, true):
                     global.append(holdout)
 
-                case (false, false):
-                    logger.e(.holdoutToFlagMappingError)
-                    
-                case (false, true):
+                case (false, _):
                     holdout.includedFlags.forEach { flagId in
                         if var existing = includedHoldouts[flagId] {
                             existing.append(holdout)
@@ -95,6 +88,7 @@ struct HoldoutConfig {
             return holdouts
         }
 
+        // Prioritize global holdouts first 
         var activeHoldouts: [Holdout] = []
 
         let excluded = excludedHoldouts[id] ?? []
@@ -112,7 +106,6 @@ struct HoldoutConfig {
         activeHoldouts += includedHoldouts
 
         flagHoldoutsMap[id] = activeHoldouts
-        cacheWriteCount += 1
 
         return flagHoldoutsMap[id] ?? []
     }

Sources/Utils/LogMessage.swift

@@ -68,7 +68,6 @@ enum LogMessage {
     case failedToAssignValue
     case valueForKeyNotFound(_ key: String)
     case lowPeriodicDownloadInterval
-    case holdoutToFlagMappingError
 }
 
 extension LogMessage: CustomStringConvertible {
@@ -131,7 +130,6 @@ extension LogMessage: CustomStringConvertible {
         case .failedToAssignValue:                                              message = "Value for path could not be assigned to provided type."
         case .valueForKeyNotFound(let key):                                     message = "Value for JSON key (\(key)) not found."
         case .lowPeriodicDownloadInterval:                                      message = "Polling intervals below 30 seconds are not recommended."
-        case .holdoutToFlagMappingError:                                        message = "A holdout cannot have include and exclude flags at the same time."
         }
 
         return message

Tests/OptimizelyTests-DataModel/HoldoutConfigTests.swift

@@ -135,50 +135,21 @@ class HoldoutConfigTests: XCTestCase {
     }
 
     func testGetHoldoutForFlag_shouldUseCacheOnSecondCall() {
-        var holdout: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData)
-        holdout.id = "1"
-        holdout.excludedFlags = ["f1"]
+        var ho1: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData)
+        ho1.id = "h1"
+        ho1.includedFlags = ["f1"]
 
-        var config = HoldoutConfig(allholdouts: [holdout])
+        var config = HoldoutConfig(allholdouts: [ho1])
 
-        _ = config.getHoldoutForFlag(id: "f1")
-        let writeCountAfterFirst = config.cacheWriteCount
+        // Initially no cache
+        XCTAssertEqual(config.flagHoldoutsMap.count, 0)
 
-        _ = config.getHoldoutForFlag(id: "f1")
-        let writeCountAfterSecond = config.cacheWriteCount
+        let _ = config.getHoldoutForFlag(id: "f1")
+        XCTAssertEqual(config.flagHoldoutsMap.count, 1)
 
-        XCTAssertEqual(writeCountAfterFirst, 1)
-        XCTAssertEqual(writeCountAfterSecond, 1, "Second call should not increase cache write count")
+        let cache_v = config.getHoldoutForFlag(id: "f1")
+        XCTAssertEqual(config.flagHoldoutsMap.count, 1)
+        XCTAssertEqual(cache_v, config.flagHoldoutsMap["f1"])
     }
 
-    func testHoldoutWithBothIncludedAndExcludedFlags_shouldLogError() {
-        class ConfigMockLogger: OPTLogger {
-            static var logLevel: OptimizelyLogLevel = .info
-            var expectedLog: String = ""
-            var logFound = false
-            
-            required init() {}
-            
-            func log(level: OptimizelyLogLevel, message: String) {
-                if (message == expectedLog) {
-                    logFound = true
-                }
-            }
-        }
-        
-        var holdout: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData)
-        holdout.id = "invalid"
-        holdout.includedFlags = ["f1"]
-        holdout.excludedFlags = ["f2"]
-        
-        let mockLogger = ConfigMockLogger()
-        mockLogger.expectedLog = LogMessage.holdoutToFlagMappingError.description
-        
-        var config = HoldoutConfig(allholdouts: [])
-        config.logger = mockLogger
-        
-        config.allHoldouts = [holdout]
-        
-        XCTAssertTrue(mockLogger.logFound)
-    }
 }