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 <[email protected]>
open-feature · Apr 18, 2024 · b473457 · b473457
1 parent 06584a7
commit b473457
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 99 deletions.
diff --git a/core/pkg/evaluator/fractional_test.go b/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(
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					FractionEvaluationName,
-					NewFractional(log).Evaluate,
-				),
-			)
+			je := NewJSON(log, store.NewFlags())
 			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(
-				log,
-				&store.Flags{Flags: tt.flags.Flags},
-				WithEvaluator(
-					FractionEvaluationName,
-					NewFractional(log).Evaluate,
-				),
-			)
+			je := NewJSON(log, &store.Flags{Flags: tt.flags.Flags})
 			for i := 0; i < b.N; i++ {
 				value, variant, reason, _, err := resolve[string](
 					ctx, reqID, tt.flagKey, tt.context, je.evaluateVariant)

diff --git a/core/pkg/evaluator/json.go b/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}
 }
 

diff --git a/core/pkg/evaluator/legacy_fractional_test.go b/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(
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					"fractionalEvaluation",
-					NewLegacyFractional(log).LegacyFractionalEvaluation,
-				),
-			)
+			je := NewJSON(log, store.NewFlags())
 			je.store.Flags = tt.flags.Flags
 
 			value, variant, reason, _, err := resolve[string](ctx, reqID, tt.flagKey, tt.context, je.evaluateVariant)

diff --git a/core/pkg/evaluator/semver_test.go b/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(
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					SemVerEvaluationName,
-					NewSemVerComparison(log).SemVerEvaluation,
-				),
-			)
+			je := NewJSON(log, store.NewFlags())
 			je.store.Flags = tt.flags.Flags
 
 			value, variant, reason, _, err := resolve[string](ctx, reqID, tt.flagKey, tt.context, je.evaluateVariant)

diff --git a/core/pkg/evaluator/string_comparison.go b/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))

diff --git a/core/pkg/evaluator/string_comparison_test.go b/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(
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					StartsWithEvaluationName,
-					NewStringComparisonEvaluator(log).StartsWithEvaluation,
-				),
-			)
+			je := NewJSON(log, store.NewFlags())
 			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(
-				log,
-				store.NewFlags(),
-				WithEvaluator(
-					EndsWithEvaluationName,
-					NewStringComparisonEvaluator(log).EndsWithEvaluation,
-				),
-			)
+			je := NewJSON(log, store.NewFlags())
 
 			je.store.Flags = tt.flags.Flags
 

diff --git a/flagd/pkg/runtime/from_config.go b/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()

diff --git a/flagd/pkg/runtime/from_config_test.go b/flagd/pkg/runtime/from_config_test.go