feat: move json logic operator registration to resolver (#1291)

## This PR

Attempts to isolate Resolver creation and related logic into a single
component.

Previously, custom Json logic operator registration was done at flagd
using `WithEvaluator` option . But this adds confusion when Resolver
needs to be reused in another component. Further the option was merely
adding the custom operator to json logic through its global methods and
did not perform anything on the evaluator . Given that Resolver is
responsible for flag resolving (core logic of the flag evaluations), I
have deprecated `WithEvaluator` option and added custom json logic
registration as part of the resolver constructor.

I have not removed `JSONEvaluatorOption` option so we have flexibility
to support any future option (ex:-option to have a customized
`IResolver` implemetnation )

Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>

core/pkg/evaluator/fractional_test.go

@@ -394,14 +394,7 @@ func TestFractionalEvaluation(t *testing.T) {
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			log := logger.NewLogger(nil, false)
-			je := NewJSON(
+			je := NewJSON(log, store.NewFlags())
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					FractionEvaluationName,
-					NewFractional(log).Evaluate,
-				),
-			)
 			je.store.Flags = tt.flags.Flags
 
 			value, variant, reason, _, err := resolve[string](ctx, reqID, tt.flagKey, tt.context, je.evaluateVariant)
@@ -530,14 +523,7 @@ func BenchmarkFractionalEvaluation(b *testing.B) {
 	for name, tt := range tests {
 		b.Run(name, func(b *testing.B) {
 			log := logger.NewLogger(nil, false)
-			je := NewJSON(
+			je := NewJSON(log, &store.Flags{Flags: tt.flags.Flags})
-				log,
-				&store.Flags{Flags: tt.flags.Flags},
-				WithEvaluator(
-					FractionEvaluationName,
-					NewFractional(log).Evaluate,
-				),
-			)
 			for i := 0; i < b.N; i++ {
 				value, variant, reason, _, err := resolve[string](
 					ctx, reqID, tt.flagKey, tt.context, je.evaluateVariant)

core/pkg/evaluator/json.go

@@ -55,6 +55,7 @@ type flagdProperties struct {
 type variantEvaluator func(context.Context, string, string, map[string]any) (
 	variant string, variants map[string]interface{}, reason string, metadata map[string]interface{}, error error)
 
+// Deprecated - this will be remove in the next release
 func WithEvaluator(name string, evalFunc func(interface{}, interface{}) interface{}) JSONEvaluatorOption {
 	return func(_ *JSON) {
 		jsonlogic.AddOperator(name, evalFunc)
@@ -145,6 +146,13 @@ type Resolver struct {
 }
 
 func NewResolver(store store.IStore, logger *logger.Logger, jsonEvalTracer trace.Tracer) Resolver {
+	// register supported json logic custom operator implementations
+	jsonlogic.AddOperator(FractionEvaluationName, NewFractional(logger).Evaluate)
+	jsonlogic.AddOperator(StartsWithEvaluationName, NewStringComparisonEvaluator(logger).StartsWithEvaluation)
+	jsonlogic.AddOperator(EndsWithEvaluationName, NewStringComparisonEvaluator(logger).EndsWithEvaluation)
+	jsonlogic.AddOperator(SemVerEvaluationName, NewSemVerComparison(logger).SemVerEvaluation)
+	jsonlogic.AddOperator(LegacyFractionEvaluationName, NewLegacyFractional(logger).LegacyFractionalEvaluation)
+
 	return Resolver{store: store, Logger: logger, tracer: jsonEvalTracer}
 }
 

core/pkg/evaluator/legacy_fractional_test.go

@@ -272,14 +272,7 @@ func TestLegacyFractionalEvaluation(t *testing.T) {
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			log := logger.NewLogger(nil, false)
-			je := NewJSON(
+			je := NewJSON(log, store.NewFlags())
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					"fractionalEvaluation",
-					NewLegacyFractional(log).LegacyFractionalEvaluation,
-				),
-			)
 			je.store.Flags = tt.flags.Flags
 
 			value, variant, reason, _, err := resolve[string](ctx, reqID, tt.flagKey, tt.context, je.evaluateVariant)

core/pkg/evaluator/semver_test.go

@@ -701,14 +701,7 @@ func TestJSONEvaluator_semVerEvaluation(t *testing.T) {
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			log := logger.NewLogger(nil, false)
-			je := NewJSON(
+			je := NewJSON(log, store.NewFlags())
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					SemVerEvaluationName,
-					NewSemVerComparison(log).SemVerEvaluation,
-				),
-			)
 			je.store.Flags = tt.flags.Flags
 
 			value, variant, reason, _, err := resolve[string](ctx, reqID, tt.flagKey, tt.context, je.evaluateVariant)

core/pkg/evaluator/string_comparison.go

@@ -68,7 +68,7 @@ func (sce *StringComparisonEvaluator) StartsWithEvaluation(values, _ interface{}
 //
 // Note that the 'ends_with'  evaluation rule must contain exactly two items, which both resolve to a
 // string value
-func (sce StringComparisonEvaluator) EndsWithEvaluation(values, _ interface{}) interface{} {
+func (sce *StringComparisonEvaluator) EndsWithEvaluation(values, _ interface{}) interface{} {
 	propertyValue, target, err := parseStringComparisonEvaluationData(values)
 	if err != nil {
 		sce.Logger.Error(fmt.Sprintf("parse ends_with evaluation data: %v", err))

core/pkg/evaluator/string_comparison_test.go

@@ -2,6 +2,7 @@ package evaluator
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"testing"
 
@@ -184,14 +185,7 @@ func TestJSONEvaluator_startsWithEvaluation(t *testing.T) {
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			log := logger.NewLogger(nil, false)
-			je := NewJSON(
+			je := NewJSON(log, store.NewFlags())
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					StartsWithEvaluationName,
-					NewStringComparisonEvaluator(log).StartsWithEvaluation,
-				),
-			)
 			je.store.Flags = tt.flags.Flags
 
 			value, variant, reason, _, err := resolve[string](ctx, reqID, tt.flagKey, tt.context, je.evaluateVariant)
@@ -208,7 +202,7 @@ func TestJSONEvaluator_startsWithEvaluation(t *testing.T) {
 				t.Errorf("expected reason '%s', got '%s'", tt.expectedReason, reason)
 			}
 
-			if err != tt.expectedError {
+			if !errors.Is(err, tt.expectedError) {
 				t.Errorf("expected err '%v', got '%v'", tt.expectedError, err)
 			}
 		})
@@ -388,14 +382,7 @@ func TestJSONEvaluator_endsWithEvaluation(t *testing.T) {
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			log := logger.NewLogger(nil, false)
-			je := NewJSON(
+			je := NewJSON(log, store.NewFlags())
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					EndsWithEvaluationName,
-					NewStringComparisonEvaluator(log).EndsWithEvaluation,
-				),
-			)
 
 			je.store.Flags = tt.flags.Flags
 

flagd/pkg/runtime/from_config.go

@@ -74,18 +74,18 @@ func FromConfig(logger *logger.Logger, version string, config Config) (*Runtime,
 	}
 
 	// derive evaluator
-	evaluator := setupJSONEvaluator(logger, s)
+	jsonEvaluator := evaluator.NewJSON(logger, s)
 
 	// derive services
 
 	// connect service
 	connectService := flageval.NewConnectService(
 		logger.WithFields(zap.String("component", "service")),
-		evaluator,
+		jsonEvaluator,
 		recorder)
 
 	// ofrep service
-	ofrepService, err := ofrep.NewOfrepService(evaluator, config.CORS, ofrep.SvcConfiguration{
+	ofrepService, err := ofrep.NewOfrepService(jsonEvaluator, config.CORS, ofrep.SvcConfiguration{
 		Logger: logger.WithFields(zap.String("component", "OFREPService")),
 		Port:   config.OfrepServicePort,
 	})
@@ -118,7 +118,7 @@ func FromConfig(logger *logger.Logger, version string, config Config) (*Runtime,
 
 	return &Runtime{
 		Logger:       logger.WithFields(zap.String("component", "runtime")),
-		Evaluator:    evaluator,
+		Evaluator:    jsonEvaluator,
 		FlagSync:     flagSyncService,
 		OfrepService: ofrepService,
 		Service:      connectService,
@@ -136,35 +136,6 @@ func FromConfig(logger *logger.Logger, version string, config Config) (*Runtime,
 	}, nil
 }
 
-func setupJSONEvaluator(logger *logger.Logger, s *store.Flags) *evaluator.JSON {
-	evaluator := evaluator.NewJSON(
-		logger,
-		s,
-		evaluator.WithEvaluator(
-			evaluator.FractionEvaluationName,
-			evaluator.NewFractional(logger).Evaluate,
-		),
-		evaluator.WithEvaluator(
-			evaluator.StartsWithEvaluationName,
-			evaluator.NewStringComparisonEvaluator(logger).StartsWithEvaluation,
-		),
-		evaluator.WithEvaluator(
-			evaluator.EndsWithEvaluationName,
-			evaluator.NewStringComparisonEvaluator(logger).EndsWithEvaluation,
-		),
-		evaluator.WithEvaluator(
-			evaluator.SemVerEvaluationName,
-			evaluator.NewSemVerComparison(logger).SemVerEvaluation,
-		),
-		// deprecated: will be removed before v1!
-		evaluator.WithEvaluator(
-			evaluator.LegacyFractionEvaluationName,
-			evaluator.NewLegacyFractional(logger).LegacyFractionalEvaluation,
-		),
-	)
-	return evaluator
-}
-
 // syncProvidersFromConfig is a helper to build ISync implementations from SourceConfig
 func syncProvidersFromConfig(logger *logger.Logger, sources []sync.SourceConfig) ([]sync.ISync, error) {
 	builder := syncbuilder.NewSyncBuilder()

flagd/pkg/runtime/from_config_test.go

@@ -1,16 +0,0 @@
-package runtime
-
-import (
-	"testing"
-
-	"github.com/open-feature/flagd/core/pkg/logger"
-	"github.com/open-feature/flagd/core/pkg/store"
-	"github.com/stretchr/testify/require"
-)
-
-func Test_setupJSONEvaluator(t *testing.T) {
-	lg := logger.NewLogger(nil, false)
-
-	je := setupJSONEvaluator(lg, store.NewFlags())
-	require.NotNil(t, je)
-}