fix: evaluationContextEnrichement with key env overriden by empty str…

…ing (#2367) * fix: evaluationContextEnrichement with key env overriden by empty string Signed-off-by: Thomas Poignant <[email protected]> * Adding documentation to explain how the special case env is working Signed-off-by: Thomas Poignant <[email protected]> * Fixing the issue for allFlag too Signed-off-by: Thomas Poignant <[email protected]> --------- Signed-off-by: Thomas Poignant <[email protected]>
thomaspoignant · Sep 18, 2024 · f3d44ea · f3d44ea
1 parent 7fc7c6a
commit f3d44ea
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 20 deletions.
diff --git a/variation.go b/variation.go
@@ -277,6 +277,7 @@ func notifyVariation[T model.JSONType](
 
 // getVariation is the internal generic func that handle the logic of a variation the result will always
 // contain a valid model.VariationResult
+// nolint:funlen
 func getVariation[T model.JSONType](
 	g *GoFeatureFlag, flagKey string, evaluationCtx ffcontext.Context, sdkDefaultValue T, expectedType string,
 ) (model.VariationResult[T], error) {
@@ -322,7 +323,9 @@ func getVariation[T model.JSONType](
 		DefaultSdkValue:             sdkDefaultValue,
 		EvaluationContextEnrichment: maps.Clone(g.config.EvaluationContextEnrichment),
 	}
-	flagCtx.AddIntoEvaluationContextEnrichment("env", g.config.Environment)
+	if g.config.Environment != "" {
+		flagCtx.AddIntoEvaluationContextEnrichment("env", g.config.Environment)
+	}
 	flagValue, resolutionDetails := f.Value(flagKey, evaluationCtx, flagCtx)
 
 	var convertedValue interface{}

diff --git a/variation_all_flags.go b/variation_all_flags.go
@@ -34,7 +34,9 @@ func (g *GoFeatureFlag) GetFlagStates(evaluationCtx ffcontext.Context, flagsToEv
 		EvaluationContextEnrichment: g.config.EvaluationContextEnrichment,
 		DefaultSdkValue:             nil,
 	}
-	flagCtx.AddIntoEvaluationContextEnrichment("env", g.config.Environment)
+	if g.config.Environment != "" {
+		flagCtx.AddIntoEvaluationContextEnrichment("env", g.config.Environment)
+	}
 
 	// Evaluate only the flags in flagsToEvaluate
 	if len(flagsToEvaluate) != 0 {

diff --git a/variation_test.go b/variation_test.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/thejerf/slogassert"
 	"github.com/thomaspoignant/go-feature-flag/exporter"
 	"github.com/thomaspoignant/go-feature-flag/exporter/logsexporter"
@@ -13,11 +14,13 @@ import (
 	"github.com/thomaspoignant/go-feature-flag/internal/dto"
 	"github.com/thomaspoignant/go-feature-flag/internal/flag"
 	"github.com/thomaspoignant/go-feature-flag/model"
+	"github.com/thomaspoignant/go-feature-flag/retriever/fileretriever"
 	"github.com/thomaspoignant/go-feature-flag/testutils"
 	"github.com/thomaspoignant/go-feature-flag/testutils/flagv1"
 	"github.com/thomaspoignant/go-feature-flag/testutils/testconvert"
 	"github.com/thomaspoignant/go-feature-flag/utils/fflog"
 	"log/slog"
+	"os"
 	"runtime"
 	"strings"
 	"testing"
@@ -3889,11 +3892,11 @@ func Test_constructMetadataParallel(t *testing.T) {
 	type args struct {
 		resolutionDetails flag.ResolutionDetails
 	}
-	tests := []struct {
+	var tests []struct {
 		name                  string
 		args                  args
 		wantEvaluatedRuleName string
-	}{}
+	}
 
 	runtime.GOMAXPROCS(runtime.NumCPU())
 
@@ -3925,3 +3928,70 @@ func Test_constructMetadataParallel(t *testing.T) {
 		})
 	}
 }
+
+func Test_OverrideContextEnrichmentWithEnvironment(t *testing.T) {
+	tempFile, err := os.CreateTemp("", "")
+	require.NoError(t, err)
+	defer tempFile.Close()
+
+	err = os.WriteFile(tempFile.Name(), []byte(`
+flag1:
+  variations:
+    enabled: true
+    disabled: false
+  targeting:
+    - query: env eq "staging"
+      variation: enabled
+  defaultRule:
+    variation: disabled
+
+`), 0644)
+	require.NoError(t, err)
+
+	goff, err := New(Config{
+		PollingInterval: 500 * time.Millisecond,
+		Retriever:       &fileretriever.Retriever{Path: tempFile.Name()},
+		EvaluationContextEnrichment: map[string]interface{}{
+			"env": "staging",
+		},
+	})
+	require.NoError(t, err)
+
+	res, err1 := goff.BoolVariation("flag1", ffcontext.NewEvaluationContextBuilder("my-key").Build(), false)
+	assert.True(t, res)
+	assert.NoError(t, err1)
+	allFlags := goff.AllFlagsState(ffcontext.NewEvaluationContextBuilder("my-key").Build())
+	assert.Equal(t, true, allFlags.GetFlags()["flag1"].Value)
+
+	goff2, err2 := New(Config{
+		PollingInterval: 500 * time.Millisecond,
+		Retriever:       &fileretriever.Retriever{Path: tempFile.Name()},
+		Environment:     "staging",
+		EvaluationContextEnrichment: map[string]interface{}{
+			"env": "staging",
+		},
+	})
+	require.NoError(t, err2)
+	res2, err3 := goff2.BoolVariation("flag1", ffcontext.NewEvaluationContextBuilder("my-key").Build(), false)
+	assert.True(t, res2)
+	assert.NoError(t, err3)
+	allFlags2 := goff2.AllFlagsState(ffcontext.NewEvaluationContextBuilder("my-key").Build())
+	assert.Equal(t, true, allFlags2.GetFlags()["flag1"].Value)
+
+	// Explicit environment should override the environment from the enrichment
+	goff3, err4 := New(Config{
+		PollingInterval: 500 * time.Millisecond,
+		Retriever:       &fileretriever.Retriever{Path: tempFile.Name()},
+		Environment:     "staging",
+		EvaluationContextEnrichment: map[string]interface{}{
+			"env": "prod",
+		},
+	})
+	require.NoError(t, err4)
+	res3, err5 := goff3.BoolVariation("flag1", ffcontext.NewEvaluationContextBuilder("my-key").Build(), false)
+	assert.True(t, res3)
+	assert.NoError(t, err5)
+
+	allFlags3 := goff3.AllFlagsState(ffcontext.NewEvaluationContextBuilder("my-key").Build())
+	assert.Equal(t, true, allFlags3.GetFlags()["flag1"].Value)
+}