diff --git a/pkg/config/datafileprojectconfig/config.go b/pkg/config/datafileprojectconfig/config.go index cddda08e..eeeb341a 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 { @@ -409,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 { @@ -434,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 } } } diff --git a/pkg/config/datafileprojectconfig/feature_rollout_test.go b/pkg/config/datafileprojectconfig/feature_rollout_test.go index 53187970..fc679e6b 100644 --- a/pkg/config/datafileprojectconfig/feature_rollout_test.go +++ b/pkg/config/datafileprojectconfig/feature_rollout_test.go @@ -328,3 +328,174 @@ 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) +} + +// 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") +}