diff --git a/.gitmodules b/.gitmodules index f46350d8..5e406b4e 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "flagengine/engine-test-data"] path = flagengine/engine-test-data url = git@github.com:Flagsmith/engine-test-data.git - branch = v3.7.0 + branch = v3.10.0 diff --git a/client_test.go b/client_test.go index fae8d670..c4cc8847 100644 --- a/client_test.go +++ b/client_test.go @@ -235,6 +235,8 @@ func TestGetFlags(t *testing.T) { assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName) assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID) assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value) + assert.Equal(t, fixtures.Feature1Reason, allFlags[0].Reason) + assert.Empty(t, allFlags[0].Variant) } func TestGetFlagsTransientIdentity(t *testing.T) { @@ -261,6 +263,8 @@ func TestGetFlagsTransientIdentity(t *testing.T) { assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName) assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID) assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value) + assert.Equal(t, fixtures.Feature1IdentityReason, allFlags[0].Reason) + assert.Equal(t, fixtures.Feature1IdentityVariant, allFlags[0].Variant) } func TestGetFlagsTransientTraits(t *testing.T) { @@ -372,6 +376,7 @@ func TestGetEnvironmentFlagsUseslocalEnvironmentWhenAvailable(t *testing.T) { assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName) assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID) assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value) + assert.Equal(t, "DEFAULT", allFlags[0].Reason) } func TestGetEnvironmentFlagsCallsAPIWhenLocalEnvironmentNotAvailable(t *testing.T) { @@ -447,6 +452,91 @@ func TestGetEnvironmentFlagsIgnoresSegmentOverrides(t *testing.T) { assert.NoError(t, err) assert.Equal(t, fixtures.Feature1Value, flag.Value) assert.Equal(t, "some_value", flag.Value) + assert.Equal(t, "DEFAULT", flag.Reason) +} + +func TestGetIdentityFlagsAppliesSegmentOverridesWithReason(t *testing.T) { + // Given + ctx := context.Background() + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(http.StatusOK) + _, _ = io.WriteString(rw, fixtures.EnvironmentJsonWithSegmentOverride) + })) + defer server.Close() + + // When + client := flagsmith.NewClient(fixtures.EnvironmentAPIKey, + flagsmith.WithLocalEvaluation(ctx), + flagsmith.WithBaseURL(server.URL+"/api/v1/")) + err := client.UpdateEnvironment(ctx) + assert.NoError(t, err) + + flags, err := client.GetIdentityFlags(ctx, "test_identity", nil) + + // Then + assert.NoError(t, err) + flag, err := flags.GetFlag(fixtures.Feature1Name) + assert.NoError(t, err) + assert.Equal(t, "segment_override", flag.Value) + assert.Equal(t, "TARGETING_MATCH; segment=Test Segment", flag.Reason) +} + +func TestGetIdentityFlagsSetsVariantForMultivariateFeature(t *testing.T) { + // Given + ctx := context.Background() + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(http.StatusOK) + _, _ = io.WriteString(rw, fixtures.EnvironmentJsonWithMultivariateFeature) + })) + defer server.Close() + + // When + client := flagsmith.NewClient(fixtures.EnvironmentAPIKey, + flagsmith.WithLocalEvaluation(ctx), + flagsmith.WithBaseURL(server.URL+"/api/v1/")) + err := client.UpdateEnvironment(ctx) + assert.NoError(t, err) + + flags, err := client.GetIdentityFlags(ctx, "test_identity", nil) + + // Then + assert.NoError(t, err) + flag, err := flags.GetFlag(fixtures.MVFeatureName) + assert.NoError(t, err) + assert.Equal(t, fixtures.MVFeatureVariantValue, flag.Value) + assert.Equal(t, fixtures.MVFeatureVariantKey, flag.Variant) + assert.Equal(t, "SPLIT; weight=100", flag.Reason) +} + +func TestGetEnvironmentFlagsHasNoVariantForMultivariateFeature(t *testing.T) { + // Given + ctx := context.Background() + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(http.StatusOK) + _, _ = io.WriteString(rw, fixtures.EnvironmentJsonWithMultivariateFeature) + })) + defer server.Close() + + // When + client := flagsmith.NewClient(fixtures.EnvironmentAPIKey, + flagsmith.WithLocalEvaluation(ctx), + flagsmith.WithBaseURL(server.URL+"/api/v1/")) + err := client.UpdateEnvironment(ctx) + assert.NoError(t, err) + + flags, err := client.GetEnvironmentFlags(ctx) + + // Then: without an identity there is nothing to bucket, so the control value is + // served without a variant + assert.NoError(t, err) + flag, err := flags.GetFlag(fixtures.MVFeatureName) + assert.NoError(t, err) + assert.Equal(t, "control_value", flag.Value) + assert.Empty(t, flag.Variant) + assert.Equal(t, "DEFAULT", flag.Reason) } func TestGetIdentityFlagsUseslocalEnvironmentWhenAvailable(t *testing.T) { @@ -499,6 +589,7 @@ func TestGetIdentityFlagsUseslocalOverridesWhenAvailable(t *testing.T) { assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName) assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID) assert.Equal(t, fixtures.Feature1OverriddenValue, allFlags[0].Value) + assert.Equal(t, "TARGETING_MATCH; segment=identity_overrides", allFlags[0].Reason) } func TestGetIdentityFlagsCallsAPIWhenLocalEnvironmentNotAvailableWithTraits(t *testing.T) { diff --git a/fixtures/fixture.go b/fixtures/fixture.go index b899f6ad..ef83a056 100644 --- a/fixtures/fixture.go +++ b/fixtures/fixture.go @@ -10,6 +10,13 @@ const EnvironmentAPIKey = "ser.test_key" const Feature1Value = "some_value" const Feature1Name = "feature_1" const Feature1ID = 1 +const Feature1Reason = "DEFAULT" +const Feature1IdentityReason = "SPLIT; weight=50.0" +const Feature1IdentityVariant = "treatment" + +const MVFeatureName = "mv_feature" +const MVFeatureVariantKey = "treatment" +const MVFeatureVariantValue = "variant_value" const Feature1OverriddenValue = "some-overridden-value" const ClientAPIKey = "B62qaMZNwfiqT76p38ggrQ" @@ -163,6 +170,54 @@ const EnvironmentJsonWithSegmentOverride = ` } ` +// EnvironmentJsonWithMultivariateFeature contains a single multivariate feature whose +// only variant is keyed and allocated 100%, so every identity is bucketed into it. +const EnvironmentJsonWithMultivariateFeature = ` +{ + "api_key": "B62qaMZNwfiqT76p38ggrQ", + "name": "Test Environment", + "updated_at": "2023-12-06T10:21:54.079725Z", + "project": { + "name": "Test project", + "organisation": { + "feature_analytics": false, + "name": "Test Org", + "id": 1, + "persist_trait_data": true, + "stop_serving_flags": false + }, + "id": 1, + "hide_disabled_flags": false, + "segments": [] + }, + "segment_overrides": [], + "id": 1, + "feature_states": [{ + "multivariate_feature_state_values": [{ + "id": 1, + "multivariate_feature_option": { + "id": 1, + "value": "variant_value", + "key": "treatment" + }, + "percentage_allocation": 100, + "mv_fs_value_uuid": "1e1e1e1e-1e1e-1e1e-1e1e-1e1e1e1e1e1e" + }], + "feature_state_value": "control_value", + "id": 2, + "featurestate_uuid": "f0c8f0c8-f0c8-f0c8-f0c8-f0c8f0c8f0c8", + "feature": { + "name": "mv_feature", + "type": "MULTIVARIATE", + "id": 2 + }, + "segment_id": null, + "enabled": true + }], + "identity_overrides": [] +} +` + const FlagsJson = ` [{ "id": 1, @@ -177,6 +232,7 @@ const FlagsJson = ` "project": 1 }, "feature_state_value": "some_value", + "reason": "DEFAULT", "enabled": true, "environment": 1, "identity": null, @@ -198,6 +254,8 @@ const IdentityResponseJson = ` "project": 1 }, "feature_state_value": "some_value", + "reason": "SPLIT; weight=50.0", + "variant": "treatment", "enabled": true, "environment": 1, "identity": null, diff --git a/flagengine/engine-test-data b/flagengine/engine-test-data index 4b29dc77..2e710ea7 160000 --- a/flagengine/engine-test-data +++ b/flagengine/engine-test-data @@ -1 +1 @@ -Subproject commit 4b29dc772a764364af2dd504ecefbdf74cf5473f +Subproject commit 2e710ea7c6994110d17d1adb40b2ec2e90e7cf44 diff --git a/flagengine/engine.go b/flagengine/engine.go index 170804fd..645b9d78 100644 --- a/flagengine/engine.go +++ b/flagengine/engine.go @@ -9,6 +9,8 @@ import ( "github.com/Flagsmith/flagsmith-go-client/v5/flagengine/utils" ) +const controlVariantKey = "control" + type featureContextWithSegmentName struct { featureContext *engine_eval.FeatureContext segmentName string @@ -130,9 +132,13 @@ func GetEvaluationResult(ec *engine_eval.EngineEvaluationContext) engine_eval.Ev // getFlagResultFromFeatureContext creates a FlagResult from a FeatureContext. func getFlagResultFromFeatureContext(featureName string, featureContext *engine_eval.FeatureContext, identityKey *string, reason string) engine_eval.FlagResult { value := featureContext.Value + variantKey := "" // Handle multivariate features if len(featureContext.Variants) > 0 && identityKey != nil && featureContext.Key != "" { + // Default to the control bucket; a selected variant overrides this + variantKey = controlVariantKey + // Sort variants by priority (lower priority value = higher priority) sortedVariants := getSortedVariantsByPriority(featureContext.Variants) @@ -146,6 +152,7 @@ func getFlagResultFromFeatureContext(featureName string, featureContext *engine_ cumulativeWeight += variant.Weight if hashPercentage <= cumulativeWeight { value = variant.Value + variantKey = variant.Key reason = fmt.Sprintf("SPLIT; weight=%g", variant.Weight) break } @@ -156,6 +163,7 @@ func getFlagResultFromFeatureContext(featureName string, featureContext *engine_ Enabled: featureContext.Enabled, Name: featureName, Value: value, + Variant: variantKey, Reason: reason, Metadata: featureContext.Metadata, } diff --git a/flagengine/engine_eval/context.go b/flagengine/engine_eval/context.go index 286f3245..d8bc8fc7 100644 --- a/flagengine/engine_eval/context.go +++ b/flagengine/engine_eval/context.go @@ -52,6 +52,9 @@ type FeatureContext struct { // Represents a multivariate value for a feature flag. type FeatureValue struct { + // A stable identifier for the variant, reported as the flag result's variant + // when this value is selected. Empty if the variant is not keyed. + Key string `json:"key,omitempty"` // The value of the feature. Value any `json:"value"` // The weight of the feature value variant, as a percentage number (i.e. 100.0). diff --git a/flagengine/engine_eval/mappers.go b/flagengine/engine_eval/mappers.go index 133f6093..e4d85a95 100644 --- a/flagengine/engine_eval/mappers.go +++ b/flagengine/engine_eval/mappers.go @@ -72,6 +72,7 @@ func mapMultivariateFeatureStateValuesToVariants(multivariateValues []*features. Value: mv.MultivariateFeatureOption.Value, Weight: mv.PercentageAllocation, Priority: mv.Priority(), + Key: mv.MultivariateFeatureOption.Key, }) } return variants diff --git a/flagengine/engine_eval/result.go b/flagengine/engine_eval/result.go index d1c56e69..3e215140 100644 --- a/flagengine/engine_eval/result.go +++ b/flagengine/engine_eval/result.go @@ -18,6 +18,10 @@ type FlagResult struct { Reason string `json:"reason,omitempty"` // Feature flag value. Value any `json:"value,omitempty"` + // Key of the multivariate variant the value was selected from: "control" when + // the identity falls in the control bucket, or the selected variant's key. + // Empty for standard features, unkeyed variants, and evaluation without an identity. + Variant string `json:"variant,omitempty"` // Metadata about the feature. Metadata FeatureMetadata `json:"metadata,omitempty"` } diff --git a/flagengine/features/models.go b/flagengine/features/models.go index 1392ad30..1e0ec13b 100644 --- a/flagengine/features/models.go +++ b/flagengine/features/models.go @@ -58,6 +58,7 @@ func (fs *FeatureStateModel) UnmarshalJSON(bytes []byte) error { type MultivariateFeatureOptionModel struct { ID int `json:"id"` Value interface{} `json:"value"` + Key string `json:"key"` } type MultivariateFeatureStateValueModel struct { diff --git a/models.go b/models.go index 36aef4f6..6ea0dee6 100644 --- a/models.go +++ b/models.go @@ -14,6 +14,8 @@ type Flag struct { IsDefault bool FeatureID int FeatureName string + Reason string + Variant string } type Trait = trait.Trait @@ -36,6 +38,8 @@ func makeFlagFromEngineEvaluationFlagResult(flagResult *engine_eval.FlagResult) IsDefault: false, FeatureID: featureID, FeatureName: flagResult.Name, + Reason: flagResult.Reason, + Variant: flagResult.Variant, } } @@ -67,6 +71,8 @@ type jsonFlag struct { Enabled bool `json:"enabled"` Value interface{} `json:"feature_state_value"` Feature jsonFeature `json:"feature"` + Reason string `json:"reason"` + Variant string `json:"variant"` } func (jf *jsonFlag) toFlag() Flag { @@ -76,6 +82,8 @@ func (jf *jsonFlag) toFlag() Flag { IsDefault: false, FeatureID: jf.Feature.ID, FeatureName: jf.Feature.Name, + Reason: jf.Reason, + Variant: jf.Variant, } } func makeFlagsFromAPIFlags(flagsJson []byte, analyticsProcessor *AnalyticsProcessor, defaultFlagHandler func(string) (Flag, error)) (Flags, error) { diff --git a/models_test.go b/models_test.go index 005b8177..d25c5b47 100644 --- a/models_test.go +++ b/models_test.go @@ -3,9 +3,69 @@ package flagsmith import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/Flagsmith/flagsmith-go-client/v5/flagengine/engine_eval" ) +func TestMakeFlagsFromAPIFlagsSetsReasonAndVariant(t *testing.T) { + // Given + flagsJson := []byte(`[{ + "enabled": true, + "feature_state_value": "test-value", + "feature": {"id": 123, "name": "test_feature"}, + "reason": "TARGETING_MATCH; segment=premium", + "variant": "treatment" + }]`) + + // When + flags, err := makeFlagsFromAPIFlags(flagsJson, nil, nil) + + // Then + require.NoError(t, err) + require.Len(t, flags.flags, 1) + assert.Equal(t, "TARGETING_MATCH; segment=premium", flags.flags[0].Reason) + assert.Equal(t, "treatment", flags.flags[0].Variant) +} + +func TestMakeFlagsFromAPIFlagsNoReasonOrVariantIsEmpty(t *testing.T) { + // Given: a response from an API that predates the reason and variant fields + flagsJson := []byte(`[{ + "enabled": true, + "feature_state_value": "test-value", + "feature": {"id": 123, "name": "test_feature"} + }]`) + + // When + flags, err := makeFlagsFromAPIFlags(flagsJson, nil, nil) + + // Then + require.NoError(t, err) + require.Len(t, flags.flags, 1) + assert.Empty(t, flags.flags[0].Reason) + assert.Empty(t, flags.flags[0].Variant) +} + +func TestMakeFlagsFromAPIFlagsNullVariantIsEmpty(t *testing.T) { + // Given: a standard feature, for which the API reports a null variant + flagsJson := []byte(`[{ + "enabled": true, + "feature_state_value": "test-value", + "feature": {"id": 123, "name": "test_feature"}, + "reason": "DEFAULT", + "variant": null + }]`) + + // When + flags, err := makeFlagsFromAPIFlags(flagsJson, nil, nil) + + // Then + require.NoError(t, err) + require.Len(t, flags.flags, 1) + assert.Empty(t, flags.flags[0].Variant) +} + func TestMakeFlagFromEngineEvaluationFlagResult(t *testing.T) { tests := []struct { name string @@ -88,11 +148,11 @@ func TestMakeFlagFromEngineEvaluationFlagResult(t *testing.T) { }, }, { - name: "flag with reason field (should be ignored in conversion)", + name: "flag with reason field", input: &engine_eval.FlagResult{ Enabled: true, Name: "reason_feature", - Reason: "TARGETING_MATCH", + Reason: "TARGETING_MATCH; segment=premium_segment", Value: "reason_value", }, expected: Flag{ @@ -101,6 +161,45 @@ func TestMakeFlagFromEngineEvaluationFlagResult(t *testing.T) { IsDefault: false, FeatureID: 0, FeatureName: "reason_feature", + Reason: "TARGETING_MATCH; segment=premium_segment", + }, + }, + { + name: "multivariate flag with selected variant", + input: &engine_eval.FlagResult{ + Enabled: true, + Name: "mv_feature", + Reason: "SPLIT; weight=30", + Value: "variant_value", + Variant: "treatment", + }, + expected: Flag{ + Enabled: true, + Value: "variant_value", + IsDefault: false, + FeatureID: 0, + FeatureName: "mv_feature", + Reason: "SPLIT; weight=30", + Variant: "treatment", + }, + }, + { + name: "multivariate flag in the control bucket", + input: &engine_eval.FlagResult{ + Enabled: true, + Name: "mv_feature", + Reason: "DEFAULT", + Value: "control_value", + Variant: "control", + }, + expected: Flag{ + Enabled: true, + Value: "control_value", + IsDefault: false, + FeatureID: 0, + FeatureName: "mv_feature", + Reason: "DEFAULT", + Variant: "control", }, }, } @@ -124,6 +223,12 @@ func TestMakeFlagFromEngineEvaluationFlagResult(t *testing.T) { if result.FeatureName != tt.expected.FeatureName { t.Errorf("Expected FeatureName %v, got %v", tt.expected.FeatureName, result.FeatureName) } + if result.Reason != tt.expected.Reason { + t.Errorf("Expected Reason %v, got %v", tt.expected.Reason, result.Reason) + } + if result.Variant != tt.expected.Variant { + t.Errorf("Expected Variant %v, got %v", tt.expected.Variant, result.Variant) + } }) } } @@ -142,6 +247,7 @@ func TestMakeFlagsFromEngineEvaluationResult(t *testing.T) { Enabled: true, Name: "feature1", Value: "value1", + Reason: "DEFAULT", }, "feature2": { Enabled: false, @@ -163,6 +269,7 @@ func TestMakeFlagsFromEngineEvaluationResult(t *testing.T) { IsDefault: false, FeatureID: 0, FeatureName: "feature1", + Reason: "DEFAULT", }, { Enabled: false, @@ -250,6 +357,9 @@ func TestMakeFlagsFromEngineEvaluationResult(t *testing.T) { if actualFlag.FeatureName != expectedFlag.FeatureName { t.Errorf("Flag %s: Expected FeatureName %v, got %v", expectedFlag.FeatureName, expectedFlag.FeatureName, actualFlag.FeatureName) } + if actualFlag.Reason != expectedFlag.Reason { + t.Errorf("Flag %s: Expected Reason %v, got %v", expectedFlag.FeatureName, expectedFlag.Reason, actualFlag.Reason) + } } // Test that analytics processor and default flag handler are set correctly