Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions pkg/cmab/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,16 @@ func (m *MockProjectConfig) GetHoldoutsForFlag(featureKey string) []entities.Hol
return args.Get(0).([]entities.Holdout)
}

func (m *MockProjectConfig) GetGlobalHoldouts() []entities.Holdout {
args := m.Called()
return args.Get(0).([]entities.Holdout)
}

func (m *MockProjectConfig) GetHoldoutsForRule(ruleID string) []entities.Holdout {
args := m.Called(ruleID)
return args.Get(0).([]entities.Holdout)
}

type CmabServiceTestSuite struct {
suite.Suite
mockClient *MockCmabClient
Expand Down
35 changes: 33 additions & 2 deletions pkg/config/datafileprojectconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ type DatafileProjectConfig struct {
holdouts []entities.Holdout
holdoutIDMap map[string]entities.Holdout
flagHoldoutsMap map[string][]entities.Holdout
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove it, no longer needed.

// ruleHoldoutsMap maps rule IDs to local holdouts targeting those rules
ruleHoldoutsMap map[string][]entities.Holdout
// globalHoldouts holds only global holdouts (IncludedRules == nil)
globalHoldouts []entities.Holdout
}

// GetDatafile returns a string representation of the environment's datafile
Expand Down Expand Up @@ -284,14 +288,31 @@ func (c DatafileProjectConfig) GetRegion() string {
return c.region
}

// GetHoldoutsForFlag returns all holdouts applicable to the given feature flag
// GetHoldoutsForFlag returns all global holdouts applicable to the given feature flag.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we no longer need flag base method anymore.

// Only global holdouts (those with IncludedRules == nil) are returned here.
// Local holdouts are retrieved per rule via GetHoldoutsForRule.
func (c DatafileProjectConfig) GetHoldoutsForFlag(featureKey string) []entities.Holdout {
if holdouts, exists := c.flagHoldoutsMap[featureKey]; exists {
return holdouts
}
return []entities.Holdout{}
}

// GetGlobalHoldouts returns all global holdouts (those with IncludedRules == nil).
// These are evaluated at flag level, before any per-rule evaluation.
func (c DatafileProjectConfig) GetGlobalHoldouts() []entities.Holdout {
return c.globalHoldouts
}

// GetHoldoutsForRule returns all local holdouts that target the given rule ID.
// These are evaluated per-rule, after forced decisions, before audience/traffic checks.
func (c DatafileProjectConfig) GetHoldoutsForRule(ruleID string) []entities.Holdout {
if holdouts, exists := c.ruleHoldoutsMap[ruleID]; exists {
return holdouts
}
return []entities.Holdout{}
}

// NewDatafileProjectConfig initializes a new datafile from a json byte array using the default JSON datafile parser
func NewDatafileProjectConfig(jsonDatafile []byte, logger logging.OptimizelyLogProducer) (*DatafileProjectConfig, error) {
datafile, err := Parse(jsonDatafile)
Expand Down Expand Up @@ -338,7 +359,15 @@ func NewDatafileProjectConfig(jsonDatafile []byte, logger logging.OptimizelyLogP

audienceMap, audienceSegmentList := mappers.MapAudiences(append(datafile.TypedAudiences, datafile.Audiences...))
flagVariationsMap := mappers.MapFlagVariations(featureMap)
holdouts, holdoutIDMap, flagHoldoutsMap := mappers.MapHoldouts(datafile.Holdouts, featureMap)
holdouts, holdoutIDMap, flagHoldoutsMap, ruleHoldoutsMap := mappers.MapHoldouts(datafile.Holdouts, featureMap)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove 'flagHoldoutsMap'


// Collect global holdouts (IncludedRules == nil) for direct access
globalHoldouts := []entities.Holdout{}
for i := range holdouts {
if holdouts[i].IsGlobal() {
globalHoldouts = append(globalHoldouts, holdouts[i])
}
}

attributeKeyMap := make(map[string]entities.Attribute)
attributeIDToKeyMap := make(map[string]string)
Expand Down Expand Up @@ -384,6 +413,8 @@ func NewDatafileProjectConfig(jsonDatafile []byte, logger logging.OptimizelyLogP
holdouts: holdouts,
holdoutIDMap: holdoutIDMap,
flagHoldoutsMap: flagHoldoutsMap,
ruleHoldoutsMap: ruleHoldoutsMap,
globalHoldouts: globalHoldouts,
}

logger.Info("Datafile is valid.")
Expand Down
39 changes: 39 additions & 0 deletions pkg/config/datafileprojectconfig/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,3 +789,42 @@ func TestGetHoldoutsForFlagWithDifferentFlag(t *testing.T) {
assert.Len(t, actual, 0)
assert.Equal(t, []entities.Holdout{}, actual)
}

func TestGetHoldoutsForRuleWithHoldouts(t *testing.T) {
ruleID := "test_rule_id"
holdout1 := entities.Holdout{
ID: "holdout_1",
Key: "test_holdout_1",
Status: entities.HoldoutStatusRunning,
}
holdout2 := entities.Holdout{
ID: "holdout_2",
Key: "test_holdout_2",
Status: entities.HoldoutStatusRunning,
}

ruleHoldoutsMap := make(map[string][]entities.Holdout)
ruleHoldoutsMap[ruleID] = []entities.Holdout{holdout1, holdout2}

config := &DatafileProjectConfig{
ruleHoldoutsMap: ruleHoldoutsMap,
}

actual := config.GetHoldoutsForRule(ruleID)
assert.Len(t, actual, 2)
assert.Equal(t, holdout1, actual[0])
assert.Equal(t, holdout2, actual[1])
}

func TestGetHoldoutsForRuleWithNoHoldouts(t *testing.T) {
ruleID := "test_rule_id"
ruleHoldoutsMap := make(map[string][]entities.Holdout)

config := &DatafileProjectConfig{
ruleHoldoutsMap: ruleHoldoutsMap,
}

actual := config.GetHoldoutsForRule(ruleID)
assert.Len(t, actual, 0)
assert.Equal(t, []entities.Holdout{}, actual)
}
4 changes: 4 additions & 0 deletions pkg/config/datafileprojectconfig/entities/entities.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ type Holdout struct {
AudienceConditions interface{} `json:"audienceConditions"`
Variations []Variation `json:"variations"`
TrafficAllocation []TrafficAllocation `json:"trafficAllocation"`
// IncludedRules is optional. nil = global holdout (applies to all rules across all flags).
// Non-nil array = local holdout (applies only to the specified rule IDs).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks consistence with other sdks

// An empty non-nil array means a local holdout that targets no rules.
IncludedRules *[]string `json:"includedRules,omitempty"`
}

// Integration represents a integration from the Optimizely datafile
Expand Down
29 changes: 21 additions & 8 deletions pkg/config/datafileprojectconfig/mappers/holdout.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,21 @@ import (
"github.com/optimizely/go-sdk/v2/pkg/entities"
)

// MapHoldouts maps the raw datafile holdout entities to SDK Holdout entities
// All running holdouts apply to all flags
// MapHoldouts maps the raw datafile holdout entities to SDK Holdout entities.
// Global holdouts (IncludedRules == nil) apply to all flags via flagHoldoutsMap.
// Local holdouts (IncludedRules != nil) are indexed by rule ID in ruleHoldoutsMap.
func MapHoldouts(holdouts []datafileEntities.Holdout, featureMap map[string]entities.Feature) (
holdoutList []entities.Holdout,
holdoutIDMap map[string]entities.Holdout,
flagHoldoutsMap map[string][]entities.Holdout,
ruleHoldoutsMap map[string][]entities.Holdout,
) {
holdoutList = []entities.Holdout{}
holdoutIDMap = make(map[string]entities.Holdout)
flagHoldoutsMap = make(map[string][]entities.Holdout)
ruleHoldoutsMap = make(map[string][]entities.Holdout)

runningHoldouts := []entities.Holdout{}
globalHoldouts := []entities.Holdout{}

for _, holdout := range holdouts {
// Only process running holdouts
Expand All @@ -44,17 +47,26 @@ func MapHoldouts(holdouts []datafileEntities.Holdout, featureMap map[string]enti
mappedHoldout := mapHoldout(holdout)
holdoutList = append(holdoutList, mappedHoldout)
holdoutIDMap[holdout.ID] = mappedHoldout
runningHoldouts = append(runningHoldouts, mappedHoldout)

if mappedHoldout.IsGlobal() {
// Global holdout: applies to all rules across all flags
globalHoldouts = append(globalHoldouts, mappedHoldout)
} else {
// Local holdout: applies only to the specified rule IDs
for _, ruleID := range *mappedHoldout.IncludedRules {
ruleHoldoutsMap[ruleID] = append(ruleHoldoutsMap[ruleID], mappedHoldout)
}
}
}

// All running holdouts apply to all flags
// Global holdouts apply to all flags (flag-level evaluation)
for _, feature := range featureMap {
if len(runningHoldouts) > 0 {
flagHoldoutsMap[feature.Key] = runningHoldouts
if len(globalHoldouts) > 0 {
flagHoldoutsMap[feature.Key] = globalHoldouts
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need flagHoldoutsMap? I think ruleHoldoutsMap is enough for targeting local HO

}
}

return holdoutList, holdoutIDMap, flagHoldoutsMap
return holdoutList, holdoutIDMap, flagHoldoutsMap, ruleHoldoutsMap
}

func mapHoldout(datafileHoldout datafileEntities.Holdout) entities.Holdout {
Expand Down Expand Up @@ -107,5 +119,6 @@ func mapHoldout(datafileHoldout datafileEntities.Holdout) entities.Holdout {
Variations: variations,
TrafficAllocation: trafficAllocation,
AudienceConditionTree: audienceConditionTree,
IncludedRules: datafileHoldout.IncludedRules,
}
}
Loading
Loading