fixup: implement

Signed-off-by: Todd Baert <[email protected]>

core/pkg/eval/fractional_evaluation_test.go

@@ -55,13 +55,13 @@ func TestFractionalEvaluation(t *testing.T) {
 	}
 
 	tests := map[string]struct {
-		flags           Flags
-		flagKey         string
-		context         map[string]any
-		expectedValue   string
-		expectedVariant string
-		expectedReason  string
-		expectedError   error
+		flags             Flags
+		flagKey           string
+		context           map[string]any
+		expectedValue     string
+		expectedVariant   string
+		expectedReason    string
+		expectedErrorCode string
 	}{
 		"[email protected]": {
 			flags:   flags,
@@ -247,38 +247,6 @@ func TestFractionalEvaluation(t *testing.T) {
 			expectedValue:   "#FF0000",
 			expectedReason:  model.DefaultReason,
 		},
-		"fallback to default variant if invalid variant as result of fractional evaluation": {
-			flags: Flags{
-				Flags: map[string]model.Flag{
-					"headerColor": {
-						State:          "ENABLED",
-						DefaultVariant: "red",
-						Variants: map[string]any{
-							"red":    "#FF0000",
-							"blue":   "#0000FF",
-							"green":  "#00FF00",
-							"yellow": "#FFFF00",
-						},
-						Targeting: []byte(`{
-							"fractional": [
-								{"var": "email"},
-								[
-								"black",
-								100
-								]
-							]
-							}`),
-					},
-				},
-			},
-			flagKey: "headerColor",
-			context: map[string]any{
-				"email": "[email protected]",
-			},
-			expectedVariant: "red",
-			expectedValue:   "#FF0000",
-			expectedReason:  model.DefaultReason,
-		},
 		"fallback to default variant if percentages don't sum to 100": {
 			flags: Flags{
 				Flags: map[string]model.Flag{
@@ -379,8 +347,11 @@ func TestFractionalEvaluation(t *testing.T) {
 				t.Errorf("expected reason '%s', got '%s'", tt.expectedReason, reason)
 			}
 
-			if err != tt.expectedError {
-				t.Errorf("expected err '%v', got '%v'", tt.expectedError, err)
+			if err != nil {
+				errorCode := err.Error()
+				if errorCode != tt.expectedErrorCode {
+					t.Errorf("expected err '%v', got '%v'", tt.expectedErrorCode, err)
+				}
 			}
 		})
 	}
@@ -433,13 +404,13 @@ func BenchmarkFractionalEvaluation(b *testing.B) {
 	}
 
 	tests := map[string]struct {
-		flags           Flags
-		flagKey         string
-		context         map[string]any
-		expectedValue   string
-		expectedVariant string
-		expectedReason  string
-		expectedError   error
+		flags             Flags
+		flagKey           string
+		context           map[string]any
+		expectedValue     string
+		expectedVariant   string
+		expectedReason    string
+		expectedErrorCode string
 	}{
 		"[email protected]": {
 			flags:   flags,
@@ -509,8 +480,11 @@ func BenchmarkFractionalEvaluation(b *testing.B) {
 					b.Errorf("expected reason '%s', got '%s'", tt.expectedReason, reason)
 				}
 
-				if err != tt.expectedError {
-					b.Errorf("expected err '%v', got '%v'", tt.expectedError, err)
+				if err != nil {
+					errorCode := err.Error()
+					if errorCode != tt.expectedErrorCode {
+						b.Errorf("expected err '%v', got '%v'", tt.expectedErrorCode, err)
+					}
 				}
 			}
 		})

core/pkg/eval/json_evaluator.go

@@ -341,21 +341,26 @@ func (je *JSONEvaluator) evaluateVariant(reqID string, flagKey string, context m
 			je.Logger.ErrorWithID(reqID, fmt.Sprintf("error applying rules: %s", err))
 			return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.ParseErrorCode)
 		}
+
+		// check if string is "null" before we strip quotes, so we can differentiate between JSON null and "null"
+		trimmed := strings.TrimSpace(result.String())
+		if trimmed == "null" {
+			return flag.DefaultVariant, flag.Variants, model.DefaultReason, metadata, nil
+		}
+
 		// strip whitespace and quotes from the variant
-		variant = strings.ReplaceAll(strings.TrimSpace(result.String()), "\"", "")
+		variant = strings.ReplaceAll(trimmed, "\"", "")
 
 		// if this is a valid variant, return it
 		if _, ok := flag.Variants[variant]; ok {
 			return variant, flag.Variants, model.TargetingMatchReason, metadata, nil
+		} else {
+			je.Logger.ErrorWithID(reqID, fmt.Sprintf("invalid or missing variant: %s for flagKey: %s, variant is not valid", variant, flagKey))
+			return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.ParseErrorCode)
 		}
-
-		je.Logger.DebugWithID(reqID, fmt.Sprintf("returning default variant for flagKey: %s, variant is not valid", flagKey))
-		reason = model.DefaultReason
 	} else {
-		reason = model.StaticReason
+		return flag.DefaultVariant, flag.Variants, model.StaticReason, metadata, nil
 	}
-
-	return flag.DefaultVariant, flag.Variants, reason, metadata, nil
 }
 
 func (je *JSONEvaluator) setFlagdProperties(

core/pkg/eval/json_evaluator_test.go

@@ -1281,3 +1281,97 @@ func TestFlagdAmbientProperties(t *testing.T) {
 		}
 	})
 }
+
+func TestTargetingVariantBehavior(t *testing.T) {
+	t.Run("missing variant error", func(t *testing.T) {
+		evaluator := eval.NewJSONEvaluator(logger.NewLogger(nil, false), store.NewFlags())
+
+		_, _, err := evaluator.SetState(sync.DataSync{FlagData: `{
+			"flags": {
+				"missing-variant": {
+					"state": "ENABLED",
+					"variants": {
+						"foo": true,
+						"bar": false
+					},
+					"defaultVariant": "foo",
+					"targeting": {
+						"if": [ true, "buz", "baz"]
+					}
+				}
+			}
+		}`})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		_, _, _, _, err = evaluator.ResolveBooleanValue(context.Background(), "default", "missing-variant", nil)
+		if err == nil {
+			t.Fatal("missing variant did not result in error")
+		}
+	})
+
+	t.Run("null fallback", func(t *testing.T) {
+		evaluator := eval.NewJSONEvaluator(logger.NewLogger(nil, false), store.NewFlags())
+
+		_, _, err := evaluator.SetState(sync.DataSync{FlagData: `{
+			"flags": {
+				"null-fallback": {
+					"state": "ENABLED",
+					"variants": {
+						"foo": true,
+						"bar": false
+					},
+					"defaultVariant": "foo",
+					"targeting": {
+						"if": [ true, null, "baz"]
+					}
+				}
+			}
+		}`})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		value, variant, reason, _, err := evaluator.ResolveBooleanValue(context.Background(), "default", "null-fallback", nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if !value || variant != "foo" || reason != model.DefaultReason {
+			t.Fatal("did not fallback to defaultValue")
+		}
+	})
+
+	t.Run("match booleans", func(t *testing.T) {
+		evaluator := eval.NewJSONEvaluator(logger.NewLogger(nil, false), store.NewFlags())
+
+		_, _, err := evaluator.SetState(sync.DataSync{FlagData: `{
+			"flags": {
+				"match-boolean": {
+					"state": "ENABLED",
+					"variants": {
+						"false": 1,
+						"true": 2
+					},
+					"defaultVariant": "false",
+					"targeting": {
+						"if": [ true, true, false]
+					}
+				}
+			}
+		}`})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		value, variant, reason, _, err := evaluator.ResolveIntValue(context.Background(), "default", "match-boolean", nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if value != 2 || variant != "true" || reason != model.TargetingMatchReason {
+			t.Fatal("did not map to stringified boolean")
+		}
+	})
+}

core/pkg/eval/legacy_fractional_evaluation_test.go

@@ -55,13 +55,13 @@ func TestLegacyFractionalEvaluation(t *testing.T) {
 	}
 
 	tests := map[string]struct {
-		flags           Flags
-		flagKey         string
-		context         map[string]any
-		expectedValue   string
-		expectedVariant string
-		expectedReason  string
-		expectedError   error
+		flags             Flags
+		flagKey           string
+		context           map[string]any
+		expectedValue     string
+		expectedVariant   string
+		expectedReason    string
+		expectedErrorCode string
 	}{
 		"[email protected]": {
 			flags:   flags,
@@ -188,11 +188,12 @@ func TestLegacyFractionalEvaluation(t *testing.T) {
 					},
 				},
 			},
-			flagKey:         "headerColor",
-			context:         map[string]any{},
-			expectedVariant: "red",
-			expectedValue:   "#FF0000",
-			expectedReason:  model.DefaultReason,
+			flagKey:           "headerColor",
+			context:           map[string]any{},
+			expectedVariant:   "",
+			expectedValue:     "",
+			expectedReason:    model.ErrorReason,
+			expectedErrorCode: model.ParseErrorCode,
 		},
 		"fallback to default variant if invalid variant as result of fractional evaluation": {
 			flags: Flags{
@@ -222,9 +223,10 @@ func TestLegacyFractionalEvaluation(t *testing.T) {
 			context: map[string]any{
 				"email": "[email protected]",
 			},
-			expectedVariant: "red",
-			expectedValue:   "#FF0000",
-			expectedReason:  model.DefaultReason,
+			expectedVariant:   "",
+			expectedValue:     "",
+			expectedReason:    model.ErrorReason,
+			expectedErrorCode: model.ParseErrorCode,
 		},
 		"fallback to default variant if percentages don't sum to 100": {
 			flags: Flags{
@@ -291,8 +293,11 @@ func TestLegacyFractionalEvaluation(t *testing.T) {
 				t.Errorf("expected reason '%s', got '%s'", tt.expectedReason, reason)
 			}
 
-			if err != tt.expectedError {
-				t.Errorf("expected err '%v', got '%v'", tt.expectedError, err)
+			if err != nil {
+				errorCode := err.Error()
+				if errorCode != tt.expectedErrorCode {
+					t.Errorf("expected err '%v', got '%v'", tt.expectedErrorCode, err)
+				}
 			}
 		})
 	}

core/pkg/eval/semver_evaluation.go

@@ -85,12 +85,12 @@ func (je *SemVerComparisonEvaluator) SemVerEvaluation(values, _ interface{}) int
 	actualVersion, targetVersion, operator, err := parseSemverEvaluationData(values)
 	if err != nil {
 		je.Logger.Error(fmt.Sprintf("parse sem_ver evaluation data: %v", err))
-		return nil
+		return false
 	}
 	res, err := operator.compare(actualVersion, targetVersion)
 	if err != nil {
 		je.Logger.Error(fmt.Sprintf("sem_ver evaluation: %v", err))
-		return nil
+		return false
 	}
 	return res
 }

core/pkg/eval/string_comparison_evaluation.go

@@ -72,7 +72,7 @@ func (sce StringComparisonEvaluator) EndsWithEvaluation(values, _ interface{}) i
 	propertyValue, target, err := parseStringComparisonEvaluationData(values)
 	if err != nil {
 		sce.Logger.Error(fmt.Sprintf("parse ends_with evaluation data: %v", err))
-		return nil
+		return false
 	}
 	return strings.HasSuffix(propertyValue, target)
 }

docs/reference/flag-definitions.md

@@ -147,10 +147,17 @@ Example of an invalid configuration:
 `targeting` is an **optional** property.
 A targeting rule **must** be valid JSON.
 Flagd uses a modified version of [JsonLogic](https://jsonlogic.com/), as well as some custom pre-processing, to evaluate these rules.
-The output of the targeting rule **must** match the name of one of the variants defined above.
-If an invalid or null value is returned by the targeting rule, the `defaultVariant` value is used.
 If no targeting rules are defined, the response reason will always be `STATIC`, this allows for the flag values to be cached, this behavior is described [here](specifications/rpc-providers.md#caching).
 
+
+#### Variants From Targeting Rules 
+
+The output of the targeting rule **must** match the name of one of the defined variants.
+Once exception to the above is that rules may return `true` or `false` which will map to the variant indexed by the equivalent string (`"true"`, `"false"`).
+If a null value is returned by the targeting rule, the `defaultVariant` is used.
+This can be useful for conditionally "exiting" targeting rules and falling back to the default (in this case the returned reason will be `DEFAULT`).
+If an invalid variant is returned (not a string, `true`, or `false`, or a string that is not in the set of variants) the evaluation is considered erroneous.
+
 #### Evaluation Context
 
 Evaluation context is included as part of the evaluation request.