Skip to content

Revert local holdout feature gate logic #637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
muzahidul-opti wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Revert local holdout feature gate logic #637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions OptimizelySwiftSDK.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2121,12 +2121,6 @@
98AC985F2DBA6721001405DD /* OptimizelyUserContextTests_Decide_With_Holdouts_Reasons.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98AC985D2DBA6721001405DD /* OptimizelyUserContextTests_Decide_With_Holdouts_Reasons.swift */; };
98C2DF242F900669003F2443 /* DecisionServiceTests_LocalHoldouts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98C2DF232F900669003F2443 /* DecisionServiceTests_LocalHoldouts.swift */; };
98C2DF252F900669003F2443 /* DecisionServiceTests_LocalHoldouts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98C2DF232F900669003F2443 /* DecisionServiceTests_LocalHoldouts.swift */; };
0921CDD1BC7F41F59B2D4CC3 /* FeatureGateTests_LocalHoldouts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C5D873BB4C44181B07ECC15 /* FeatureGateTests_LocalHoldouts.swift */; };
BB21E5C3E29D4F0C892E523C /* FeatureGateTests_LocalHoldouts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C5D873BB4C44181B07ECC15 /* FeatureGateTests_LocalHoldouts.swift */; };
98C2DF752FA8D055003F2443 /* BaseHoldoutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98C2DF742FA8D055003F2443 /* BaseHoldoutTests.swift */; };
98C2DF762FA8D055003F2443 /* BaseHoldoutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98C2DF742FA8D055003F2443 /* BaseHoldoutTests.swift */; };
98C2DF772FA8D1FF003F2443 /* BaseHoldoutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98C2DF742FA8D055003F2443 /* BaseHoldoutTests.swift */; };
98C2DF782FA8D207003F2443 /* BaseHoldoutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98C2DF742FA8D055003F2443 /* BaseHoldoutTests.swift */; };
98D5AE842DBB91C0000D5844 /* OptimizelyUserContextTests_Decide_Holdouts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98D5AE832DBB91C0000D5844 /* OptimizelyUserContextTests_Decide_Holdouts.swift */; };
98D5AE852DBB91C0000D5844 /* OptimizelyUserContextTests_Decide_Holdouts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98D5AE832DBB91C0000D5844 /* OptimizelyUserContextTests_Decide_Holdouts.swift */; };
98F28A1D2E01940500A86546 /* Cmab.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98F28A1C2E01940500A86546 /* Cmab.swift */; };
Expand Down Expand Up @@ -2644,8 +2638,6 @@
98AC98482DB8FC29001405DD /* DecisionServiceTests_Holdouts.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DecisionServiceTests_Holdouts.swift; sourceTree = "<group>"; };
98AC985D2DBA6721001405DD /* OptimizelyUserContextTests_Decide_With_Holdouts_Reasons.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OptimizelyUserContextTests_Decide_With_Holdouts_Reasons.swift; sourceTree = "<group>"; };
98C2DF232F900669003F2443 /* DecisionServiceTests_LocalHoldouts.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DecisionServiceTests_LocalHoldouts.swift; sourceTree = "<group>"; };
7C5D873BB4C44181B07ECC15 /* FeatureGateTests_LocalHoldouts.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeatureGateTests_LocalHoldouts.swift; sourceTree = "<group>"; };
98C2DF742FA8D055003F2443 /* BaseHoldoutTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BaseHoldoutTests.swift; sourceTree = "<group>"; };
98D5AE832DBB91C0000D5844 /* OptimizelyUserContextTests_Decide_Holdouts.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OptimizelyUserContextTests_Decide_Holdouts.swift; sourceTree = "<group>"; };
98F28A1C2E01940500A86546 /* Cmab.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Cmab.swift; sourceTree = "<group>"; };
98F28A2D2E01968000A86546 /* CmabTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CmabTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3177,7 +3169,6 @@
6E75198022C5211100B2B157 /* DecisionServiceTests_Experiments.swift */,
6E75199122C5211100B2B157 /* DecisionServiceTests_Features.swift */,
98AC98482DB8FC29001405DD /* DecisionServiceTests_Holdouts.swift */,
98C2DF742FA8D055003F2443 /* BaseHoldoutTests.swift */,
6E75199422C5211100B2B157 /* DecisionServiceTests_Others.swift */,
6E75198622C5211100B2B157 /* DecisionServiceTests_UserProfiles.swift */,
6E75198822C5211100B2B157 /* DefaultLoggerTests.swift */,
Expand All @@ -3190,7 +3181,6 @@
6E75198B22C5211100B2B157 /* NotificationCenterTests.swift */,
84861810286D0B8900B7F41B /* OdpEventManagerTests.swift */,
98C2DF232F900669003F2443 /* DecisionServiceTests_LocalHoldouts.swift */,
7C5D873BB4C44181B07ECC15 /* FeatureGateTests_LocalHoldouts.swift */,
8486180E286D0B8900B7F41B /* OdpManagerTests.swift */,
8486180D286D0B8900B7F41B /* OdpSegmentManagerTests.swift */,
98F28A512E02E81500A86546 /* CMABClientTests.swift */,
Expand Down Expand Up @@ -5134,7 +5124,6 @@
6E75192D22C520D500B2B157 /* DataStoreQueueStack.swift in Sources */,
6E7516D322C520D400B2B157 /* OPTLogger.swift in Sources */,
6E75180122C520D400B2B157 /* DataStoreUserDefaults.swift in Sources */,
98C2DF752FA8D055003F2443 /* BaseHoldoutTests.swift in Sources */,
98F28A672E05220300A86546 /* CmabServiceTests.swift in Sources */,
98AC98472DB7B762001405DD /* BucketTests_HoldoutToVariation.swift in Sources */,
6E75175722C520D400B2B157 /* LogMessage.swift in Sources */,
Expand Down Expand Up @@ -5207,7 +5196,6 @@
6E75187922C520D400B2B157 /* Variation.swift in Sources */,
6E75191522C520D500B2B157 /* BackgroundingCallbacks.swift in Sources */,
98C2DF242F900669003F2443 /* DecisionServiceTests_LocalHoldouts.swift in Sources */,
0921CDD1BC7F41F59B2D4CC3 /* FeatureGateTests_LocalHoldouts.swift in Sources */,
6E75195D22C520D500B2B157 /* OPTBucketer.swift in Sources */,
6E9B117622C5487100C22D81 /* DatafileHandlerTests.swift in Sources */,
84E2E97F2855875E001114AB /* OdpEventManager.swift in Sources */,
Expand Down Expand Up @@ -5306,7 +5294,6 @@
6E7516EC22C520D400B2B157 /* OPTEventDispatcher.swift in Sources */,
6E75181A22C520D400B2B157 /* DataStoreQueueStackImpl.swift in Sources */,
6EF8DE2624BD1BB2008B9488 /* OptimizelyDecideOption.swift in Sources */,
98C2DF782FA8D207003F2443 /* BaseHoldoutTests.swift in Sources */,
8464087E28130D3200CCF97D /* Integration.swift in Sources */,
6E9B119722C5488300C22D81 /* ConditionLeafTests.swift in Sources */,
6E75184A22C520D400B2B157 /* Event.swift in Sources */,
Expand Down Expand Up @@ -5441,7 +5428,6 @@
6E75193322C520D500B2B157 /* OPTDataStore.swift in Sources */,
84861811286D0B8900B7F41B /* OdpSegmentManagerTests.swift in Sources */,
6E7517EF22C520D400B2B157 /* DataStoreMemory.swift in Sources */,
98C2DF762FA8D055003F2443 /* BaseHoldoutTests.swift in Sources */,
98F28A682E05220300A86546 /* CmabServiceTests.swift in Sources */,
6E75194B22C520D500B2B157 /* OPTDatafileHandler.swift in Sources */,
6E75195722C520D500B2B157 /* OPTBucketer.swift in Sources */,
Expand Down Expand Up @@ -5514,7 +5500,6 @@
6E7516FD22C520D400B2B157 /* OptimizelyLogLevel.swift in Sources */,
6E75187322C520D400B2B157 /* Variation.swift in Sources */,
98C2DF252F900669003F2443 /* DecisionServiceTests_LocalHoldouts.swift in Sources */,
BB21E5C3E29D4F0C892E523C /* FeatureGateTests_LocalHoldouts.swift in Sources */,
6E7517E322C520D400B2B157 /* DefaultDecisionService.swift in Sources */,
6E75179922C520D400B2B157 /* DataStoreQueueStackImpl+Extension.swift in Sources */,
6E9B115C22C5486E00C22D81 /* DatafileHandlerTests.swift in Sources */,
Expand Down Expand Up @@ -5587,7 +5572,6 @@
6E75193522C520D500B2B157 /* OPTDataStore.swift in Sources */,
6EC6DD4824ABF89B0017D296 /* OptimizelyUserContext.swift in Sources */,
6E75182122C520D400B2B157 /* BatchEventBuilder.swift in Sources */,
98C2DF772FA8D1FF003F2443 /* BaseHoldoutTests.swift in Sources */,
983F81852F801E7500CDBC8D /* FeatureRolloutTests.swift in Sources */,
6E86CEA924FDC847005DAFED /* OptimizelyUserContext+ObjC.swift in Sources */,
6E9B118322C5488100C22D81 /* UserAttributeTests_Evaluate.swift in Sources */,
Expand Down
52 changes: 24 additions & 28 deletions Sources/Implementation/DefaultDecisionService.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,20 +658,18 @@ class DefaultDecisionService: OPTDecisionService {
}

// check local holdouts targeting this rule
if FeatureGates.localHoldouts {
let localHoldouts = config.getHoldoutsForRule(ruleId: rule.id)
for holdout in localHoldouts {
let holdoutDecision = getVariationForHoldout(config: config,
flagKey: flagKey,
holdout: holdout,
user: user,
options: options)
reasons.merge(holdoutDecision.reasons)
if let variation = holdoutDecision.result {
// User is in holdout — return holdout variation immediately, skip this rule
let variationDecision = VariationDecision(variation: variation, holdout: holdout)
return DecisionResponse(result: variationDecision, reasons: reasons)
}
let localHoldouts = config.getHoldoutsForRule(ruleId: rule.id)
for holdout in localHoldouts {
let holdoutDecision = getVariationForHoldout(config: config,
flagKey: flagKey,
holdout: holdout,
user: user,
options: options)
reasons.merge(holdoutDecision.reasons)
if let variation = holdoutDecision.result {
// User is in holdout — return holdout variation immediately, skip this rule
let variationDecision = VariationDecision(variation: variation, holdout: holdout)
return DecisionResponse(result: variationDecision, reasons: reasons)
}
}

Expand Down Expand Up @@ -718,20 +716,18 @@ class DefaultDecisionService: OPTDecisionService {
}

// check local holdouts targeting this delivery rule
if FeatureGates.localHoldouts {
let localHoldouts = config.getHoldoutsForRule(ruleId: rule.id)
for holdout in localHoldouts {
let holdoutDecision = getVariationForHoldout(config: config,
flagKey: flagKey,
holdout: holdout,
user: user,
options: options)
reasons.merge(holdoutDecision.reasons)
if let variation = holdoutDecision.result {
// User is in holdout — return holdout variation with holdout info
let decision = DeliveryRuleDecision(variation: variation, skipToEveryoneElse: skipToEveryoneElse, holdout: holdout)
return DecisionResponse(result: decision, reasons: reasons)
}
let localHoldouts = config.getHoldoutsForRule(ruleId: rule.id)
for holdout in localHoldouts {
let holdoutDecision = getVariationForHoldout(config: config,
flagKey: flagKey,
holdout: holdout,
user: user,
options: options)
reasons.merge(holdoutDecision.reasons)
if let variation = holdoutDecision.result {
// User is in holdout — return holdout variation with holdout info
let decision = DeliveryRuleDecision(variation: variation, skipToEveryoneElse: skipToEveryoneElse, holdout: holdout)
return DecisionResponse(result: decision, reasons: reasons)
}
}

Expand Down
4 changes: 0 additions & 4 deletions Sources/Utils/Constants.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@

import Foundation

struct FeatureGates {
static var localHoldouts = false
}

struct Constants {
struct Attributes {
static let reservedBucketIdAttribute = "$opt_bucketing_id"
Expand Down
27 changes: 0 additions & 27 deletions Tests/OptimizelyTests-Common/BaseHoldoutTests.swift
View file
Open in desktop

This file was deleted.

11 changes: 4 additions & 7 deletions Tests/OptimizelyTests-Common/DecisionListenerTest_Holdouts.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import XCTest

class DecisionListenerTests_Holdouts: BaseHoldoutTests {
class DecisionListenerTests_Holdouts: XCTestCase {
let kUserId = "11111"
var optimizely: OptimizelyClient!
var notificationCenter: OPTNotificationCenter!
Expand Down Expand Up @@ -61,12 +61,13 @@ class DecisionListenerTests_Holdouts: BaseHoldoutTests {

override func setUp() {
super.setUp()

optimizely = OptimizelyClient(sdkKey: OTUtils.randomSdkKey,
eventDispatcher: eventDispatcher,
userProfileService: OTUtils.createClearUserProfileService())

try! optimizely.start(datafile: OTUtils.loadJSONDatafile("decide_datafile")!)

var holdout = try! OTUtils.model(from: sampleHoldout) as Holdout
// Audience "13389130056" requires "country" = "US"
holdout.audienceIds = ["13389130056"]
Expand All @@ -78,10 +79,6 @@ class DecisionListenerTests_Holdouts: BaseHoldoutTests {

self.notificationCenter = self.optimizely.notificationCenter!
}

override func tearDown() {
super.tearDown()
}

func testDecisionListenerDecideWithUserInHoldout() {
let exp = expectation(description: "x")
Expand Down
7 changes: 2 additions & 5 deletions Tests/OptimizelyTests-Common/DecisionServiceTests_Holdouts.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import XCTest

class DecisionServiceTests_Holdouts: BaseHoldoutTests {
class DecisionServiceTests_Holdouts: XCTestCase {

var optimizely: OptimizelyClient!
var config: ProjectConfig!
Expand Down Expand Up @@ -189,6 +189,7 @@ class DecisionServiceTests_Holdouts: BaseHoldoutTests {

override func setUp() {
super.setUp()

self.optimizely = OTUtils.createOptimizely(datafileName: "empty_datafile",
clearUserProfileService: true)
self.config = self.optimizely.config!
Expand All @@ -211,10 +212,6 @@ class DecisionServiceTests_Holdouts: BaseHoldoutTests {
self.config.holdoutConfig.allHoldouts = [holdout]
}

override func tearDown() {
super.tearDown()
}

}

// MARK: - Test doesMeetAudienceConditions()
Expand Down
7 changes: 2 additions & 5 deletions Tests/OptimizelyTests-Common/DecisionServiceTests_LocalHoldouts.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import XCTest
/// Integration tests for Local Holdouts functionality
/// Tests that local holdouts are correctly evaluated at the rule level
/// and global holdouts are evaluated at the flag level before any rules
class DecisionServiceTests_LocalHoldouts: BaseHoldoutTests {
class DecisionServiceTests_LocalHoldouts: XCTestCase {

var optimizely: OptimizelyClient!
var config: ProjectConfig!
Expand Down Expand Up @@ -52,17 +52,14 @@ class DecisionServiceTests_LocalHoldouts: BaseHoldoutTests {

override func setUp() {
super.setUp()

// Load a real datafile for testing
optimizely = OTUtils.createOptimizely(datafileName: "decide_datafile",
clearUserProfileService: true)
config = optimizely.config!
decisionService = optimizely.decisionService as? DefaultDecisionService
}

override func tearDown() {
super.tearDown()
}

// MARK: - Global Holdouts Tests

func testGlobalHoldout_EvaluatedBeforeAllRules() {
Expand Down
Loading
Loading