fix: missing/nil custom variables in fractional operator (#1295)

## This PR Fixes #1285 Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
open-feature · Apr 19, 2024 · 418c5cd · 418c5cd
1 parent 281c809
commit 418c5cd
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 10 deletions.
diff --git a/core/pkg/evaluator/fractional.go b/core/pkg/evaluator/fractional.go
@@ -56,6 +56,11 @@ func parseFractionalEvaluationData(values, data any) (string, []fractionalEvalua
 	if ok {
 		valuesArray = valuesArray[1:]
 	} else {
+		// check for nil here as custom property could be nil/missing
+		if valuesArray[0] == nil {
+			valuesArray = valuesArray[1:]
+		}
+
 		targetingKey, ok := dataMap[targetingKeyKey].(string)
 		if !ok {
 			return "", nil, errors.New("bucketing value not supplied and no targetingKey in context")
@@ -78,7 +83,8 @@ func parseFractionalEvaluationDistributions(values []any) ([]fractionalEvaluatio
 	for i := 0; i < len(values); i++ {
 		distributionArray, ok := values[i].([]any)
 		if !ok {
-			return nil, errors.New("distribution elements aren't of type []any")
+			return nil, errors.New("distribution elements aren't of type []any. " +
+				"please check your rule in flag definition")
 		}
 
 		if len(distributionArray) != 2 {

diff --git a/core/pkg/evaluator/fractional_test.go b/core/pkg/evaluator/fractional_test.go
@@ -12,7 +12,7 @@ import (
 func TestFractionalEvaluation(t *testing.T) {
 	ctx := context.Background()
 
-	flags := Flags{
+	commonFlags := Flags{
 		Flags: map[string]model.Flag{
 			"headerColor": {
 				State:          "ENABLED",
@@ -95,7 +95,7 @@ func TestFractionalEvaluation(t *testing.T) {
 		expectedErrorCode string
 	}{
 		"rachel@faas.com": {
-			flags:   flags,
+			flags:   commonFlags,
 			flagKey: "headerColor",
 			context: map[string]any{
 				"email": "rachel@faas.com",
@@ -105,7 +105,7 @@ func TestFractionalEvaluation(t *testing.T) {
 			expectedReason:  model.TargetingMatchReason,
 		},
 		"monica@faas.com": {
-			flags:   flags,
+			flags:   commonFlags,
 			flagKey: "headerColor",
 			context: map[string]any{
 				"email": "monica@faas.com",
@@ -115,7 +115,7 @@ func TestFractionalEvaluation(t *testing.T) {
 			expectedReason:  model.TargetingMatchReason,
 		},
 		"joey@faas.com": {
-			flags:   flags,
+			flags:   commonFlags,
 			flagKey: "headerColor",
 			context: map[string]any{
 				"email": "joey@faas.com",
@@ -125,7 +125,7 @@ func TestFractionalEvaluation(t *testing.T) {
 			expectedReason:  model.TargetingMatchReason,
 		},
 		"ross@faas.com": {
-			flags:   flags,
+			flags:   commonFlags,
 			flagKey: "headerColor",
 			context: map[string]any{
 				"email": "ross@faas.com",
@@ -135,7 +135,7 @@ func TestFractionalEvaluation(t *testing.T) {
 			expectedReason:  model.TargetingMatchReason,
 		},
 		"rachel@faas.com with custom seed": {
-			flags:   flags,
+			flags:   commonFlags,
 			flagKey: "customSeededHeaderColor",
 			context: map[string]any{
 				"email": "rachel@faas.com",
@@ -145,7 +145,7 @@ func TestFractionalEvaluation(t *testing.T) {
 			expectedReason:  model.TargetingMatchReason,
 		},
 		"monica@faas.com with custom seed": {
-			flags:   flags,
+			flags:   commonFlags,
 			flagKey: "customSeededHeaderColor",
 			context: map[string]any{
 				"email": "monica@faas.com",
@@ -155,7 +155,7 @@ func TestFractionalEvaluation(t *testing.T) {
 			expectedReason:  model.TargetingMatchReason,
 		},
 		"joey@faas.com with custom seed": {
-			flags:   flags,
+			flags:   commonFlags,
 			flagKey: "customSeededHeaderColor",
 			context: map[string]any{
 				"email": "joey@faas.com",
@@ -165,7 +165,7 @@ func TestFractionalEvaluation(t *testing.T) {
 			expectedReason:  model.TargetingMatchReason,
 		},
 		"ross@faas.com with custom seed": {
-			flags:   flags,
+			flags:   commonFlags,
 			flagKey: "customSeededHeaderColor",
 			context: map[string]any{
 				"email": "ross@faas.com",
@@ -389,6 +389,35 @@ func TestFractionalEvaluation(t *testing.T) {
 			expectedValue:   "#0000FF",
 			expectedReason:  model.TargetingMatchReason,
 		},
+		"missing email - parser should ignore nil/missing custom variables and continue": {
+			flags: Flags{
+				Flags: map[string]model.Flag{
+					"headerColor": {
+						State:          "ENABLED",
+						DefaultVariant: "red",
+						Variants: map[string]any{
+							"red":  "#FF0000",
+							"blue": "#0000FF",
+						},
+						Targeting: []byte(
+							`{
+								"fractional": [
+									{"var": "email"},
+									["red",50],
+									["blue",50]
+								]
+							}`),
+					},
+				},
+			},
+			flagKey: "headerColor",
+			context: map[string]any{
+				"targetingKey": "foo@foo.com",
+			},
+			expectedVariant: "red",
+			expectedValue:   "#FF0000",
+			expectedReason:  model.TargetingMatchReason,
+		},
 	}
 	const reqID = "default"
 	for name, tt := range tests {