Skip to content

fix: various custom operator conformance fixes #1950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
leakonvalinka wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: various custom operator conformance fixes #1950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f033374
fix: accept uppercase V version prefix
leakonvalinka Apr 17, 2026
a532971
fix: further custom-ops inconsistencies
leakonvalinka Apr 23, 2026
7a844e0
fix: duplication and in-process exclusion tags
leakonvalinka Apr 23, 2026
39c4820
fixup: update go.mod, update use JSON round trip to make $ref regex s…
toddbaert Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions core/pkg/evaluator/fractional.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func parseFractionalEvaluationData(values, data any, logger *logger.Logger) (str
if !ok {
return "", nil, errors.New("fractional evaluation data is not an array")
}
if len(valuesArray) < 1 {
if len(valuesArray) < 1 {
return "", nil, errors.New("fractional evaluation data must contain at least one distribution")
}

Expand Down Expand Up @@ -195,7 +195,7 @@ func parseFractionalEvaluationDistributions(values []any, data any, logger *logg
// It maps a 32-bit hash to the range [0, totalWeight) and finds the variant bucket that contains that value.
func distributeValue(hashValue uint32, feDistribution *fractionalEvaluationDistribution) any {
if feDistribution.totalWeight == 0 {
return ""
return nil
}

bucket := (uint64(hashValue) * uint64(feDistribution.totalWeight)) >> 32
Expand All @@ -209,5 +209,5 @@ func distributeValue(hashValue uint32, feDistribution *fractionalEvaluationDistr
}

// unreachable given validation
return ""
return nil
}
54 changes: 54 additions & 0 deletions core/pkg/evaluator/fractional_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1073,3 +1073,57 @@ func TestFractionalVariantBoolNumericAndOperators(t *testing.T) {
})
}
}

func TestFractionalEvaluation_ErrorFallbackWhenUsedDirectly(t *testing.T) {
const source = "testSource"
ctx := context.Background()

tests := map[string]struct {
targeting string
context map[string]any
}{
"missing bucket key falls back": {
targeting: `{
"fractional": [
{"var": "missing_key"},
["one", 50],
["two", 50]
]
}`,
context: map[string]any{},
},
"all zero weights fall back": {
targeting: `{
"fractional": [
{"var": "targetingKey"},
["one", 0],
["two", 0]
]
}`,
context: map[string]any{"targetingKey": "any-user"},
},
}

for name, tt := range tests {
t.Run(name, func(t *testing.T) {
je, err := setupEvaluator(source, []model.Flag{{
Key: "fractional-op-error-fallback",
State: "ENABLED",
DefaultVariant: "fallback",
Variants: map[string]any{
"one": "one",
"two": "two",
"fallback": "fallback",
},
Targeting: []byte(tt.targeting),
}})
assert.NoError(t, err)

value, variant, reason, _, err := resolve[string](ctx, "default", "fractional-op-error-fallback", tt.context, je.evaluateVariant)
assert.NoError(t, err)
assert.Equal(t, "fallback", value)
assert.Equal(t, "fallback", variant)
assert.Equal(t, model.DefaultReason, reason)
})
}
}
31 changes: 15 additions & 16 deletions core/pkg/evaluator/json.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"encoding/json"
"errors"
"fmt"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -35,12 +34,6 @@ const (
ProtoVersionKey = "__flagd.protoVersion__" // used to mark if the request is coming from an older proto source, which has different fallback behavior
)

var regBrace *regexp.Regexp

func init() {
regBrace = regexp.MustCompile("^[^{]*{|}[^}]*$")
}

func addSchemaResource(compiler *jsonschema.Compiler, url string, schemaData string) error {
unmarshalJSON, err := jsonschema.UnmarshalJSON(strings.NewReader(schemaData))
if err != nil {
Expand Down Expand Up @@ -548,13 +541,18 @@ func transposeEvaluators(state string) (string, error) {
return "", fmt.Errorf("unmarshal: %w", err)
}

for evalName, evalRaw := range evaluators.Evaluators {
// replace any occurrences of "evaluator": "evalName"
regex, err := regexp.Compile(fmt.Sprintf(`"\$ref":(\s)*"%s"`, evalName))
if err != nil {
return "", fmt.Errorf("compile regex: %w", err)
}
// round-trip to normalize whitespace so we can use plain string matching
var raw interface{}
if err := json.Unmarshal([]byte(state), &raw); err != nil {
return "", fmt.Errorf("normalize: %w", err)
}
normalizedBytes, err := json.Marshal(raw)
if err != nil {
return "", fmt.Errorf("normalize marshal: %w", err)
}
result := string(normalizedBytes)

for evalName, evalRaw := range evaluators.Evaluators {
marshalledEval, err := evalRaw.MarshalJSON()
if err != nil {
return "", fmt.Errorf("marshal evaluator: %w", err)
Expand All @@ -564,9 +562,10 @@ func transposeEvaluators(state string) (string, error) {
if len(evalValue) < 3 {
return "", errors.New("evaluator object is empty")
}
evalValue = regBrace.ReplaceAllString(evalValue, "")
state = regex.ReplaceAllString(state, evalValue)

refPattern := `{"$ref":"` + evalName + `"}`
result = strings.ReplaceAll(result, refPattern, evalValue)
}

return state, nil
return result, nil
}
73 changes: 46 additions & 27 deletions core/pkg/evaluator/json_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1145,37 +1145,56 @@ func TestState_Evaluator(t *testing.T) {
},
},
},
"invalid evaluator json": {
"string-valued evaluator": {
// string-valued evaluators are valid; the string is substituted as-is (with quotes)
inputState: `
{
"flags": {
"fibAlgo": {
"variants": {
"recursive": "recursive",
"memo": "memo",
"loop": "loop",
"binet": "binet"
},
"defaultVariant": "recursive",
"state": "ENABLED",
"metadata": {
"flagSetId": "flagSetId"
},
"targeting": {
"if": [
{
"$ref": "emailWithFaas"
}, "binet", null
]
}
}
"fibAlgo": {
"variants": {
"recursive": "recursive",
"memo": "memo",
"loop": "loop",
"binet": "binet"
},
"defaultVariant": "recursive",
"state": "ENABLED",
"metadata": {
"flagSetId": "flagSetId"
},
"targeting": {
"if": [
{
"$ref": "emailWithFaas"
}, "binet", null
]
}
}
},
"$evaluators": {
"emailWithFaas": "foo"
}
}
`,
expectedOutputState: map[string]model.Flag{
"fibAlgo": {
Key: "fibAlgo",
Variants: map[string]any{
"recursive": "recursive",
"memo": "memo",
"loop": "loop",
"binet": "binet",
},
"$evaluators": {
"emailWithFaas": "foo"
}
}
`,
expectedError: true,
DefaultVariant: "recursive",
State: "ENABLED",
Source: "testSource",
Targeting: json.RawMessage(`{"if":["foo","binet",null]}`),
Metadata: map[string]interface{}{
"flagSetId": "flagSetId",
},
FlagSetId: "flagSetId",
},
},
},
"invalid targeting": {
inputState: `
Expand Down
12 changes: 7 additions & 5 deletions core/pkg/evaluator/semver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ func (je *SemVerComparison) SemVerEvaluation(values, _ interface{}) interface{}
actualVersion, targetVersion, operator, err := parseSemverEvaluationData(values)
if err != nil {
je.Logger.Error(fmt.Sprintf("parse sem_ver evaluation data: %v", err))
return false
return nil
}
res, err := operator.compare(actualVersion, targetVersion)
if err != nil {
je.Logger.Error(fmt.Sprintf("sem_ver evaluation: %v", err))
return false
return nil
}
return res
}
Expand All @@ -105,7 +105,7 @@ func parseSemverEvaluationData(values interface{}) (string, string, SemVerOperat
return "", "", "", errors.New("sem_ver evaluation must contain a value, an operator, and a comparison target")
}

actualVersion, err := parseSemanticVersion(parsed[0])
actualVersion, err := normalizeVersion(parsed[0])
if err != nil {
return "", "", "", fmt.Errorf("sem_ver evaluation: could not parse target property value: %w", err)
}
Expand All @@ -115,7 +115,7 @@ func parseSemverEvaluationData(values interface{}) (string, string, SemVerOperat
return "", "", "", fmt.Errorf("sem_ver evaluation: could not parse operator: %w", err)
}

targetVersion, err := parseSemanticVersion(parsed[2])
targetVersion, err := normalizeVersion(parsed[2])
if err != nil {
return "", "", "", fmt.Errorf("sem_ver evaluation: could not parse target value: %w", err)
}
Expand All @@ -131,11 +131,13 @@ func ensureString(v interface{}) string {
return fmt.Sprintf("%v", v)
}

func parseSemanticVersion(v interface{}) (string, error) {
func normalizeVersion(v interface{}) (string, error) {
version := ensureString(v)
// version strings are only valid in the semver package if they start with a 'v'
// if it's not present in the given value, we prepend it
// 'V' is normalized to 'v'
if !strings.HasPrefix(version, "v") {
version = strings.TrimPrefix(version, "V")
version = "v" + version
}

Expand Down
70 changes: 31 additions & 39 deletions core/pkg/evaluator/semver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@ package evaluator

import (
"context"
"errors"
"testing"

"github.com/open-feature/flagd/core/pkg/logger"
"github.com/open-feature/flagd/core/pkg/model"
"github.com/open-feature/flagd/core/pkg/store"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -83,6 +80,16 @@ func TestSemVerOperator_Compare(t *testing.T) {
want: true,
wantErr: false,
},
{
name: "uppercase V prefix equals lowercase or no prefix",
svo: Equals,
args: args{
v1: "V1.0.0",
v2: "1.0.0",
},
want: true,
wantErr: false,
},
{
name: "no prefixed v both",
svo: Greater,
Expand Down Expand Up @@ -320,15 +327,7 @@ func TestJSONEvaluator_semVerEvaluation(t *testing.T) {
var sources = []string{source}
ctx := context.Background()

tests := map[string]struct {
flags []model.Flag
flagKey string
context map[string]any
expectedValue string
expectedVariant string
expectedReason string
expectedError error
}{
tests := map[string]stringFlagEvalTestCase{
"versions and operator provided - match": {
flags: []model.Flag{{
Key: "headerColor",
Expand Down Expand Up @@ -789,34 +788,27 @@ func TestJSONEvaluator_semVerEvaluation(t *testing.T) {
},
}

const reqID = "default"
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
log := logger.NewLogger(nil, false)
s, err := store.NewStore(log, sources)
if err != nil {
t.Fatalf("NewStore failed: %v", err)
}
je := NewJSON(log, s)
je.store.Update(source, tt.flags, model.Metadata{}, false)

value, variant, reason, _, err := resolve[string](ctx, reqID, tt.flagKey, tt.context, je.evaluateVariant)

if value != tt.expectedValue {
t.Errorf("expected value '%s', got '%s'", tt.expectedValue, value)
}

if variant != tt.expectedVariant {
t.Errorf("expected variant '%s', got '%s'", tt.expectedVariant, variant)
}
runStringFlagEvalTests(t, ctx, source, sources, tests)
}

if reason != tt.expectedReason {
t.Errorf("expected reason '%s', got '%s'", tt.expectedReason, reason)
}
func TestSemVerEvaluation_ErrorFallbackWhenUsedDirectly(t *testing.T) {
const source = "testSource"
ctx := context.Background()

if !errors.Is(err, tt.expectedError) {
t.Errorf("expected err '%v', got '%v'", tt.expectedError, err)
}
})
tests := map[string]errorFallbackTestCase{
"invalid context version falls back": {
targeting: `{"sem_ver": [{"var": "version"}, "=", "1.0.0"]}`,
context: map[string]any{"version": "not-a-version"},
},
"invalid operator falls back": {
targeting: `{"sem_ver": [{"var": "version"}, "===", "1.0.0"]}`,
context: map[string]any{"version": "1.0.0"},
},
"wrong arg count falls back": {
targeting: `{"sem_ver": [{"var": "version"}, "="]}`,
context: map[string]any{"version": "1.0.0"},
},
}

runErrorFallbackTests(t, ctx, source, "semver-op-error-fallback", tests)
}
2 changes: 1 addition & 1 deletion core/pkg/evaluator/string_comparison.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (sce *StringComparisonEvaluator) EndsWithEvaluation(values, _ interface{})
propertyValue, target, err := parseStringComparisonEvaluationData(values)
if err != nil {
sce.Logger.Error(fmt.Sprintf("parse ends_with evaluation data: %v", err))
return false
return nil
}
return strings.HasSuffix(propertyValue, target)
}
Expand Down
Loading
Loading