Skip to content

Commit e976715

Browse files
Mat001claude
andauthored
[AI-FSSDK] [FSSDK-12369] Add local holdouts support to Go SDK (#451)
* [AI-FSSDK] [FSSDK-12369] Add local holdouts support to Go SDK * [FSSDK-12369] Add local holdout tests and fix cyclomatic complexity - Add rollout service tests for local holdout in regular and fallback rules, and FD-beats-local-holdout ordering enforcement - Add feature experiment service FD-beats-100%-local-holdout enforcement test - Add GetHoldoutsForRule config tests (previously untested) - Extract checkForLocalHoldout closure to getLocalHoldoutDecision method to bring GetDecision cyclomatic complexity from 18 to 16 (lint fix) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * trigger CI * [FSSDK-12369] Address reviewer feedback: remove flagHoldoutsMap, rename HoldoutService methods - Remove flagHoldoutsMap from mapper, config struct, and config.go - Remove GetHoldoutsForFlag from ProjectConfig interface and DatafileProjectConfig - MapHoldouts now returns globalHoldouts []entities.Holdout (not per-flag map) - Rename HoldoutService.GetDecision → GetGlobalDecision - Rename HoldoutService.GetDecisionForRule → GetLocalDecisionForRule - Update CompositeFeatureService to call GetGlobalDecision explicitly - Update all call sites and tests to match new API Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 36f5b01 commit e976715

18 files changed

Lines changed: 886 additions & 147 deletions

pkg/cmab/service_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,16 @@ func (m *MockProjectConfig) GetHoldoutsForFlag(featureKey string) []entities.Hol
235235
return args.Get(0).([]entities.Holdout)
236236
}
237237

238+
func (m *MockProjectConfig) GetGlobalHoldouts() []entities.Holdout {
239+
args := m.Called()
240+
return args.Get(0).([]entities.Holdout)
241+
}
242+
243+
func (m *MockProjectConfig) GetHoldoutsForRule(ruleID string) []entities.Holdout {
244+
args := m.Called(ruleID)
245+
return args.Get(0).([]entities.Holdout)
246+
}
247+
238248
type CmabServiceTestSuite struct {
239249
suite.Suite
240250
mockClient *MockCmabClient

pkg/config/datafileprojectconfig/config.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@ type DatafileProjectConfig struct {
6262
flagVariationsMap map[string][]entities.Variation
6363
holdouts []entities.Holdout
6464
holdoutIDMap map[string]entities.Holdout
65-
flagHoldoutsMap map[string][]entities.Holdout
65+
// ruleHoldoutsMap maps rule IDs to local holdouts targeting those rules
66+
ruleHoldoutsMap map[string][]entities.Holdout
67+
// globalHoldouts holds only global holdouts (IncludedRules == nil)
68+
globalHoldouts []entities.Holdout
6669
}
6770

6871
// GetDatafile returns a string representation of the environment's datafile
@@ -284,9 +287,16 @@ func (c DatafileProjectConfig) GetRegion() string {
284287
return c.region
285288
}
286289

287-
// GetHoldoutsForFlag returns all holdouts applicable to the given feature flag
288-
func (c DatafileProjectConfig) GetHoldoutsForFlag(featureKey string) []entities.Holdout {
289-
if holdouts, exists := c.flagHoldoutsMap[featureKey]; exists {
290+
// GetGlobalHoldouts returns all global holdouts (those with IncludedRules == nil).
291+
// These are evaluated at flag level, before any per-rule evaluation.
292+
func (c DatafileProjectConfig) GetGlobalHoldouts() []entities.Holdout {
293+
return c.globalHoldouts
294+
}
295+
296+
// GetHoldoutsForRule returns all local holdouts that target the given rule ID.
297+
// These are evaluated per-rule, after forced decisions, before audience/traffic checks.
298+
func (c DatafileProjectConfig) GetHoldoutsForRule(ruleID string) []entities.Holdout {
299+
if holdouts, exists := c.ruleHoldoutsMap[ruleID]; exists {
290300
return holdouts
291301
}
292302
return []entities.Holdout{}
@@ -338,7 +348,7 @@ func NewDatafileProjectConfig(jsonDatafile []byte, logger logging.OptimizelyLogP
338348

339349
audienceMap, audienceSegmentList := mappers.MapAudiences(append(datafile.TypedAudiences, datafile.Audiences...))
340350
flagVariationsMap := mappers.MapFlagVariations(featureMap)
341-
holdouts, holdoutIDMap, flagHoldoutsMap := mappers.MapHoldouts(datafile.Holdouts, featureMap)
351+
holdouts, holdoutIDMap, globalHoldouts, ruleHoldoutsMap := mappers.MapHoldouts(datafile.Holdouts)
342352

343353
attributeKeyMap := make(map[string]entities.Attribute)
344354
attributeIDToKeyMap := make(map[string]string)
@@ -383,7 +393,8 @@ func NewDatafileProjectConfig(jsonDatafile []byte, logger logging.OptimizelyLogP
383393
region: region,
384394
holdouts: holdouts,
385395
holdoutIDMap: holdoutIDMap,
386-
flagHoldoutsMap: flagHoldoutsMap,
396+
ruleHoldoutsMap: ruleHoldoutsMap,
397+
globalHoldouts: globalHoldouts,
387398
}
388399

389400
logger.Info("Datafile is valid.")

pkg/config/datafileprojectconfig/config_test.go

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -729,8 +729,8 @@ func TestGetAttributeByKeyWithDirectMapping(t *testing.T) {
729729
assert.Equal(t, attribute, actual)
730730
}
731731

732-
func TestGetHoldoutsForFlagWithHoldouts(t *testing.T) {
733-
flagKey := "test_flag"
732+
func TestGetHoldoutsForRuleWithHoldouts(t *testing.T) {
733+
ruleID := "test_rule_id"
734734
holdout1 := entities.Holdout{
735735
ID: "holdout_1",
736736
Key: "test_holdout_1",
@@ -742,50 +742,28 @@ func TestGetHoldoutsForFlagWithHoldouts(t *testing.T) {
742742
Status: entities.HoldoutStatusRunning,
743743
}
744744

745-
flagHoldoutsMap := make(map[string][]entities.Holdout)
746-
flagHoldoutsMap[flagKey] = []entities.Holdout{holdout1, holdout2}
745+
ruleHoldoutsMap := make(map[string][]entities.Holdout)
746+
ruleHoldoutsMap[ruleID] = []entities.Holdout{holdout1, holdout2}
747747

748748
config := &DatafileProjectConfig{
749-
flagHoldoutsMap: flagHoldoutsMap,
749+
ruleHoldoutsMap: ruleHoldoutsMap,
750750
}
751751

752-
actual := config.GetHoldoutsForFlag(flagKey)
752+
actual := config.GetHoldoutsForRule(ruleID)
753753
assert.Len(t, actual, 2)
754754
assert.Equal(t, holdout1, actual[0])
755755
assert.Equal(t, holdout2, actual[1])
756756
}
757757

758-
func TestGetHoldoutsForFlagWithNoHoldouts(t *testing.T) {
759-
flagKey := "test_flag"
760-
flagHoldoutsMap := make(map[string][]entities.Holdout)
758+
func TestGetHoldoutsForRuleWithNoHoldouts(t *testing.T) {
759+
ruleID := "test_rule_id"
760+
ruleHoldoutsMap := make(map[string][]entities.Holdout)
761761

762762
config := &DatafileProjectConfig{
763-
flagHoldoutsMap: flagHoldoutsMap,
763+
ruleHoldoutsMap: ruleHoldoutsMap,
764764
}
765765

766-
actual := config.GetHoldoutsForFlag(flagKey)
767-
assert.Len(t, actual, 0)
768-
assert.Equal(t, []entities.Holdout{}, actual)
769-
}
770-
771-
func TestGetHoldoutsForFlagWithDifferentFlag(t *testing.T) {
772-
flagKey := "test_flag"
773-
otherFlagKey := "other_flag"
774-
holdout := entities.Holdout{
775-
ID: "holdout_1",
776-
Key: "test_holdout_1",
777-
Status: entities.HoldoutStatusRunning,
778-
}
779-
780-
flagHoldoutsMap := make(map[string][]entities.Holdout)
781-
flagHoldoutsMap[otherFlagKey] = []entities.Holdout{holdout}
782-
783-
config := &DatafileProjectConfig{
784-
flagHoldoutsMap: flagHoldoutsMap,
785-
}
786-
787-
// Request different flag - should return empty
788-
actual := config.GetHoldoutsForFlag(flagKey)
766+
actual := config.GetHoldoutsForRule(ruleID)
789767
assert.Len(t, actual, 0)
790768
assert.Equal(t, []entities.Holdout{}, actual)
791769
}

pkg/config/datafileprojectconfig/entities/entities.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ type Holdout struct {
123123
AudienceConditions interface{} `json:"audienceConditions"`
124124
Variations []Variation `json:"variations"`
125125
TrafficAllocation []TrafficAllocation `json:"trafficAllocation"`
126+
// IncludedRules is optional. nil = global holdout (applies to all rules across all flags).
127+
// Non-nil array = local holdout (applies only to the specified rule IDs).
128+
// An empty non-nil array means a local holdout that targets no rules.
129+
IncludedRules *[]string `json:"includedRules,omitempty"`
126130
}
127131

128132
// Integration represents a integration from the Optimizely datafile

pkg/config/datafileprojectconfig/mappers/holdout.go

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,19 @@ import (
2222
"github.com/optimizely/go-sdk/v2/pkg/entities"
2323
)
2424

25-
// MapHoldouts maps the raw datafile holdout entities to SDK Holdout entities
26-
// All running holdouts apply to all flags
27-
func MapHoldouts(holdouts []datafileEntities.Holdout, featureMap map[string]entities.Feature) (
25+
// MapHoldouts maps the raw datafile holdout entities to SDK Holdout entities.
26+
// Global holdouts (IncludedRules == nil) are returned in globalHoldouts for flag-level evaluation.
27+
// Local holdouts (IncludedRules != nil) are indexed by rule ID in ruleHoldoutsMap.
28+
func MapHoldouts(holdouts []datafileEntities.Holdout) (
2829
holdoutList []entities.Holdout,
2930
holdoutIDMap map[string]entities.Holdout,
30-
flagHoldoutsMap map[string][]entities.Holdout,
31+
globalHoldouts []entities.Holdout,
32+
ruleHoldoutsMap map[string][]entities.Holdout,
3133
) {
3234
holdoutList = []entities.Holdout{}
3335
holdoutIDMap = make(map[string]entities.Holdout)
34-
flagHoldoutsMap = make(map[string][]entities.Holdout)
35-
36-
runningHoldouts := []entities.Holdout{}
36+
globalHoldouts = []entities.Holdout{}
37+
ruleHoldoutsMap = make(map[string][]entities.Holdout)
3738

3839
for _, holdout := range holdouts {
3940
// Only process running holdouts
@@ -44,17 +45,18 @@ func MapHoldouts(holdouts []datafileEntities.Holdout, featureMap map[string]enti
4445
mappedHoldout := mapHoldout(holdout)
4546
holdoutList = append(holdoutList, mappedHoldout)
4647
holdoutIDMap[holdout.ID] = mappedHoldout
47-
runningHoldouts = append(runningHoldouts, mappedHoldout)
48-
}
4948

50-
// All running holdouts apply to all flags
51-
for _, feature := range featureMap {
52-
if len(runningHoldouts) > 0 {
53-
flagHoldoutsMap[feature.Key] = runningHoldouts
49+
if mappedHoldout.IsGlobal() {
50+
globalHoldouts = append(globalHoldouts, mappedHoldout)
51+
} else {
52+
// Local holdout: applies only to the specified rule IDs
53+
for _, ruleID := range *mappedHoldout.IncludedRules {
54+
ruleHoldoutsMap[ruleID] = append(ruleHoldoutsMap[ruleID], mappedHoldout)
55+
}
5456
}
5557
}
5658

57-
return holdoutList, holdoutIDMap, flagHoldoutsMap
59+
return holdoutList, holdoutIDMap, globalHoldouts, ruleHoldoutsMap
5860
}
5961

6062
func mapHoldout(datafileHoldout datafileEntities.Holdout) entities.Holdout {
@@ -107,5 +109,6 @@ func mapHoldout(datafileHoldout datafileEntities.Holdout) entities.Holdout {
107109
Variations: variations,
108110
TrafficAllocation: trafficAllocation,
109111
AudienceConditionTree: audienceConditionTree,
112+
IncludedRules: datafileHoldout.IncludedRules,
110113
}
111114
}

0 commit comments

Comments
 (0)