From da396b4a2429f22d4c3477d7b154c681a55300fd Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Thu, 2 Apr 2026 21:52:44 +0600 Subject: [PATCH 1/3] [AI-FSSDK] [FSSDK-12418] Remove experiment type validation from config parsing --- pkg/config/datafileprojectconfig/config.go | 15 ----- .../feature_rollout_test.go | 64 +++++++++++++++++++ 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/pkg/config/datafileprojectconfig/config.go b/pkg/config/datafileprojectconfig/config.go index cddda08e..4f8faf3e 100644 --- a/pkg/config/datafileprojectconfig/config.go +++ b/pkg/config/datafileprojectconfig/config.go @@ -325,21 +325,6 @@ func NewDatafileProjectConfig(jsonDatafile []byte, logger logging.OptimizelyLogP groupMap, experimentGroupMap := mappers.MapGroups(datafile.Groups) experimentIDMap, experimentKeyMap := mappers.MapExperiments(allExperiments, experimentGroupMap) - validExperimentTypes := map[entities.ExperimentType]bool{ - entities.ExperimentTypeAB: true, - entities.ExperimentTypeMAB: true, - entities.ExperimentTypeCMAB: true, - entities.ExperimentTypeTD: true, - entities.ExperimentTypeFR: true, - } - for _, experiment := range experimentIDMap { - if experiment.Type != "" && !validExperimentTypes[experiment.Type] { - err = fmt.Errorf(`experiment "%s" has invalid type "%s"`, experiment.Key, experiment.Type) - logger.Error(err.Error(), err) - return nil, err - } - } - rollouts, rolloutMap := mappers.MapRollouts(datafile.Rollouts) integrations := []entities.Integration{} for _, integration := range datafile.Integrations { diff --git a/pkg/config/datafileprojectconfig/feature_rollout_test.go b/pkg/config/datafileprojectconfig/feature_rollout_test.go index 53187970..49af31c3 100644 --- a/pkg/config/datafileprojectconfig/feature_rollout_test.go +++ b/pkg/config/datafileprojectconfig/feature_rollout_test.go @@ -328,3 +328,67 @@ func TestTypeFieldCorrectlyParsed(t *testing.T) { assert.NoError(t, err) assert.Empty(t, noTypeExp.Type) } + +// Test 7: Unknown experiment type accepted - config parsing succeeds with unknown type value +func TestUnknownExperimentTypeAccepted(t *testing.T) { + datafile := `{ + "accountId": "12345", + "anonymizeIP": false, + "sendFlagDecisions": true, + "botFiltering": false, + "projectId": "67890", + "revision": "1", + "sdkKey": "UnknownTypeTest", + "environmentKey": "production", + "version": "4", + "audiences": [], + "typedAudiences": [], + "attributes": [], + "events": [], + "groups": [], + "integrations": [], + "experiments": [ + { + "id": "exp_unknown", + "key": "unknown_type_experiment", + "status": "Running", + "layerId": "layer_1", + "audienceIds": [], + "forcedVariations": {}, + "type": "new_unknown_type", + "variations": [ + { + "id": "var_1", + "key": "variation_1", + "featureEnabled": true + } + ], + "trafficAllocation": [ + { + "entityId": "var_1", + "endOfRange": 10000 + } + ] + } + ], + "featureFlags": [ + { + "id": "flag_1", + "key": "test_flag", + "rolloutId": "", + "experimentIds": ["exp_unknown"], + "variables": [] + } + ], + "rollouts": [] +}` + + logger := logging.GetLogger("test", "TestUnknownExperimentTypeAccepted") + config, err := NewDatafileProjectConfig([]byte(datafile), logger) + require.NoError(t, err, "Config parsing should succeed with unknown experiment type") + require.NotNil(t, config) + + experiment, err := config.GetExperimentByKey("unknown_type_experiment") + assert.NoError(t, err) + assert.Equal(t, entities.ExperimentType("new_unknown_type"), experiment.Type) +} From 0a5b37817c080a21b5938da8a41af45f809fcfc6 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Thu, 2 Apr 2026 10:04:27 -0700 Subject: [PATCH 2/3] Fix Feature Rollout injection not updating feature.FeatureExperiments The injectFeatureRolloutVariations function was modifying experiments in experimentIDMap but not updating the copies in feature.FeatureExperiments. This caused the decision service to use experiments without the injected "everyone else" variation, leading to incorrect bucketing and fallthrough to rollout rules instead of returning the FR experiment decision. When MapFeatures runs, it creates feature.FeatureExperiments by copying experiments from experimentIDMap. Since Go copies struct values, later modifications to experimentIDMap experiments (like appending to TrafficAllocation slice) don't affect the copies in FeatureExperiments. The fix updates both experimentIDMap AND feature.FeatureExperiments to ensure the decision service sees the complete injected variations. This fixes FSC test failures where users were getting rule_key from rollout rules (e.g., "ee_rule_1002") instead of from FR experiments (e.g., "fr_zero_exp"). --- pkg/config/datafileprojectconfig/config.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/config/datafileprojectconfig/config.go b/pkg/config/datafileprojectconfig/config.go index 4f8faf3e..eeeb341a 100644 --- a/pkg/config/datafileprojectconfig/config.go +++ b/pkg/config/datafileprojectconfig/config.go @@ -394,12 +394,13 @@ func NewDatafileProjectConfig(jsonDatafile []byte, logger logging.OptimizelyLogP // into any experiment with type "feature_rollout". This enables Feature Rollout experiments // to fall back to the everyone else variation when users are outside the rollout percentage. func injectFeatureRolloutVariations(featureMap map[string]entities.Feature, experimentMap map[string]entities.Experiment) { - for _, feature := range featureMap { + for featureKey, feature := range featureMap { everyoneElseVariation := getEveryoneElseVariation(feature) if everyoneElseVariation == nil { continue } + featureModified := false for _, experimentID := range feature.ExperimentIDs { experiment, ok := experimentMap[experimentID] if !ok { @@ -419,6 +420,20 @@ func injectFeatureRolloutVariations(featureMap map[string]entities.Feature, expe // Update the experiment in the map experimentMap[experimentID] = experiment + + // Also update the experiment in the feature's FeatureExperiments slice + for j := range feature.FeatureExperiments { + if feature.FeatureExperiments[j].ID == experimentID { + feature.FeatureExperiments[j] = experiment + featureModified = true + break + } + } + } + + // Update the feature in the map if it was modified + if featureModified { + featureMap[featureKey] = feature } } } From 434222ab0a0825bf704a39682ce4325dcb87990b Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Thu, 2 Apr 2026 10:05:13 -0700 Subject: [PATCH 3/3] Add test to verify injection updates both experimentIDMap and FeatureExperiments This test ensures that the Feature Rollout injection properly updates both the experimentIDMap and the feature's FeatureExperiments slice, preventing regression of the bug where only experimentIDMap was updated. --- .../feature_rollout_test.go | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/pkg/config/datafileprojectconfig/feature_rollout_test.go b/pkg/config/datafileprojectconfig/feature_rollout_test.go index 49af31c3..fc679e6b 100644 --- a/pkg/config/datafileprojectconfig/feature_rollout_test.go +++ b/pkg/config/datafileprojectconfig/feature_rollout_test.go @@ -392,3 +392,110 @@ func TestUnknownExperimentTypeAccepted(t *testing.T) { assert.NoError(t, err) assert.Equal(t, entities.ExperimentType("new_unknown_type"), experiment.Type) } + +// Test 8: Injection updates both experimentIDMap and feature.FeatureExperiments +func TestFeatureRolloutInjectionUpdatesFeatureExperiments(t *testing.T) { + datafile := `{ + "accountId": "12345", + "anonymizeIP": false, + "projectId": "67890", + "revision": "1", + "version": "4", + "audiences": [], + "attributes": [], + "events": [{"id": "1", "key": "test_event", "experimentIds": []}], + "groups": [], + "experiments": [ + { + "id": "3002", + "key": "fr_zero_exp", + "status": "Running", + "layerId": "layer_3002", + "audienceIds": [], + "forcedVariations": {}, + "type": "fr", + "variations": [ + { + "id": "5002", + "key": "rollout_var", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "5002", + "endOfRange": 0 + } + ] + } + ], + "featureFlags": [ + { + "id": "1002", + "key": "flag_fr_zero", + "rolloutId": "rollout_1002", + "experimentIds": ["3002"], + "variables": [] + } + ], + "rollouts": [ + { + "id": "rollout_1002", + "experiments": [ + { + "id": "ee_rule_1002", + "key": "ee_rule_1002", + "status": "Running", + "layerId": "rollout_1002", + "audienceIds": [], + "variations": [ + { + "id": "5102", + "key": "everyone_else_var", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "5102", + "endOfRange": 10000 + } + ], + "forcedVariations": {} + } + ] + } + ] +}` + + logger := logging.GetLogger("test", "TestFeatureRolloutInjectionUpdatesFeatureExperiments") + config, err := NewDatafileProjectConfig([]byte(datafile), logger) + require.NoError(t, err) + + // Get experiment from experimentIDMap + experimentByID, err := config.GetExperimentByID("3002") + assert.NoError(t, err) + assert.Equal(t, 2, len(experimentByID.Variations), "experimentIDMap should have injected variation") + assert.Equal(t, 2, len(experimentByID.TrafficAllocation), "experimentIDMap should have injected traffic allocation") + + // Get experiment from feature.FeatureExperiments + feature, err := config.GetFeatureByKey("flag_fr_zero") + assert.NoError(t, err) + assert.Equal(t, 1, len(feature.FeatureExperiments), "Feature should have 1 experiment") + + featureExperiment := feature.FeatureExperiments[0] + assert.Equal(t, "3002", featureExperiment.ID) + assert.Equal(t, 2, len(featureExperiment.Variations), "Feature experiment should have injected variation") + assert.Equal(t, 2, len(featureExperiment.TrafficAllocation), "Feature experiment should have injected traffic allocation") + + // Verify the injected traffic allocation is correct + lastAllocation := featureExperiment.TrafficAllocation[len(featureExperiment.TrafficAllocation)-1] + assert.Equal(t, "5102", lastAllocation.EntityID, "Last allocation should be everyone else variation") + assert.Equal(t, 10000, lastAllocation.EndOfRange, "Last allocation should have endOfRange 10000") + + // Verify both maps have the same traffic allocations + assert.Equal(t, len(experimentByID.TrafficAllocation), len(featureExperiment.TrafficAllocation), + "Traffic allocations should match between experimentIDMap and FeatureExperiments") +}