From 0e447ecb873f427e60a8314391852c93f99b1822 Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Tue, 27 Jun 2023 13:25:30 -0500 Subject: [PATCH 01/12] feat: implement gRPC server for the Variant RPC method --- internal/cmd/grpc.go | 3 + internal/cmd/http.go | 13 +- internal/server/evaluation/evaluation.go | 32 + .../server/evaluation/evaluation_server.go | 30 + internal/server/evaluation/evaluator.go | 433 +++++++++ internal/server/evaluation/evaluator_test.go | 736 +++++++++++++++ internal/server/evaluator.go | 406 +-------- internal/server/evaluator_test.go | 836 +----------------- internal/server/middleware/grpc/middleware.go | 23 + internal/server/server.go | 7 +- 10 files changed, 1296 insertions(+), 1223 deletions(-) create mode 100644 internal/server/evaluation/evaluation.go create mode 100644 internal/server/evaluation/evaluation_server.go create mode 100644 internal/server/evaluation/evaluator.go create mode 100644 internal/server/evaluation/evaluator_test.go diff --git a/internal/cmd/grpc.go b/internal/cmd/grpc.go index 9f6598fbeb..460f5897c0 100644 --- a/internal/cmd/grpc.go +++ b/internal/cmd/grpc.go @@ -20,6 +20,7 @@ import ( "go.flipt.io/flipt/internal/server/cache" "go.flipt.io/flipt/internal/server/cache/memory" "go.flipt.io/flipt/internal/server/cache/redis" + evaluation "go.flipt.io/flipt/internal/server/evaluation" "go.flipt.io/flipt/internal/server/metadata" middlewaregrpc "go.flipt.io/flipt/internal/server/middleware/grpc" "go.flipt.io/flipt/internal/storage" @@ -343,6 +344,8 @@ func NewGRPCServer( register.Add(fliptserver.New(logger, store)) register.Add(metadata.NewServer(cfg, info)) + register.Add(evaluation.NewEvaluateServer(logger, store)) + // initialize grpc server server.Server = grpc.NewServer(grpcOpts...) diff --git a/internal/cmd/http.go b/internal/cmd/http.go index 7f6c88b19e..67d7feaa42 100644 --- a/internal/cmd/http.go +++ b/internal/cmd/http.go @@ -21,6 +21,7 @@ import ( "go.flipt.io/flipt/internal/gateway" "go.flipt.io/flipt/internal/info" "go.flipt.io/flipt/rpc/flipt" + "go.flipt.io/flipt/rpc/flipt/evaluation" "go.flipt.io/flipt/rpc/flipt/meta" "go.flipt.io/flipt/ui" "go.uber.org/zap" @@ -54,9 +55,10 @@ func NewHTTPServer( } isConsole = cfg.Log.Encoding == config.LogEncodingConsole - r = chi.NewRouter() - api = gateway.NewGatewayServeMux(logger) - httpPort = cfg.Server.HTTPPort + r = chi.NewRouter() + api = gateway.NewGatewayServeMux(logger) + evaluateAPI = gateway.NewGatewayServeMux(logger) + httpPort = cfg.Server.HTTPPort ) if cfg.Server.Protocol == config.HTTPS { @@ -67,6 +69,10 @@ func NewHTTPServer( return nil, fmt.Errorf("registering grpc gateway: %w", err) } + if err := evaluation.RegisterEvaluationServiceHandler(ctx, evaluateAPI, conn); err != nil { + return nil, fmt.Errorf("registering grpc gateway: %w", err) + } + if cfg.Cors.Enabled { cors := cors.New(cors.Options{ AllowedOrigins: cfg.Cors.AllowedOrigins, @@ -128,6 +134,7 @@ func NewHTTPServer( } r.Mount("/api/v1", api) + r.Mount("/evaluate/v1", evaluateAPI) // mount all authentication related HTTP components // to the chi router. diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go new file mode 100644 index 0000000000..6a0e4c785a --- /dev/null +++ b/internal/server/evaluation/evaluation.go @@ -0,0 +1,32 @@ +package evaluation + +import ( + "context" + + "go.flipt.io/flipt/rpc/flipt" + rpcEvaluation "go.flipt.io/flipt/rpc/flipt/evaluation" +) + +// Variant evaluates a request for a multi-variate flag and entity. +func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.VariantEvaluationResponse, error) { + resp, err := e.mvEvaluator.Evaluate(ctx, &flipt.EvaluationRequest{ + RequestId: v.RequestId, + FlagKey: v.FlagKey, + EntityId: v.EntityId, + Context: v.Context, + NamespaceKey: v.NamespaceKey, + }) + if err != nil { + return nil, err + } + + ver := &rpcEvaluation.VariantEvaluationResponse{ + Match: resp.Match, + SegmentKey: resp.SegmentKey, + Reason: rpcEvaluation.EvaluationReason(resp.Reason), + VariantKey: resp.Value, + VariantAttachment: resp.Attachment, + } + + return ver, nil +} diff --git a/internal/server/evaluation/evaluation_server.go b/internal/server/evaluation/evaluation_server.go new file mode 100644 index 0000000000..5068f8f971 --- /dev/null +++ b/internal/server/evaluation/evaluation_server.go @@ -0,0 +1,30 @@ +package evaluation + +import ( + "go.flipt.io/flipt/internal/storage" + rpcEvalution "go.flipt.io/flipt/rpc/flipt/evaluation" + "go.uber.org/zap" + "google.golang.org/grpc" +) + +// EvaluateServer serves the Flipt evaluate v2 gRPC Server. +type EvaluateServer struct { + logger *zap.Logger + store storage.Store + mvEvaluator MultiVariateEvaluator + rpcEvalution.UnimplementedEvaluationServiceServer +} + +// NewEvaluateServer is constructs a new EvaluateServer. +func NewEvaluateServer(logger *zap.Logger, store storage.Store) *EvaluateServer { + return &EvaluateServer{ + logger: logger, + store: store, + mvEvaluator: NewEvaluator(logger, store), + } +} + +// RegisterGRPC registers the EvaluateServer onto the provided gRPC Server. +func (e *EvaluateServer) RegisterGRPC(server *grpc.Server) { + rpcEvalution.RegisterEvaluationServiceServer(server, e) +} diff --git a/internal/server/evaluation/evaluator.go b/internal/server/evaluation/evaluator.go new file mode 100644 index 0000000000..51d177fbd7 --- /dev/null +++ b/internal/server/evaluation/evaluator.go @@ -0,0 +1,433 @@ +package evaluation + +import ( + "context" + "errors" + "hash/crc32" + "sort" + "strconv" + "strings" + "time" + + errs "go.flipt.io/flipt/errors" + "go.flipt.io/flipt/internal/server/metrics" + "go.flipt.io/flipt/internal/storage" + "go.flipt.io/flipt/rpc/flipt" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + "go.uber.org/zap" +) + +// Evaluator is an implementation of the MultiVariateEvaluator. +type Evaluator struct { + logger *zap.Logger + store storage.Store +} + +// NewEvalutor is the constructor for an Evaluator. +func NewEvaluator(logger *zap.Logger, store storage.Store) *Evaluator { + return &Evaluator{ + logger: logger, + store: store, + } +} + +// MultiVariateEvaluator is an abstraction for evaluating a flag against a set of rules for multi-variate flags. +type MultiVariateEvaluator interface { + Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (*flipt.EvaluationResponse, error) +} + +func (e *Evaluator) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (resp *flipt.EvaluationResponse, err error) { + var ( + startTime = time.Now().UTC() + namespaceAttr = metrics.AttributeNamespace.String(r.NamespaceKey) + flagAttr = metrics.AttributeFlag.String(r.FlagKey) + ) + + metrics.EvaluationsTotal.Add(ctx, 1, metric.WithAttributeSet(attribute.NewSet(namespaceAttr, flagAttr))) + + defer func() { + if err == nil { + metrics.EvaluationResultsTotal.Add(ctx, 1, + metric.WithAttributeSet( + attribute.NewSet( + namespaceAttr, + flagAttr, + metrics.AttributeMatch.Bool(resp.Match), + metrics.AttributeSegment.String(resp.SegmentKey), + metrics.AttributeReason.String(resp.Reason.String()), + metrics.AttributeValue.String(resp.Value), + ), + ), + ) + } else { + metrics.EvaluationErrorsTotal.Add(ctx, 1, metric.WithAttributeSet(attribute.NewSet(namespaceAttr, flagAttr))) + } + + metrics.EvaluationLatency.Record( + ctx, + float64(time.Since(startTime).Nanoseconds())/1e6, + metric.WithAttributeSet( + attribute.NewSet( + namespaceAttr, + flagAttr, + ), + ), + ) + }() + + resp = &flipt.EvaluationResponse{ + RequestId: r.RequestId, + EntityId: r.EntityId, + RequestContext: r.Context, + FlagKey: r.FlagKey, + NamespaceKey: r.NamespaceKey, + } + + flag, err := e.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) + if err != nil { + resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON + + var errnf errs.ErrNotFound + if errors.As(err, &errnf) { + resp.Reason = flipt.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON + } + + return resp, err + } + + if !flag.Enabled { + resp.Match = false + resp.Reason = flipt.EvaluationReason_FLAG_DISABLED_EVALUATION_REASON + return resp, nil + } + + rules, err := e.store.GetEvaluationRules(ctx, r.NamespaceKey, r.FlagKey) + if err != nil { + resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON + return resp, err + } + + if len(rules) == 0 { + e.logger.Debug("no rules match") + return resp, nil + } + + var lastRank int32 + + // rule loop + for _, rule := range rules { + if rule.Rank < lastRank { + resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON + return resp, errs.ErrInvalidf("rule rank: %d detected out of order", rule.Rank) + } + + lastRank = rule.Rank + + constraintMatches := 0 + + // constraint loop + for _, c := range rule.Constraints { + v := r.Context[c.Property] + + var ( + match bool + err error + ) + + switch c.Type { + case flipt.ComparisonType_STRING_COMPARISON_TYPE: + match = matchesString(c, v) + case flipt.ComparisonType_NUMBER_COMPARISON_TYPE: + match, err = matchesNumber(c, v) + case flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE: + match, err = matchesBool(c, v) + case flipt.ComparisonType_DATETIME_COMPARISON_TYPE: + match, err = matchesDateTime(c, v) + default: + resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON + return resp, errs.ErrInvalid("unknown constraint type") + } + + if err != nil { + resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON + return resp, err + } + + if match { + e.logger.Debug("constraint matches", zap.Reflect("constraint", c)) + + // increase the matchCount + constraintMatches++ + + switch rule.SegmentMatchType { + case flipt.MatchType_ANY_MATCH_TYPE: + // can short circuit here since we had at least one match + break + default: + // keep looping as we need to match all constraints + continue + } + } else { + // no match + e.logger.Debug("constraint does not match", zap.Reflect("constraint", c)) + + switch rule.SegmentMatchType { + case flipt.MatchType_ALL_MATCH_TYPE: + // we can short circuit because we must match all constraints + break + default: + // keep looping to see if we match the next constraint + continue + } + } + } // end constraint loop + + switch rule.SegmentMatchType { + case flipt.MatchType_ALL_MATCH_TYPE: + if len(rule.Constraints) != constraintMatches { + // all constraints did not match, continue to next rule + e.logger.Debug("did not match ALL constraints") + continue + } + + case flipt.MatchType_ANY_MATCH_TYPE: + if len(rule.Constraints) > 0 && constraintMatches == 0 { + // no constraints matched, continue to next rule + e.logger.Debug("did not match ANY constraints") + continue + } + default: + e.logger.Error("unknown match type", zap.Int32("match_type", int32(rule.SegmentMatchType))) + continue + } + + // otherwise, this is our matching rule, determine the flag variant to return + // based on the distributions + resp.SegmentKey = rule.SegmentKey + + distributions, err := e.store.GetEvaluationDistributions(ctx, rule.ID) + if err != nil { + resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON + return resp, err + } + + var ( + validDistributions []*storage.EvaluationDistribution + buckets []int + ) + + for _, d := range distributions { + // don't include 0% rollouts + if d.Rollout > 0 { + validDistributions = append(validDistributions, d) + + if buckets == nil { + bucket := int(d.Rollout * percentMultiplier) + buckets = append(buckets, bucket) + } else { + bucket := buckets[len(buckets)-1] + int(d.Rollout*percentMultiplier) + buckets = append(buckets, bucket) + } + } + } + + // no distributions for rule + if len(validDistributions) == 0 { + e.logger.Info("no distributions for rule") + resp.Match = true + resp.Reason = flipt.EvaluationReason_MATCH_EVALUATION_REASON + return resp, nil + } + + var ( + bucket = crc32Num(r.EntityId, r.FlagKey) + // sort.SearchInts searches for x in a sorted slice of ints and returns the index + // as specified by Search. The return value is the index to insert x if x is + // not present (it could be len(a)). + index = sort.SearchInts(buckets, int(bucket)+1) + ) + + // if index is outside of our existing buckets then it does not match any distribution + if index == len(validDistributions) { + resp.Match = false + e.logger.Debug("did not match any distributions") + return resp, nil + } + + d := validDistributions[index] + e.logger.Debug("matched distribution", zap.Reflect("evaluation_distribution", d)) + + resp.Match = true + resp.Value = d.VariantKey + resp.Attachment = d.VariantAttachment + resp.Reason = flipt.EvaluationReason_MATCH_EVALUATION_REASON + return resp, nil + } // end rule loop + + return resp, nil +} + +func crc32Num(entityID string, salt string) uint { + return uint(crc32.ChecksumIEEE([]byte(salt+entityID))) % totalBucketNum +} + +const ( + // totalBucketNum represents how many buckets we can use to determine the consistent hashing + // distribution and rollout + totalBucketNum uint = 1000 + + // percentMultiplier implies that the multiplier between percentage (100) and totalBucketNum + percentMultiplier float32 = float32(totalBucketNum) / 100 +) + +func matchesString(c storage.EvaluationConstraint, v string) bool { + switch c.Operator { + case flipt.OpEmpty: + return len(strings.TrimSpace(v)) == 0 + case flipt.OpNotEmpty: + return len(strings.TrimSpace(v)) != 0 + } + + if v == "" { + return false + } + + value := c.Value + + switch c.Operator { + case flipt.OpEQ: + return value == v + case flipt.OpNEQ: + return value != v + case flipt.OpPrefix: + return strings.HasPrefix(strings.TrimSpace(v), value) + case flipt.OpSuffix: + return strings.HasSuffix(strings.TrimSpace(v), value) + } + + return false +} + +func matchesNumber(c storage.EvaluationConstraint, v string) (bool, error) { + switch c.Operator { + case flipt.OpNotPresent: + return len(strings.TrimSpace(v)) == 0, nil + case flipt.OpPresent: + return len(strings.TrimSpace(v)) != 0, nil + } + + // can't parse an empty string + if v == "" { + return false, nil + } + + n, err := strconv.ParseFloat(v, 64) + if err != nil { + return false, errs.ErrInvalidf("parsing number from %q", v) + } + + // TODO: we should consider parsing this at creation time since it doesn't change and it doesnt make sense to allow invalid constraint values + value, err := strconv.ParseFloat(c.Value, 64) + if err != nil { + return false, errs.ErrInvalidf("parsing number from %q", c.Value) + } + + switch c.Operator { + case flipt.OpEQ: + return value == n, nil + case flipt.OpNEQ: + return value != n, nil + case flipt.OpLT: + return n < value, nil + case flipt.OpLTE: + return n <= value, nil + case flipt.OpGT: + return n > value, nil + case flipt.OpGTE: + return n >= value, nil + } + + return false, nil +} + +func matchesBool(c storage.EvaluationConstraint, v string) (bool, error) { + switch c.Operator { + case flipt.OpNotPresent: + return len(strings.TrimSpace(v)) == 0, nil + case flipt.OpPresent: + return len(strings.TrimSpace(v)) != 0, nil + } + + // can't parse an empty string + if v == "" { + return false, nil + } + + value, err := strconv.ParseBool(v) + if err != nil { + return false, errs.ErrInvalidf("parsing boolean from %q", v) + } + + switch c.Operator { + case flipt.OpTrue: + return value, nil + case flipt.OpFalse: + return !value, nil + } + + return false, nil +} + +func matchesDateTime(c storage.EvaluationConstraint, v string) (bool, error) { + switch c.Operator { + case flipt.OpNotPresent: + return len(strings.TrimSpace(v)) == 0, nil + case flipt.OpPresent: + return len(strings.TrimSpace(v)) != 0, nil + } + + // can't parse an empty string + if v == "" { + return false, nil + } + + d, err := tryParseDateTime(v) + if err != nil { + return false, err + } + + value, err := tryParseDateTime(c.Value) + if err != nil { + return false, err + } + + switch c.Operator { + case flipt.OpEQ: + return value.Equal(d), nil + case flipt.OpNEQ: + return !value.Equal(d), nil + case flipt.OpLT: + return d.Before(value), nil + case flipt.OpLTE: + return d.Before(value) || value.Equal(d), nil + case flipt.OpGT: + return d.After(value), nil + case flipt.OpGTE: + return d.After(value) || value.Equal(d), nil + } + + return false, nil +} + +func tryParseDateTime(v string) (time.Time, error) { + if d, err := time.Parse(time.RFC3339, v); err == nil { + return d.UTC(), nil + } + + if d, err := time.Parse(time.DateOnly, v); err == nil { + return d.UTC(), nil + } + + return time.Time{}, errs.ErrInvalidf("parsing datetime from %q", v) +} diff --git a/internal/server/evaluation/evaluator_test.go b/internal/server/evaluation/evaluator_test.go new file mode 100644 index 0000000000..e9e49088cf --- /dev/null +++ b/internal/server/evaluation/evaluator_test.go @@ -0,0 +1,736 @@ +package evaluation + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + errs "go.flipt.io/flipt/errors" + "go.flipt.io/flipt/internal/storage" +) + +func Test_matchesString(t *testing.T) { + tests := []struct { + name string + constraint storage.EvaluationConstraint + value string + wantMatch bool + }{ + { + name: "eq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "bar", + }, + value: "bar", + wantMatch: true, + }, + { + name: "negative eq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "bar", + }, + value: "baz", + }, + { + name: "neq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "neq", + Value: "bar", + }, + value: "baz", + wantMatch: true, + }, + { + name: "negative neq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "neq", + Value: "bar", + }, + value: "bar", + }, + { + name: "empty", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "empty", + }, + value: " ", + wantMatch: true, + }, + { + name: "negative empty", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "empty", + }, + value: "bar", + }, + { + name: "not empty", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "notempty", + }, + value: "bar", + wantMatch: true, + }, + { + name: "negative not empty", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "notempty", + }, + value: "", + }, + { + name: "unknown operator", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "foo", + Value: "bar", + }, + value: "bar", + }, + { + name: "prefix", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "prefix", + Value: "ba", + }, + value: "bar", + wantMatch: true, + }, + { + name: "negative prefix", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "prefix", + Value: "bar", + }, + value: "nope", + }, + { + name: "suffix", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "suffix", + Value: "ar", + }, + value: "bar", + wantMatch: true, + }, + { + name: "negative suffix", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "suffix", + Value: "bar", + }, + value: "nope", + }, + } + for _, tt := range tests { + var ( + constraint = tt.constraint + value = tt.value + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + match := matchesString(constraint, value) + assert.Equal(t, wantMatch, match) + }) + } +} + +func Test_matchesNumber(t *testing.T) { + tests := []struct { + name string + constraint storage.EvaluationConstraint + value string + wantMatch bool + wantErr bool + }{ + { + name: "present", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "present", + }, + value: "1", + wantMatch: true, + }, + { + name: "negative present", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "present", + }, + }, + { + name: "not present", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "notpresent", + }, + wantMatch: true, + }, + { + name: "negative notpresent", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "notpresent", + }, + value: "1", + }, + { + name: "NAN constraint value", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "bar", + }, + value: "5", + wantErr: true, + }, + { + name: "NAN context value", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "5", + }, + value: "foo", + wantErr: true, + }, + { + name: "eq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "42.0", + }, + value: "42.0", + wantMatch: true, + }, + { + name: "negative eq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "42.0", + }, + value: "50", + }, + { + name: "neq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "neq", + Value: "42.0", + }, + value: "50", + wantMatch: true, + }, + { + name: "negative neq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "neq", + Value: "42.0", + }, + value: "42.0", + }, + { + name: "lt", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "lt", + Value: "42.0", + }, + value: "8", + wantMatch: true, + }, + { + name: "negative lt", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "lt", + Value: "42.0", + }, + value: "50", + }, + { + name: "lte", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "lte", + Value: "42.0", + }, + value: "42.0", + wantMatch: true, + }, + { + name: "negative lte", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "lte", + Value: "42.0", + }, + value: "102.0", + }, + { + name: "gt", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "gt", + Value: "10.11", + }, + value: "10.12", + wantMatch: true, + }, + { + name: "negative gt", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "gt", + Value: "10.11", + }, + value: "1", + }, + { + name: "gte", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "gte", + Value: "10.11", + }, + value: "10.11", + wantMatch: true, + }, + { + name: "negative gte", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "gte", + Value: "10.11", + }, + value: "0.11", + }, + { + name: "empty value", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "0.11", + }, + }, + { + name: "unknown operator", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "foo", + Value: "0.11", + }, + value: "0.11", + }, + { + name: "negative suffix empty value", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "suffix", + Value: "bar", + }, + }, + } + + for _, tt := range tests { + var ( + constraint = tt.constraint + value = tt.value + wantMatch = tt.wantMatch + wantErr = tt.wantErr + ) + + t.Run(tt.name, func(t *testing.T) { + match, err := matchesNumber(constraint, value) + + if wantErr { + require.Error(t, err) + var ierr errs.ErrInvalid + require.ErrorAs(t, err, &ierr) + return + } + + require.NoError(t, err) + assert.Equal(t, wantMatch, match) + }) + } +} + +func Test_matchesBool(t *testing.T) { + tests := []struct { + name string + constraint storage.EvaluationConstraint + value string + wantMatch bool + wantErr bool + }{ + { + name: "present", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "present", + }, + value: "true", + wantMatch: true, + }, + { + name: "negative present", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "present", + }, + }, + { + name: "not present", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "notpresent", + }, + wantMatch: true, + }, + { + name: "negative notpresent", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "notpresent", + }, + value: "true", + }, + { + name: "not a bool", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "true", + }, + value: "foo", + wantErr: true, + }, + { + name: "is true", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "true", + }, + value: "true", + wantMatch: true, + }, + { + name: "negative is true", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "true", + }, + value: "false", + }, + { + name: "is false", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "false", + }, + value: "false", + wantMatch: true, + }, + { + name: "negative is false", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "false", + }, + value: "true", + }, + { + name: "empty value", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "false", + }, + }, + { + name: "unknown operator", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "foo", + }, + value: "true", + }, + } + + for _, tt := range tests { + var ( + constraint = tt.constraint + value = tt.value + wantMatch = tt.wantMatch + wantErr = tt.wantErr + ) + + t.Run(tt.name, func(t *testing.T) { + match, err := matchesBool(constraint, value) + + if wantErr { + require.Error(t, err) + var ierr errs.ErrInvalid + require.ErrorAs(t, err, &ierr) + return + } + + require.NoError(t, err) + assert.Equal(t, wantMatch, match) + }) + } +} + +func Test_matchesDateTime(t *testing.T) { + tests := []struct { + name string + constraint storage.EvaluationConstraint + value string + wantMatch bool + wantErr bool + }{ + { + name: "present", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "present", + }, + value: "2006-01-02T15:04:05Z", + wantMatch: true, + }, + { + name: "negative present", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "present", + }, + }, + { + name: "not present", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "notpresent", + }, + wantMatch: true, + }, + { + name: "negative notpresent", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "notpresent", + }, + value: "2006-01-02T15:04:05Z", + }, + { + name: "not a datetime constraint value", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "bar", + }, + value: "2006-01-02T15:04:05Z", + wantErr: true, + }, + { + name: "not a datetime context value", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "2006-01-02T15:04:05Z", + }, + value: "foo", + wantErr: true, + }, + { + name: "eq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "2006-01-02T15:04:05Z", + }, + value: "2006-01-02T15:04:05Z", + wantMatch: true, + }, + { + name: "eq date only", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "2006-01-02", + }, + value: "2006-01-02", + wantMatch: true, + }, + { + name: "negative eq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "2006-01-02T15:04:05Z", + }, + value: "2007-01-02T15:04:05Z", + }, + { + name: "neq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "neq", + Value: "2006-01-02T15:04:05Z", + }, + value: "2007-01-02T15:04:05Z", + wantMatch: true, + }, + { + name: "negative neq", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "neq", + Value: "2006-01-02T15:04:05Z", + }, + value: "2006-01-02T15:04:05Z", + }, + { + name: "negative neq date only", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "neq", + Value: "2006-01-02", + }, + value: "2006-01-02", + }, + { + name: "lt", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "lt", + Value: "2006-01-02T15:04:05Z", + }, + value: "2005-01-02T15:04:05Z", + wantMatch: true, + }, + { + name: "negative lt", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "lt", + Value: "2005-01-02T15:04:05Z", + }, + value: "2006-01-02T15:04:05Z", + }, + { + name: "lte", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "lte", + Value: "2006-01-02T15:04:05Z", + }, + value: "2006-01-02T15:04:05Z", + wantMatch: true, + }, + { + name: "negative lte", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "lte", + Value: "2006-01-02T15:04:05Z", + }, + value: "2007-01-02T15:04:05Z", + }, + { + name: "gt", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "gt", + Value: "2006-01-02T15:04:05Z", + }, + value: "2007-01-02T15:04:05Z", + wantMatch: true, + }, + { + name: "negative gt", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "gt", + Value: "2007-01-02T15:04:05Z", + }, + value: "2006-01-02T15:04:05Z", + }, + { + name: "gte", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "gte", + Value: "2006-01-02T15:04:05Z", + }, + value: "2006-01-02T15:04:05Z", + wantMatch: true, + }, + { + name: "negative gte", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "gte", + Value: "2006-01-02T15:04:05Z", + }, + value: "2005-01-02T15:04:05Z", + }, + { + name: "empty value", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "eq", + Value: "2006-01-02T15:04:05Z", + }, + }, + { + name: "unknown operator", + constraint: storage.EvaluationConstraint{ + Property: "foo", + Operator: "foo", + Value: "2006-01-02T15:04:05Z", + }, + value: "2006-01-02T15:04:05Z", + }, + } + + for _, tt := range tests { + var ( + constraint = tt.constraint + value = tt.value + wantMatch = tt.wantMatch + wantErr = tt.wantErr + ) + + t.Run(tt.name, func(t *testing.T) { + match, err := matchesDateTime(constraint, value) + + if wantErr { + require.Error(t, err) + var ierr errs.ErrInvalid + require.ErrorAs(t, err, &ierr) + return + } + + require.NoError(t, err) + assert.Equal(t, wantMatch, match) + }) + } +} diff --git a/internal/server/evaluator.go b/internal/server/evaluator.go index 3d614b48c2..675da41a80 100644 --- a/internal/server/evaluator.go +++ b/internal/server/evaluator.go @@ -3,19 +3,12 @@ package server import ( "context" "errors" - "hash/crc32" - "sort" - "strconv" - "strings" - "time" errs "go.flipt.io/flipt/errors" - "go.flipt.io/flipt/internal/server/metrics" fliptotel "go.flipt.io/flipt/internal/server/otel" "go.flipt.io/flipt/internal/storage" flipt "go.flipt.io/flipt/rpc/flipt" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" ) @@ -28,7 +21,7 @@ func (s *Server) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (*fli r.NamespaceKey = storage.DefaultNamespace } - resp, err := s.evaluate(ctx, r) + resp, err := s.mvEvaluator.Evaluate(ctx, r) if err != nil { return resp, err } @@ -89,7 +82,7 @@ func (s *Server) batchEvaluate(ctx context.Context, r *flipt.BatchEvaluationRequ } // TODO: we also need to validate each request, we should likely do this in the validation middleware - f, err := s.evaluate(ctx, req) + f, err := s.mvEvaluator.Evaluate(ctx, req) if err != nil { var errnf errs.ErrNotFound if r.GetExcludeNotFound() && errors.As(err, &errnf) { @@ -103,398 +96,3 @@ func (s *Server) batchEvaluate(ctx context.Context, r *flipt.BatchEvaluationRequ return &res, nil } - -func (s *Server) evaluate(ctx context.Context, r *flipt.EvaluationRequest) (resp *flipt.EvaluationResponse, err error) { - var ( - startTime = time.Now().UTC() - namespaceAttr = metrics.AttributeNamespace.String(r.NamespaceKey) - flagAttr = metrics.AttributeFlag.String(r.FlagKey) - ) - - metrics.EvaluationsTotal.Add(ctx, 1, metric.WithAttributeSet(attribute.NewSet(namespaceAttr, flagAttr))) - - defer func() { - if err == nil { - metrics.EvaluationResultsTotal.Add(ctx, 1, - metric.WithAttributeSet( - attribute.NewSet( - namespaceAttr, - flagAttr, - metrics.AttributeMatch.Bool(resp.Match), - metrics.AttributeSegment.String(resp.SegmentKey), - metrics.AttributeReason.String(resp.Reason.String()), - metrics.AttributeValue.String(resp.Value), - ), - ), - ) - } else { - metrics.EvaluationErrorsTotal.Add(ctx, 1, metric.WithAttributeSet(attribute.NewSet(namespaceAttr, flagAttr))) - } - - metrics.EvaluationLatency.Record( - ctx, - float64(time.Since(startTime).Nanoseconds())/1e6, - metric.WithAttributeSet( - attribute.NewSet( - namespaceAttr, - flagAttr, - ), - ), - ) - }() - - resp = &flipt.EvaluationResponse{ - RequestId: r.RequestId, - EntityId: r.EntityId, - RequestContext: r.Context, - FlagKey: r.FlagKey, - NamespaceKey: r.NamespaceKey, - } - - flag, err := s.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) - if err != nil { - resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON - - var errnf errs.ErrNotFound - if errors.As(err, &errnf) { - resp.Reason = flipt.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON - } - - return resp, err - } - - if !flag.Enabled { - resp.Match = false - resp.Reason = flipt.EvaluationReason_FLAG_DISABLED_EVALUATION_REASON - return resp, nil - } - - rules, err := s.store.GetEvaluationRules(ctx, r.NamespaceKey, r.FlagKey) - if err != nil { - resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON - return resp, err - } - - if len(rules) == 0 { - s.logger.Debug("no rules match") - return resp, nil - } - - var lastRank int32 - - // rule loop - for _, rule := range rules { - if rule.Rank < lastRank { - resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON - return resp, errs.ErrInvalidf("rule rank: %d detected out of order", rule.Rank) - } - - lastRank = rule.Rank - - constraintMatches := 0 - - // constraint loop - for _, c := range rule.Constraints { - v := r.Context[c.Property] - - var ( - match bool - err error - ) - - switch c.Type { - case flipt.ComparisonType_STRING_COMPARISON_TYPE: - match = matchesString(c, v) - case flipt.ComparisonType_NUMBER_COMPARISON_TYPE: - match, err = matchesNumber(c, v) - case flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE: - match, err = matchesBool(c, v) - case flipt.ComparisonType_DATETIME_COMPARISON_TYPE: - match, err = matchesDateTime(c, v) - default: - resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON - return resp, errs.ErrInvalid("unknown constraint type") - } - - if err != nil { - resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON - return resp, err - } - - if match { - s.logger.Debug("constraint matches", zap.Reflect("constraint", c)) - - // increase the matchCount - constraintMatches++ - - switch rule.SegmentMatchType { - case flipt.MatchType_ANY_MATCH_TYPE: - // can short circuit here since we had at least one match - break - default: - // keep looping as we need to match all constraints - continue - } - } else { - // no match - s.logger.Debug("constraint does not match", zap.Reflect("constraint", c)) - - switch rule.SegmentMatchType { - case flipt.MatchType_ALL_MATCH_TYPE: - // we can short circuit because we must match all constraints - break - default: - // keep looping to see if we match the next constraint - continue - } - } - } // end constraint loop - - switch rule.SegmentMatchType { - case flipt.MatchType_ALL_MATCH_TYPE: - if len(rule.Constraints) != constraintMatches { - // all constraints did not match, continue to next rule - s.logger.Debug("did not match ALL constraints") - continue - } - - case flipt.MatchType_ANY_MATCH_TYPE: - if len(rule.Constraints) > 0 && constraintMatches == 0 { - // no constraints matched, continue to next rule - s.logger.Debug("did not match ANY constraints") - continue - } - default: - s.logger.Error("unknown match type", zap.Int32("match_type", int32(rule.SegmentMatchType))) - continue - } - - // otherwise, this is our matching rule, determine the flag variant to return - // based on the distributions - resp.SegmentKey = rule.SegmentKey - - distributions, err := s.store.GetEvaluationDistributions(ctx, rule.ID) - if err != nil { - resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON - return resp, err - } - - var ( - validDistributions []*storage.EvaluationDistribution - buckets []int - ) - - for _, d := range distributions { - // don't include 0% rollouts - if d.Rollout > 0 { - validDistributions = append(validDistributions, d) - - if buckets == nil { - bucket := int(d.Rollout * percentMultiplier) - buckets = append(buckets, bucket) - } else { - bucket := buckets[len(buckets)-1] + int(d.Rollout*percentMultiplier) - buckets = append(buckets, bucket) - } - } - } - - // no distributions for rule - if len(validDistributions) == 0 { - s.logger.Info("no distributions for rule") - resp.Match = true - resp.Reason = flipt.EvaluationReason_MATCH_EVALUATION_REASON - return resp, nil - } - - var ( - bucket = crc32Num(r.EntityId, r.FlagKey) - // sort.SearchInts searches for x in a sorted slice of ints and returns the index - // as specified by Search. The return value is the index to insert x if x is - // not present (it could be len(a)). - index = sort.SearchInts(buckets, int(bucket)+1) - ) - - // if index is outside of our existing buckets then it does not match any distribution - if index == len(validDistributions) { - resp.Match = false - s.logger.Debug("did not match any distributions") - return resp, nil - } - - d := validDistributions[index] - s.logger.Debug("matched distribution", zap.Reflect("evaluation_distribution", d)) - - resp.Match = true - resp.Value = d.VariantKey - resp.Attachment = d.VariantAttachment - resp.Reason = flipt.EvaluationReason_MATCH_EVALUATION_REASON - return resp, nil - } // end rule loop - - return resp, nil -} - -func crc32Num(entityID string, salt string) uint { - return uint(crc32.ChecksumIEEE([]byte(salt+entityID))) % totalBucketNum -} - -const ( - // totalBucketNum represents how many buckets we can use to determine the consistent hashing - // distribution and rollout - totalBucketNum uint = 1000 - - // percentMultiplier implies that the multiplier between percentage (100) and totalBucketNum - percentMultiplier float32 = float32(totalBucketNum) / 100 -) - -func matchesString(c storage.EvaluationConstraint, v string) bool { - switch c.Operator { - case flipt.OpEmpty: - return len(strings.TrimSpace(v)) == 0 - case flipt.OpNotEmpty: - return len(strings.TrimSpace(v)) != 0 - } - - if v == "" { - return false - } - - value := c.Value - - switch c.Operator { - case flipt.OpEQ: - return value == v - case flipt.OpNEQ: - return value != v - case flipt.OpPrefix: - return strings.HasPrefix(strings.TrimSpace(v), value) - case flipt.OpSuffix: - return strings.HasSuffix(strings.TrimSpace(v), value) - } - - return false -} - -func matchesNumber(c storage.EvaluationConstraint, v string) (bool, error) { - switch c.Operator { - case flipt.OpNotPresent: - return len(strings.TrimSpace(v)) == 0, nil - case flipt.OpPresent: - return len(strings.TrimSpace(v)) != 0, nil - } - - // can't parse an empty string - if v == "" { - return false, nil - } - - n, err := strconv.ParseFloat(v, 64) - if err != nil { - return false, errs.ErrInvalidf("parsing number from %q", v) - } - - // TODO: we should consider parsing this at creation time since it doesn't change and it doesnt make sense to allow invalid constraint values - value, err := strconv.ParseFloat(c.Value, 64) - if err != nil { - return false, errs.ErrInvalidf("parsing number from %q", c.Value) - } - - switch c.Operator { - case flipt.OpEQ: - return value == n, nil - case flipt.OpNEQ: - return value != n, nil - case flipt.OpLT: - return n < value, nil - case flipt.OpLTE: - return n <= value, nil - case flipt.OpGT: - return n > value, nil - case flipt.OpGTE: - return n >= value, nil - } - - return false, nil -} - -func matchesBool(c storage.EvaluationConstraint, v string) (bool, error) { - switch c.Operator { - case flipt.OpNotPresent: - return len(strings.TrimSpace(v)) == 0, nil - case flipt.OpPresent: - return len(strings.TrimSpace(v)) != 0, nil - } - - // can't parse an empty string - if v == "" { - return false, nil - } - - value, err := strconv.ParseBool(v) - if err != nil { - return false, errs.ErrInvalidf("parsing boolean from %q", v) - } - - switch c.Operator { - case flipt.OpTrue: - return value, nil - case flipt.OpFalse: - return !value, nil - } - - return false, nil -} - -func matchesDateTime(c storage.EvaluationConstraint, v string) (bool, error) { - switch c.Operator { - case flipt.OpNotPresent: - return len(strings.TrimSpace(v)) == 0, nil - case flipt.OpPresent: - return len(strings.TrimSpace(v)) != 0, nil - } - - // can't parse an empty string - if v == "" { - return false, nil - } - - d, err := tryParseDateTime(v) - if err != nil { - return false, err - } - - value, err := tryParseDateTime(c.Value) - if err != nil { - return false, err - } - - switch c.Operator { - case flipt.OpEQ: - return value.Equal(d), nil - case flipt.OpNEQ: - return !value.Equal(d), nil - case flipt.OpLT: - return d.Before(value), nil - case flipt.OpLTE: - return d.Before(value) || value.Equal(d), nil - case flipt.OpGT: - return d.After(value), nil - case flipt.OpGTE: - return d.After(value) || value.Equal(d), nil - } - - return false, nil -} - -func tryParseDateTime(v string) (time.Time, error) { - if d, err := time.Parse(time.RFC3339, v); err == nil { - return d.UTC(), nil - } - - if d, err := time.Parse(time.DateOnly, v); err == nil { - return d.UTC(), nil - } - - return time.Time{}, errs.ErrInvalidf("parsing datetime from %q", v) -} diff --git a/internal/server/evaluator_test.go b/internal/server/evaluator_test.go index c787d8a381..f07ea3b4eb 100644 --- a/internal/server/evaluator_test.go +++ b/internal/server/evaluator_test.go @@ -30,10 +30,7 @@ func TestBatchEvaluate(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) disabled := &flipt.Flag{ @@ -72,10 +69,7 @@ func TestBatchEvaluate_NamespaceMismatch(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) _, err := s.BatchEvaluate(context.TODO(), &flipt.BatchEvaluationRequest{ @@ -101,10 +95,7 @@ func TestBatchEvaluate_FlagNotFoundExcluded(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) disabled := &flipt.Flag{ @@ -149,10 +140,7 @@ func TestBatchEvaluate_FlagNotFound(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) disabled := &flipt.Flag{ @@ -195,10 +183,7 @@ func TestEvaluate_FlagNotFound(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(&flipt.Flag{}, errs.ErrNotFoundf("flag %q", "foo")) @@ -221,10 +206,7 @@ func TestEvaluate_FlagDisabled(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(disabledFlag, nil) @@ -246,10 +228,7 @@ func TestEvaluate_FlagNoRules(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -273,10 +252,7 @@ func TestEvaluate_ErrorGettingRules(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -300,10 +276,7 @@ func TestEvaluate_RulesOutOfOrder(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -362,10 +335,7 @@ func TestEvaluate_ErrorGettingDistributions(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -410,10 +380,7 @@ func TestEvaluate_MatchAll_NoVariants_NoDistributions(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -499,10 +466,7 @@ func TestEvaluate_MatchAll_SingleVariantDistribution(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -629,10 +593,7 @@ func TestEvaluate_MatchAll_RolloutDistribution(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -750,10 +711,7 @@ func TestEvaluate_MatchAll_RolloutDistribution_MultiRule(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -825,10 +783,7 @@ func TestEvaluate_MatchAll_NoConstraints(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -936,10 +891,7 @@ func TestEvaluate_MatchAny_NoVariants_NoDistributions(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -1025,10 +977,7 @@ func TestEvaluate_MatchAny_SingleVariantDistribution(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -1187,10 +1136,7 @@ func TestEvaluate_MatchAny_RolloutDistribution(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -1308,10 +1254,7 @@ func TestEvaluate_MatchAny_RolloutDistribution_MultiRule(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -1383,10 +1326,7 @@ func TestEvaluate_MatchAny_NoConstraints(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -1494,10 +1434,7 @@ func TestEvaluate_FirstRolloutRuleIsZero(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -1594,10 +1531,7 @@ func TestEvaluate_MultipleZeroRolloutDistributions(t *testing.T) { var ( store = &storeMock{} logger = zaptest.NewLogger(t) - s = &Server{ - logger: logger, - store: store, - } + s = New(logger, store) ) store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) @@ -1716,729 +1650,3 @@ func TestEvaluate_MultipleZeroRolloutDistributions(t *testing.T) { }) } } - -func Test_matchesString(t *testing.T) { - tests := []struct { - name string - constraint storage.EvaluationConstraint - value string - wantMatch bool - }{ - { - name: "eq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "bar", - }, - value: "bar", - wantMatch: true, - }, - { - name: "negative eq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "bar", - }, - value: "baz", - }, - { - name: "neq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "neq", - Value: "bar", - }, - value: "baz", - wantMatch: true, - }, - { - name: "negative neq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "neq", - Value: "bar", - }, - value: "bar", - }, - { - name: "empty", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "empty", - }, - value: " ", - wantMatch: true, - }, - { - name: "negative empty", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "empty", - }, - value: "bar", - }, - { - name: "not empty", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "notempty", - }, - value: "bar", - wantMatch: true, - }, - { - name: "negative not empty", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "notempty", - }, - value: "", - }, - { - name: "unknown operator", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "foo", - Value: "bar", - }, - value: "bar", - }, - { - name: "prefix", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "prefix", - Value: "ba", - }, - value: "bar", - wantMatch: true, - }, - { - name: "negative prefix", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "prefix", - Value: "bar", - }, - value: "nope", - }, - { - name: "suffix", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "suffix", - Value: "ar", - }, - value: "bar", - wantMatch: true, - }, - { - name: "negative suffix", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "suffix", - Value: "bar", - }, - value: "nope", - }, - } - for _, tt := range tests { - var ( - constraint = tt.constraint - value = tt.value - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - match := matchesString(constraint, value) - assert.Equal(t, wantMatch, match) - }) - } -} - -func Test_matchesNumber(t *testing.T) { - tests := []struct { - name string - constraint storage.EvaluationConstraint - value string - wantMatch bool - wantErr bool - }{ - { - name: "present", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "present", - }, - value: "1", - wantMatch: true, - }, - { - name: "negative present", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "present", - }, - }, - { - name: "not present", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "notpresent", - }, - wantMatch: true, - }, - { - name: "negative notpresent", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "notpresent", - }, - value: "1", - }, - { - name: "NAN constraint value", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "bar", - }, - value: "5", - wantErr: true, - }, - { - name: "NAN context value", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "5", - }, - value: "foo", - wantErr: true, - }, - { - name: "eq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "42.0", - }, - value: "42.0", - wantMatch: true, - }, - { - name: "negative eq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "42.0", - }, - value: "50", - }, - { - name: "neq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "neq", - Value: "42.0", - }, - value: "50", - wantMatch: true, - }, - { - name: "negative neq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "neq", - Value: "42.0", - }, - value: "42.0", - }, - { - name: "lt", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "lt", - Value: "42.0", - }, - value: "8", - wantMatch: true, - }, - { - name: "negative lt", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "lt", - Value: "42.0", - }, - value: "50", - }, - { - name: "lte", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "lte", - Value: "42.0", - }, - value: "42.0", - wantMatch: true, - }, - { - name: "negative lte", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "lte", - Value: "42.0", - }, - value: "102.0", - }, - { - name: "gt", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "gt", - Value: "10.11", - }, - value: "10.12", - wantMatch: true, - }, - { - name: "negative gt", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "gt", - Value: "10.11", - }, - value: "1", - }, - { - name: "gte", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "gte", - Value: "10.11", - }, - value: "10.11", - wantMatch: true, - }, - { - name: "negative gte", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "gte", - Value: "10.11", - }, - value: "0.11", - }, - { - name: "empty value", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "0.11", - }, - }, - { - name: "unknown operator", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "foo", - Value: "0.11", - }, - value: "0.11", - }, - { - name: "negative suffix empty value", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "suffix", - Value: "bar", - }, - }, - } - - for _, tt := range tests { - var ( - constraint = tt.constraint - value = tt.value - wantMatch = tt.wantMatch - wantErr = tt.wantErr - ) - - t.Run(tt.name, func(t *testing.T) { - match, err := matchesNumber(constraint, value) - - if wantErr { - require.Error(t, err) - var ierr errs.ErrInvalid - require.ErrorAs(t, err, &ierr) - return - } - - require.NoError(t, err) - assert.Equal(t, wantMatch, match) - }) - } -} - -func Test_matchesBool(t *testing.T) { - tests := []struct { - name string - constraint storage.EvaluationConstraint - value string - wantMatch bool - wantErr bool - }{ - { - name: "present", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "present", - }, - value: "true", - wantMatch: true, - }, - { - name: "negative present", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "present", - }, - }, - { - name: "not present", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "notpresent", - }, - wantMatch: true, - }, - { - name: "negative notpresent", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "notpresent", - }, - value: "true", - }, - { - name: "not a bool", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "true", - }, - value: "foo", - wantErr: true, - }, - { - name: "is true", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "true", - }, - value: "true", - wantMatch: true, - }, - { - name: "negative is true", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "true", - }, - value: "false", - }, - { - name: "is false", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "false", - }, - value: "false", - wantMatch: true, - }, - { - name: "negative is false", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "false", - }, - value: "true", - }, - { - name: "empty value", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "false", - }, - }, - { - name: "unknown operator", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "foo", - }, - value: "true", - }, - } - - for _, tt := range tests { - var ( - constraint = tt.constraint - value = tt.value - wantMatch = tt.wantMatch - wantErr = tt.wantErr - ) - - t.Run(tt.name, func(t *testing.T) { - match, err := matchesBool(constraint, value) - - if wantErr { - require.Error(t, err) - var ierr errs.ErrInvalid - require.ErrorAs(t, err, &ierr) - return - } - - require.NoError(t, err) - assert.Equal(t, wantMatch, match) - }) - } -} - -func Test_matchesDateTime(t *testing.T) { - tests := []struct { - name string - constraint storage.EvaluationConstraint - value string - wantMatch bool - wantErr bool - }{ - { - name: "present", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "present", - }, - value: "2006-01-02T15:04:05Z", - wantMatch: true, - }, - { - name: "negative present", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "present", - }, - }, - { - name: "not present", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "notpresent", - }, - wantMatch: true, - }, - { - name: "negative notpresent", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "notpresent", - }, - value: "2006-01-02T15:04:05Z", - }, - { - name: "not a datetime constraint value", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "bar", - }, - value: "2006-01-02T15:04:05Z", - wantErr: true, - }, - { - name: "not a datetime context value", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "2006-01-02T15:04:05Z", - }, - value: "foo", - wantErr: true, - }, - { - name: "eq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "2006-01-02T15:04:05Z", - }, - value: "2006-01-02T15:04:05Z", - wantMatch: true, - }, - { - name: "eq date only", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "2006-01-02", - }, - value: "2006-01-02", - wantMatch: true, - }, - { - name: "negative eq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "2006-01-02T15:04:05Z", - }, - value: "2007-01-02T15:04:05Z", - }, - { - name: "neq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "neq", - Value: "2006-01-02T15:04:05Z", - }, - value: "2007-01-02T15:04:05Z", - wantMatch: true, - }, - { - name: "negative neq", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "neq", - Value: "2006-01-02T15:04:05Z", - }, - value: "2006-01-02T15:04:05Z", - }, - { - name: "negative neq date only", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "neq", - Value: "2006-01-02", - }, - value: "2006-01-02", - }, - { - name: "lt", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "lt", - Value: "2006-01-02T15:04:05Z", - }, - value: "2005-01-02T15:04:05Z", - wantMatch: true, - }, - { - name: "negative lt", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "lt", - Value: "2005-01-02T15:04:05Z", - }, - value: "2006-01-02T15:04:05Z", - }, - { - name: "lte", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "lte", - Value: "2006-01-02T15:04:05Z", - }, - value: "2006-01-02T15:04:05Z", - wantMatch: true, - }, - { - name: "negative lte", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "lte", - Value: "2006-01-02T15:04:05Z", - }, - value: "2007-01-02T15:04:05Z", - }, - { - name: "gt", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "gt", - Value: "2006-01-02T15:04:05Z", - }, - value: "2007-01-02T15:04:05Z", - wantMatch: true, - }, - { - name: "negative gt", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "gt", - Value: "2007-01-02T15:04:05Z", - }, - value: "2006-01-02T15:04:05Z", - }, - { - name: "gte", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "gte", - Value: "2006-01-02T15:04:05Z", - }, - value: "2006-01-02T15:04:05Z", - wantMatch: true, - }, - { - name: "negative gte", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "gte", - Value: "2006-01-02T15:04:05Z", - }, - value: "2005-01-02T15:04:05Z", - }, - { - name: "empty value", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "eq", - Value: "2006-01-02T15:04:05Z", - }, - }, - { - name: "unknown operator", - constraint: storage.EvaluationConstraint{ - Property: "foo", - Operator: "foo", - Value: "2006-01-02T15:04:05Z", - }, - value: "2006-01-02T15:04:05Z", - }, - } - - for _, tt := range tests { - var ( - constraint = tt.constraint - value = tt.value - wantMatch = tt.wantMatch - wantErr = tt.wantErr - ) - - t.Run(tt.name, func(t *testing.T) { - match, err := matchesDateTime(constraint, value) - - if wantErr { - require.Error(t, err) - var ierr errs.ErrInvalid - require.ErrorAs(t, err, &ierr) - return - } - - require.NoError(t, err) - assert.Equal(t, wantMatch, match) - }) - } -} diff --git a/internal/server/middleware/grpc/middleware.go b/internal/server/middleware/grpc/middleware.go index 2b182c1aea..6ecabec629 100644 --- a/internal/server/middleware/grpc/middleware.go +++ b/internal/server/middleware/grpc/middleware.go @@ -15,6 +15,7 @@ import ( "go.flipt.io/flipt/internal/server/metrics" flipt "go.flipt.io/flipt/rpc/flipt" fauth "go.flipt.io/flipt/rpc/flipt/auth" + "go.flipt.io/flipt/rpc/flipt/evaluation" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "google.golang.org/grpc" @@ -73,6 +74,28 @@ func ErrorUnaryInterceptor(ctx context.Context, req interface{}, _ *grpc.UnarySe // Note: this should be added before any caching interceptor to ensure the request id/response fields are unique. func EvaluationUnaryInterceptor(ctx context.Context, req interface{}, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) { switch r := req.(type) { + case *evaluation.EvaluationRequest: + startTime := time.Now() + + // set request ID if not present + if r.RequestId == "" { + r.RequestId = uuid.Must(uuid.NewV4()).String() + } + + resp, err = handler(ctx, req) + if err != nil { + return resp, err + } + + // set response fields + if resp != nil { + if rr, ok := resp.(*evaluation.VariantEvaluationResponse); ok { + rr.Timestamp = timestamp.New(time.Now().UTC()) + rr.RequestDurationMillis = float64(time.Since(startTime)) / float64(time.Millisecond) + } + return resp, nil + } + case *flipt.EvaluationRequest: startTime := time.Now() diff --git a/internal/server/server.go b/internal/server/server.go index c53f07edec..46769923ce 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1,6 +1,7 @@ package server import ( + "go.flipt.io/flipt/internal/server/evaluation" "go.flipt.io/flipt/internal/storage" flipt "go.flipt.io/flipt/rpc/flipt" "go.uber.org/zap" @@ -14,13 +15,15 @@ type Server struct { logger *zap.Logger store storage.Store flipt.UnimplementedFliptServer + mvEvaluator evaluation.MultiVariateEvaluator } // New creates a new Server func New(logger *zap.Logger, store storage.Store) *Server { return &Server{ - logger: logger, - store: store, + logger: logger, + store: store, + mvEvaluator: evaluation.NewEvaluator(logger, store), } } From e64d19018adc75d31a3999ab37a76753259beefe Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Wed, 28 Jun 2023 08:28:52 -0500 Subject: [PATCH 02/12] chore: rename Server field to evaluator --- internal/server/evaluation/evaluation.go | 2 +- internal/server/evaluation/evaluation_server.go | 12 ++++++------ internal/server/evaluator.go | 4 ++-- internal/server/server.go | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index 6a0e4c785a..7a560e7fe7 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -9,7 +9,7 @@ import ( // Variant evaluates a request for a multi-variate flag and entity. func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.VariantEvaluationResponse, error) { - resp, err := e.mvEvaluator.Evaluate(ctx, &flipt.EvaluationRequest{ + resp, err := e.evaluator.Evaluate(ctx, &flipt.EvaluationRequest{ RequestId: v.RequestId, FlagKey: v.FlagKey, EntityId: v.EntityId, diff --git a/internal/server/evaluation/evaluation_server.go b/internal/server/evaluation/evaluation_server.go index 5068f8f971..7f5de179b9 100644 --- a/internal/server/evaluation/evaluation_server.go +++ b/internal/server/evaluation/evaluation_server.go @@ -9,18 +9,18 @@ import ( // EvaluateServer serves the Flipt evaluate v2 gRPC Server. type EvaluateServer struct { - logger *zap.Logger - store storage.Store - mvEvaluator MultiVariateEvaluator + logger *zap.Logger + store storage.Store + evaluator MultiVariateEvaluator rpcEvalution.UnimplementedEvaluationServiceServer } // NewEvaluateServer is constructs a new EvaluateServer. func NewEvaluateServer(logger *zap.Logger, store storage.Store) *EvaluateServer { return &EvaluateServer{ - logger: logger, - store: store, - mvEvaluator: NewEvaluator(logger, store), + logger: logger, + store: store, + evaluator: NewEvaluator(logger, store), } } diff --git a/internal/server/evaluator.go b/internal/server/evaluator.go index 675da41a80..787d6b1286 100644 --- a/internal/server/evaluator.go +++ b/internal/server/evaluator.go @@ -21,7 +21,7 @@ func (s *Server) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (*fli r.NamespaceKey = storage.DefaultNamespace } - resp, err := s.mvEvaluator.Evaluate(ctx, r) + resp, err := s.evaluator.Evaluate(ctx, r) if err != nil { return resp, err } @@ -82,7 +82,7 @@ func (s *Server) batchEvaluate(ctx context.Context, r *flipt.BatchEvaluationRequ } // TODO: we also need to validate each request, we should likely do this in the validation middleware - f, err := s.mvEvaluator.Evaluate(ctx, req) + f, err := s.evaluator.Evaluate(ctx, req) if err != nil { var errnf errs.ErrNotFound if r.GetExcludeNotFound() && errors.As(err, &errnf) { diff --git a/internal/server/server.go b/internal/server/server.go index 46769923ce..b7b293daca 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -15,15 +15,15 @@ type Server struct { logger *zap.Logger store storage.Store flipt.UnimplementedFliptServer - mvEvaluator evaluation.MultiVariateEvaluator + evaluator evaluation.MultiVariateEvaluator } // New creates a new Server func New(logger *zap.Logger, store storage.Store) *Server { return &Server{ - logger: logger, - store: store, - mvEvaluator: evaluation.NewEvaluator(logger, store), + logger: logger, + store: store, + evaluator: evaluation.NewEvaluator(logger, store), } } From 5c7814f3f889fcb8045955840a90dd86e35ffa47 Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Wed, 28 Jun 2023 11:22:41 -0500 Subject: [PATCH 03/12] chore: move around tests, and other fixes --- internal/server/evaluation/evaluation.go | 30 + .../evaluation/evaluation_store_mock.go | 35 + internal/server/evaluation/evaluator.go | 13 +- internal/server/evaluation/evaluator_test.go | 1516 ++++++++++++++++- internal/server/evaluator_test.go | 1478 ---------------- 5 files changed, 1577 insertions(+), 1495 deletions(-) create mode 100644 internal/server/evaluation/evaluation_store_mock.go diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index 7a560e7fe7..8d6c9207cd 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -3,12 +3,22 @@ package evaluation import ( "context" + fliptotel "go.flipt.io/flipt/internal/server/otel" + "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" rpcEvaluation "go.flipt.io/flipt/rpc/flipt/evaluation" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) // Variant evaluates a request for a multi-variate flag and entity. func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.VariantEvaluationResponse, error) { + e.logger.Debug("evaluate", zap.Stringer("request", v)) + if v.NamespaceKey == "" { + v.NamespaceKey = storage.DefaultNamespace + } + resp, err := e.evaluator.Evaluate(ctx, &flipt.EvaluationRequest{ RequestId: v.RequestId, FlagKey: v.FlagKey, @@ -20,6 +30,21 @@ func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.Evaluatio return nil, err } + spanAttrs := []attribute.KeyValue{ + fliptotel.AttributeNamespace.String(v.NamespaceKey), + fliptotel.AttributeFlag.String(v.FlagKey), + fliptotel.AttributeEntityID.String(v.EntityId), + fliptotel.AttributeRequestID.String(v.RequestId), + } + + if resp != nil { + spanAttrs = append(spanAttrs, + fliptotel.AttributeMatch.Bool(resp.Match), + fliptotel.AttributeValue.String(resp.Value), + fliptotel.AttributeReason.String(resp.Reason.String()), + ) + } + ver := &rpcEvaluation.VariantEvaluationResponse{ Match: resp.Match, SegmentKey: resp.SegmentKey, @@ -28,5 +53,10 @@ func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.Evaluatio VariantAttachment: resp.Attachment, } + // add otel attributes to span + span := trace.SpanFromContext(ctx) + span.SetAttributes(spanAttrs...) + + e.logger.Debug("evaluate", zap.Stringer("response", resp)) return ver, nil } diff --git a/internal/server/evaluation/evaluation_store_mock.go b/internal/server/evaluation/evaluation_store_mock.go new file mode 100644 index 0000000000..efe11318d7 --- /dev/null +++ b/internal/server/evaluation/evaluation_store_mock.go @@ -0,0 +1,35 @@ +package evaluation + +import ( + "context" + + "github.com/stretchr/testify/mock" + "go.flipt.io/flipt/internal/storage" + flipt "go.flipt.io/flipt/rpc/flipt" +) + +var _ EvaluationStorer = &evaluationStoreMock{} + +type evaluationStoreMock struct { + mock.Mock + storage.Store +} + +func (e *evaluationStoreMock) String() string { + return "mock" +} + +func (e *evaluationStoreMock) GetFlag(ctx context.Context, namespaceKey, key string) (*flipt.Flag, error) { + args := e.Called(ctx, namespaceKey, key) + return args.Get(0).(*flipt.Flag), args.Error(1) +} + +func (e *evaluationStoreMock) GetEvaluationRules(ctx context.Context, namespaceKey string, flagKey string) ([]*storage.EvaluationRule, error) { + args := e.Called(ctx, namespaceKey, flagKey) + return args.Get(0).([]*storage.EvaluationRule), args.Error(1) +} + +func (e *evaluationStoreMock) GetEvaluationDistributions(ctx context.Context, ruleID string) ([]*storage.EvaluationDistribution, error) { + args := e.Called(ctx, ruleID) + return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1) +} diff --git a/internal/server/evaluation/evaluator.go b/internal/server/evaluation/evaluator.go index 51d177fbd7..351faf01e4 100644 --- a/internal/server/evaluation/evaluator.go +++ b/internal/server/evaluation/evaluator.go @@ -18,14 +18,21 @@ import ( "go.uber.org/zap" ) +// EvaluationStorer is the minimal abstraction for interacting with the storage layer for evaluation. +type EvaluationStorer interface { + GetFlag(ctx context.Context, namespaceKey, key string) (*flipt.Flag, error) + GetEvaluationRules(ctx context.Context, namespaceKey string, flagKey string) ([]*storage.EvaluationRule, error) + GetEvaluationDistributions(ctx context.Context, ruleID string) ([]*storage.EvaluationDistribution, error) +} + // Evaluator is an implementation of the MultiVariateEvaluator. type Evaluator struct { logger *zap.Logger - store storage.Store + store EvaluationStorer } -// NewEvalutor is the constructor for an Evaluator. -func NewEvaluator(logger *zap.Logger, store storage.Store) *Evaluator { +// NewEvaluator is the constructor for an Evaluator. +func NewEvaluator(logger *zap.Logger, store EvaluationStorer) *Evaluator { return &Evaluator{ logger: logger, store: store, diff --git a/internal/server/evaluation/evaluator_test.go b/internal/server/evaluation/evaluator_test.go index e9e49088cf..e8649434e4 100644 --- a/internal/server/evaluation/evaluator_test.go +++ b/internal/server/evaluation/evaluator_test.go @@ -1,12 +1,17 @@ package evaluation import ( + "context" + "errors" "testing" + "github.com/gofrs/uuid" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - errs "go.flipt.io/flipt/errors" + "github.com/stretchr/testify/mock" + ferrors "go.flipt.io/flipt/errors" "go.flipt.io/flipt/internal/storage" + "go.flipt.io/flipt/rpc/flipt" + "go.uber.org/zap/zaptest" ) func Test_matchesString(t *testing.T) { @@ -363,13 +368,13 @@ func Test_matchesNumber(t *testing.T) { match, err := matchesNumber(constraint, value) if wantErr { - require.Error(t, err) - var ierr errs.ErrInvalid - require.ErrorAs(t, err, &ierr) + assert.Error(t, err) + var ierr ferrors.ErrInvalid + assert.ErrorAs(t, err, &ierr) return } - require.NoError(t, err) + assert.NoError(t, err) assert.Equal(t, wantMatch, match) }) } @@ -487,13 +492,13 @@ func Test_matchesBool(t *testing.T) { match, err := matchesBool(constraint, value) if wantErr { - require.Error(t, err) - var ierr errs.ErrInvalid - require.ErrorAs(t, err, &ierr) + assert.Error(t, err) + var ierr ferrors.ErrInvalid + assert.ErrorAs(t, err, &ierr) return } - require.NoError(t, err) + assert.NoError(t, err) assert.Equal(t, wantMatch, match) }) } @@ -723,14 +728,1497 @@ func Test_matchesDateTime(t *testing.T) { match, err := matchesDateTime(constraint, value) if wantErr { - require.Error(t, err) - var ierr errs.ErrInvalid - require.ErrorAs(t, err, &ierr) + assert.Error(t, err) + var ierr ferrors.ErrInvalid + assert.ErrorAs(t, err, &ierr) return } - require.NoError(t, err) + assert.NoError(t, err) assert.Equal(t, wantMatch, match) }) } } + +var ( + enabledFlag = &flipt.Flag{ + Key: "foo", + Enabled: true, + } + disabledFlag = &flipt.Flag{ + Key: "foo", + Enabled: false, + } +) + +func TestEvaluator_FlagNotFound(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(&flipt.Flag{}, ferrors.ErrNotFoundf("flag %q", "foo")) + + resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + EntityId: "1", + FlagKey: "foo", + Context: map[string]string{ + "bar": "boz", + }, + }) + + assert.Error(t, err) + assert.EqualError(t, err, "flag \"foo\" not found") + assert.False(t, resp.Match) + assert.Equal(t, flipt.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON, resp.Reason) +} + +func TestEvaluator_FlagDisabled(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(disabledFlag, nil) + + resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + EntityId: "1", + FlagKey: "foo", + Context: map[string]string{ + "bar": "boz", + }, + }) + + assert.NoError(t, err) + assert.False(t, resp.Match) + assert.Equal(t, flipt.EvaluationReason_FLAG_DISABLED_EVALUATION_REASON, resp.Reason) +} + +func TestEvaluator_FlagNoRules(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return([]*storage.EvaluationRule{}, nil) + + resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + EntityId: "1", + FlagKey: "foo", + Context: map[string]string{ + "bar": "boz", + }, + }) + + assert.NoError(t, err) + assert.False(t, resp.Match) + assert.Equal(t, flipt.EvaluationReason_UNKNOWN_EVALUATION_REASON, resp.Reason) +} + +func TestEvaluator_ErrorGettingRules(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return([]*storage.EvaluationRule{}, errors.New("error getting rules!")) + + resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + EntityId: "1", + FlagKey: "foo", + Context: map[string]string{ + "bar": "boz", + }, + }) + + assert.Error(t, err) + assert.False(t, resp.Match) + assert.Equal(t, flipt.EvaluationReason_ERROR_EVALUATION_REASON, resp.Reason) +} + +func TestEvaluator_RulesOutOfOrder(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 1, + Constraints: []storage.EvaluationConstraint{ + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + }, + }, + { + ID: "2", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + }, + }, + }, nil) + + resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + EntityId: "1", + FlagKey: "foo", + Context: map[string]string{ + "bar": "boz", + }, + }) + + assert.Error(t, err) + assert.EqualError(t, err, "rule rank: 0 detected out of order") + assert.False(t, resp.Match) + assert.Equal(t, flipt.EvaluationReason_ERROR_EVALUATION_REASON, resp.Reason) +} + +func TestEvaluator_ErrorGettingDistributions(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return([]*storage.EvaluationDistribution{}, errors.New("error getting distributions!")) + + resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + EntityId: "1", + FlagKey: "foo", + Context: map[string]string{ + "bar": "baz", + }, + }) + + assert.Error(t, err) + assert.False(t, resp.Match) + assert.Equal(t, flipt.EvaluationReason_ERROR_EVALUATION_REASON, resp.Reason) +} + +// Match ALL constraints +func TestEvaluator_MatchAll_NoVariants_NoDistributions(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return([]*storage.EvaluationDistribution{}, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + wantMatch bool + }{ + { + name: "match string value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + }, + }, + wantMatch: true, + }, + { + name: "no match string value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "boz", + }, + }, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Empty(t, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} + +func TestEvaluator_MatchAll_SingleVariantDistribution(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + // constraint: bar (string) == baz + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + // constraint: admin (bool) == true + { + ID: "3", + Type: flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE, + Property: "admin", + Operator: flipt.OpTrue, + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 100, + VariantKey: "boz", + VariantAttachment: `{"key":"value"}`, + }, + }, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + wantMatch bool + }{ + { + name: "matches all", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + "admin": "true", + }, + }, + wantMatch: true, + }, + { + name: "no match all", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "boz", + "admin": "true", + }, + }, + }, + { + name: "no match just bool value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "admin": "true", + }, + }, + }, + { + name: "no match just string value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + }, + }, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Equal(t, "boz", resp.Value) + assert.Equal(t, `{"key":"value"}`, resp.Attachment) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} + +func TestEvaluator_MatchAll_RolloutDistribution(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + // constraint: bar (string) == baz + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 50, + VariantKey: "boz", + }, + { + ID: "6", + RuleID: "1", + VariantID: "7", + Rollout: 50, + VariantKey: "booz", + }, + }, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + matchesVariantKey string + wantMatch bool + }{ + { + name: "match string value - variant 1", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + }, + }, + matchesVariantKey: "boz", + wantMatch: true, + }, + { + name: "match string value - variant 2", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "2", + Context: map[string]string{ + "bar": "baz", + }, + }, + matchesVariantKey: "booz", + wantMatch: true, + }, + { + name: "no match string value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "boz", + }, + }, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + matchesVariantKey = tt.matchesVariantKey + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Equal(t, matchesVariantKey, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} + +func TestEvaluator_MatchAll_RolloutDistribution_MultiRule(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "subscribers", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + // constraint: premium_user (bool) == true + { + ID: "2", + Type: flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE, + Property: "premium_user", + Operator: flipt.OpTrue, + }, + }, + }, + { + ID: "2", + FlagKey: "foo", + SegmentKey: "all_users", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 1, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 50, + VariantKey: "released", + }, + { + ID: "6", + RuleID: "1", + VariantID: "7", + Rollout: 50, + VariantKey: "unreleased", + }, + }, nil) + + resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: uuid.Must(uuid.NewV4()).String(), + Context: map[string]string{ + "premium_user": "true", + }, + }) + + assert.NoError(t, err) + + assert.NotNil(t, resp) + assert.True(t, resp.Match) + assert.Equal(t, "subscribers", resp.SegmentKey) + assert.Equal(t, "foo", resp.FlagKey) + assert.NotEmpty(t, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) +} + +func TestEvaluator_MatchAll_NoConstraints(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 50, + VariantKey: "boz", + }, + { + ID: "6", + RuleID: "1", + VariantID: "7", + Rollout: 50, + VariantKey: "moz", + }, + }, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + matchesVariantKey string + wantMatch bool + }{ + { + name: "match no value - variant 1", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "10", + Context: map[string]string{}, + }, + matchesVariantKey: "boz", + wantMatch: true, + }, + { + name: "match no value - variant 2", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "01", + Context: map[string]string{}, + }, + matchesVariantKey: "moz", + wantMatch: true, + }, + { + name: "match string value - variant 2", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "01", + Context: map[string]string{ + "bar": "boz", + }, + }, + matchesVariantKey: "moz", + wantMatch: true, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + matchesVariantKey = tt.matchesVariantKey + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Equal(t, matchesVariantKey, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} + +// Match ANY constraints + +func TestEvaluator_MatchAny_NoVariants_NoDistributions(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return([]*storage.EvaluationDistribution{}, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + wantMatch bool + }{ + { + name: "match string value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + }, + }, + wantMatch: true, + }, + { + name: "no match string value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "boz", + }, + }, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Empty(t, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} + +func TestEvaluator_MatchAny_SingleVariantDistribution(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + // constraint: bar (string) == baz + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + // constraint: admin (bool) == true + { + ID: "3", + Type: flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE, + Property: "admin", + Operator: flipt.OpTrue, + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 100, + VariantKey: "boz", + }, + }, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + wantMatch bool + }{ + { + name: "matches all", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + "admin": "true", + }, + }, + wantMatch: true, + }, + { + name: "matches one", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "boz", + "admin": "true", + }, + }, + wantMatch: true, + }, + { + name: "matches just bool value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "admin": "true", + }, + }, + wantMatch: true, + }, + { + name: "matches just string value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + }, + }, + wantMatch: true, + }, + { + name: "no matches wrong bool value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "admin": "false", + }, + }, + }, + { + name: "no matches wrong string value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "boz", + }, + }, + }, + { + name: "no match none", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "boss": "boz", + "user": "true", + }, + }, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Equal(t, "boz", resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} + +func TestEvaluator_MatchAny_RolloutDistribution(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + // constraint: bar (string) == baz + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 50, + VariantKey: "boz", + }, + { + ID: "6", + RuleID: "1", + VariantID: "7", + Rollout: 50, + VariantKey: "booz", + }, + }, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + matchesVariantKey string + wantMatch bool + }{ + { + name: "match string value - variant 1", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + }, + }, + matchesVariantKey: "boz", + wantMatch: true, + }, + { + name: "match string value - variant 2", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "2", + Context: map[string]string{ + "bar": "baz", + }, + }, + matchesVariantKey: "booz", + wantMatch: true, + }, + { + name: "no match string value", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "boz", + }, + }, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + matchesVariantKey = tt.matchesVariantKey + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Equal(t, matchesVariantKey, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} + +func TestEvaluator_MatchAny_RolloutDistribution_MultiRule(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "subscribers", + SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + // constraint: premium_user (bool) == true + { + ID: "2", + Type: flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE, + Property: "premium_user", + Operator: flipt.OpTrue, + }, + }, + }, + { + ID: "2", + FlagKey: "foo", + SegmentKey: "all_users", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 1, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 50, + VariantKey: "released", + }, + { + ID: "6", + RuleID: "1", + VariantID: "7", + Rollout: 50, + VariantKey: "unreleased", + }, + }, nil) + + resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: uuid.Must(uuid.NewV4()).String(), + Context: map[string]string{ + "premium_user": "true", + }, + }) + + assert.NoError(t, err) + + assert.NotNil(t, resp) + assert.True(t, resp.Match) + assert.Equal(t, "subscribers", resp.SegmentKey) + assert.Equal(t, "foo", resp.FlagKey) + assert.NotEmpty(t, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) +} + +func TestEvaluator_MatchAny_NoConstraints(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, + Rank: 0, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 50, + VariantKey: "boz", + }, + { + ID: "6", + RuleID: "1", + VariantID: "7", + Rollout: 50, + VariantKey: "moz", + }, + }, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + matchesVariantKey string + wantMatch bool + }{ + { + name: "match no value - variant 1", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "10", + Context: map[string]string{}, + }, + matchesVariantKey: "boz", + wantMatch: true, + }, + { + name: "match no value - variant 2", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "01", + Context: map[string]string{}, + }, + matchesVariantKey: "moz", + wantMatch: true, + }, + { + name: "match string value - variant 2", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "01", + Context: map[string]string{ + "bar": "boz", + }, + }, + matchesVariantKey: "moz", + wantMatch: true, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + matchesVariantKey = tt.matchesVariantKey + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Equal(t, matchesVariantKey, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} + +// Since we skip rollout buckets that have 0% distribution, ensure that things still work +// when a 0% distribution is the first available one. +func TestEvaluator_FirstRolloutRuleIsZero(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + // constraint: bar (string) == baz + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 0, + VariantKey: "boz", + }, + { + ID: "6", + RuleID: "1", + VariantID: "7", + Rollout: 100, + VariantKey: "booz", + }, + }, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + matchesVariantKey string + wantMatch bool + }{ + { + name: "match string value - variant 1", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + }, + }, + matchesVariantKey: "booz", + wantMatch: true, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + matchesVariantKey = tt.matchesVariantKey + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Equal(t, matchesVariantKey, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} + +// Ensure things work properly when many rollout distributions have a 0% value. +func TestEvaluator_MultipleZeroRolloutDistributions(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + // constraint: bar (string) == baz + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "1", + RuleID: "1", + VariantID: "1", + VariantKey: "1", + Rollout: 0, + }, + { + ID: "2", + RuleID: "1", + VariantID: "2", + VariantKey: "2", + Rollout: 0, + }, + { + ID: "3", + RuleID: "1", + VariantID: "3", + VariantKey: "3", + Rollout: 50, + }, + { + ID: "4", + RuleID: "4", + VariantID: "4", + VariantKey: "4", + Rollout: 0, + }, + { + ID: "5", + RuleID: "1", + VariantID: "5", + VariantKey: "5", + Rollout: 0, + }, + { + ID: "6", + RuleID: "1", + VariantID: "6", + VariantKey: "6", + Rollout: 50, + }, + }, nil) + + tests := []struct { + name string + req *flipt.EvaluationRequest + matchesVariantKey string + wantMatch bool + }{ + { + name: "match string value - variant 1", + req: &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "1", + Context: map[string]string{ + "bar": "baz", + }, + }, + matchesVariantKey: "3", + wantMatch: true, + }, + } + + for _, tt := range tests { + var ( + req = tt.req + matchesVariantKey = tt.matchesVariantKey + wantMatch = tt.wantMatch + ) + + t.Run(tt.name, func(t *testing.T) { + resp, err := s.Evaluate(context.TODO(), req) + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + assert.Equal(t, req.Context, resp.RequestContext) + + if !wantMatch { + assert.False(t, resp.Match) + assert.Empty(t, resp.SegmentKey) + return + } + + assert.True(t, resp.Match) + assert.Equal(t, "bar", resp.SegmentKey) + assert.Equal(t, matchesVariantKey, resp.Value) + assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) + }) + } +} diff --git a/internal/server/evaluator_test.go b/internal/server/evaluator_test.go index f07ea3b4eb..392a1e09b5 100644 --- a/internal/server/evaluator_test.go +++ b/internal/server/evaluator_test.go @@ -2,10 +2,8 @@ package server import ( "context" - "errors" "testing" - "github.com/gofrs/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -20,10 +18,6 @@ var ( Key: "foo", Enabled: true, } - disabledFlag = &flipt.Flag{ - Key: "foo", - Enabled: false, - } ) func TestBatchEvaluate(t *testing.T) { @@ -178,1475 +172,3 @@ func TestBatchEvaluate_FlagNotFound(t *testing.T) { require.Error(t, err) assert.EqualError(t, err, "flag \"NotFoundFlag\" not found") } - -func TestEvaluate_FlagNotFound(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(&flipt.Flag{}, errs.ErrNotFoundf("flag %q", "foo")) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ - EntityId: "1", - FlagKey: "foo", - Context: map[string]string{ - "bar": "boz", - }, - }) - - require.Error(t, err) - assert.EqualError(t, err, "flag \"foo\" not found") - assert.False(t, resp.Match) - assert.Equal(t, flipt.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON, resp.Reason) -} - -func TestEvaluate_FlagDisabled(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(disabledFlag, nil) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ - EntityId: "1", - FlagKey: "foo", - Context: map[string]string{ - "bar": "boz", - }, - }) - - require.NoError(t, err) - assert.False(t, resp.Match) - assert.Equal(t, flipt.EvaluationReason_FLAG_DISABLED_EVALUATION_REASON, resp.Reason) -} - -func TestEvaluate_FlagNoRules(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return([]*storage.EvaluationRule{}, nil) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ - EntityId: "1", - FlagKey: "foo", - Context: map[string]string{ - "bar": "boz", - }, - }) - - require.NoError(t, err) - assert.False(t, resp.Match) - assert.Equal(t, flipt.EvaluationReason_UNKNOWN_EVALUATION_REASON, resp.Reason) -} - -func TestEvaluate_ErrorGettingRules(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return([]*storage.EvaluationRule{}, errors.New("error getting rules!")) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ - EntityId: "1", - FlagKey: "foo", - Context: map[string]string{ - "bar": "boz", - }, - }) - - require.Error(t, err) - assert.False(t, resp.Match) - assert.Equal(t, flipt.EvaluationReason_ERROR_EVALUATION_REASON, resp.Reason) -} - -func TestEvaluate_RulesOutOfOrder(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 1, - Constraints: []storage.EvaluationConstraint{ - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - }, - }, - { - ID: "2", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - }, - }, - }, nil) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ - EntityId: "1", - FlagKey: "foo", - Context: map[string]string{ - "bar": "boz", - }, - }) - - require.Error(t, err) - assert.EqualError(t, err, "rule rank: 0 detected out of order") - assert.False(t, resp.Match) - assert.Equal(t, flipt.EvaluationReason_ERROR_EVALUATION_REASON, resp.Reason) -} - -func TestEvaluate_ErrorGettingDistributions(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - }, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return([]*storage.EvaluationDistribution{}, errors.New("error getting distributions!")) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ - EntityId: "1", - FlagKey: "foo", - Context: map[string]string{ - "bar": "baz", - }, - }) - - require.Error(t, err) - assert.False(t, resp.Match) - assert.Equal(t, flipt.EvaluationReason_ERROR_EVALUATION_REASON, resp.Reason) -} - -// Match ALL constraints -func TestEvaluate_MatchAll_NoVariants_NoDistributions(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - }, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return([]*storage.EvaluationDistribution{}, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - wantMatch bool - }{ - { - name: "match string value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - }, - }, - wantMatch: true, - }, - { - name: "no match string value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "boz", - }, - }, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Empty(t, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} - -func TestEvaluate_MatchAll_SingleVariantDistribution(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - // constraint: bar (string) == baz - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - // constraint: admin (bool) == true - { - ID: "3", - Type: flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE, - Property: "admin", - Operator: flipt.OpTrue, - }, - }, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "4", - RuleID: "1", - VariantID: "5", - Rollout: 100, - VariantKey: "boz", - VariantAttachment: `{"key":"value"}`, - }, - }, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - wantMatch bool - }{ - { - name: "matches all", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - "admin": "true", - }, - }, - wantMatch: true, - }, - { - name: "no match all", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "boz", - "admin": "true", - }, - }, - }, - { - name: "no match just bool value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "admin": "true", - }, - }, - }, - { - name: "no match just string value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - }, - }, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Equal(t, "boz", resp.Value) - assert.Equal(t, `{"key":"value"}`, resp.Attachment) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} - -func TestEvaluate_MatchAll_RolloutDistribution(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - // constraint: bar (string) == baz - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - }, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "4", - RuleID: "1", - VariantID: "5", - Rollout: 50, - VariantKey: "boz", - }, - { - ID: "6", - RuleID: "1", - VariantID: "7", - Rollout: 50, - VariantKey: "booz", - }, - }, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - matchesVariantKey string - wantMatch bool - }{ - { - name: "match string value - variant 1", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - }, - }, - matchesVariantKey: "boz", - wantMatch: true, - }, - { - name: "match string value - variant 2", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "2", - Context: map[string]string{ - "bar": "baz", - }, - }, - matchesVariantKey: "booz", - wantMatch: true, - }, - { - name: "no match string value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "boz", - }, - }, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - matchesVariantKey = tt.matchesVariantKey - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Equal(t, matchesVariantKey, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} - -func TestEvaluate_MatchAll_RolloutDistribution_MultiRule(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "subscribers", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - // constraint: premium_user (bool) == true - { - ID: "2", - Type: flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE, - Property: "premium_user", - Operator: flipt.OpTrue, - }, - }, - }, - { - ID: "2", - FlagKey: "foo", - SegmentKey: "all_users", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 1, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "4", - RuleID: "1", - VariantID: "5", - Rollout: 50, - VariantKey: "released", - }, - { - ID: "6", - RuleID: "1", - VariantID: "7", - Rollout: 50, - VariantKey: "unreleased", - }, - }, nil) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: uuid.Must(uuid.NewV4()).String(), - Context: map[string]string{ - "premium_user": "true", - }, - }) - - require.NoError(t, err) - - assert.NotNil(t, resp) - assert.True(t, resp.Match) - assert.Equal(t, "subscribers", resp.SegmentKey) - assert.Equal(t, "foo", resp.FlagKey) - assert.NotEmpty(t, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) -} - -func TestEvaluate_MatchAll_NoConstraints(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 0, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "4", - RuleID: "1", - VariantID: "5", - Rollout: 50, - VariantKey: "boz", - }, - { - ID: "6", - RuleID: "1", - VariantID: "7", - Rollout: 50, - VariantKey: "moz", - }, - }, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - matchesVariantKey string - wantMatch bool - }{ - { - name: "match no value - variant 1", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "10", - Context: map[string]string{}, - }, - matchesVariantKey: "boz", - wantMatch: true, - }, - { - name: "match no value - variant 2", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "01", - Context: map[string]string{}, - }, - matchesVariantKey: "moz", - wantMatch: true, - }, - { - name: "match string value - variant 2", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "01", - Context: map[string]string{ - "bar": "boz", - }, - }, - matchesVariantKey: "moz", - wantMatch: true, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - matchesVariantKey = tt.matchesVariantKey - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Equal(t, matchesVariantKey, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} - -// Match ANY constraints - -func TestEvaluate_MatchAny_NoVariants_NoDistributions(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - }, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return([]*storage.EvaluationDistribution{}, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - wantMatch bool - }{ - { - name: "match string value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - }, - }, - wantMatch: true, - }, - { - name: "no match string value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "boz", - }, - }, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Empty(t, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} - -func TestEvaluate_MatchAny_SingleVariantDistribution(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - // constraint: bar (string) == baz - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - // constraint: admin (bool) == true - { - ID: "3", - Type: flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE, - Property: "admin", - Operator: flipt.OpTrue, - }, - }, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "4", - RuleID: "1", - VariantID: "5", - Rollout: 100, - VariantKey: "boz", - }, - }, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - wantMatch bool - }{ - { - name: "matches all", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - "admin": "true", - }, - }, - wantMatch: true, - }, - { - name: "matches one", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "boz", - "admin": "true", - }, - }, - wantMatch: true, - }, - { - name: "matches just bool value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "admin": "true", - }, - }, - wantMatch: true, - }, - { - name: "matches just string value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - }, - }, - wantMatch: true, - }, - { - name: "no matches wrong bool value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "admin": "false", - }, - }, - }, - { - name: "no matches wrong string value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "boz", - }, - }, - }, - { - name: "no match none", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "boss": "boz", - "user": "true", - }, - }, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Equal(t, "boz", resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} - -func TestEvaluate_MatchAny_RolloutDistribution(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - // constraint: bar (string) == baz - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - }, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "4", - RuleID: "1", - VariantID: "5", - Rollout: 50, - VariantKey: "boz", - }, - { - ID: "6", - RuleID: "1", - VariantID: "7", - Rollout: 50, - VariantKey: "booz", - }, - }, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - matchesVariantKey string - wantMatch bool - }{ - { - name: "match string value - variant 1", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - }, - }, - matchesVariantKey: "boz", - wantMatch: true, - }, - { - name: "match string value - variant 2", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "2", - Context: map[string]string{ - "bar": "baz", - }, - }, - matchesVariantKey: "booz", - wantMatch: true, - }, - { - name: "no match string value", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "boz", - }, - }, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - matchesVariantKey = tt.matchesVariantKey - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Equal(t, matchesVariantKey, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} - -func TestEvaluate_MatchAny_RolloutDistribution_MultiRule(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "subscribers", - SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - // constraint: premium_user (bool) == true - { - ID: "2", - Type: flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE, - Property: "premium_user", - Operator: flipt.OpTrue, - }, - }, - }, - { - ID: "2", - FlagKey: "foo", - SegmentKey: "all_users", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 1, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "4", - RuleID: "1", - VariantID: "5", - Rollout: 50, - VariantKey: "released", - }, - { - ID: "6", - RuleID: "1", - VariantID: "7", - Rollout: 50, - VariantKey: "unreleased", - }, - }, nil) - - resp, err := s.Evaluate(context.TODO(), &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: uuid.Must(uuid.NewV4()).String(), - Context: map[string]string{ - "premium_user": "true", - }, - }) - - require.NoError(t, err) - - assert.NotNil(t, resp) - assert.True(t, resp.Match) - assert.Equal(t, "subscribers", resp.SegmentKey) - assert.Equal(t, "foo", resp.FlagKey) - assert.NotEmpty(t, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) -} - -func TestEvaluate_MatchAny_NoConstraints(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, - Rank: 0, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "4", - RuleID: "1", - VariantID: "5", - Rollout: 50, - VariantKey: "boz", - }, - { - ID: "6", - RuleID: "1", - VariantID: "7", - Rollout: 50, - VariantKey: "moz", - }, - }, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - matchesVariantKey string - wantMatch bool - }{ - { - name: "match no value - variant 1", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "10", - Context: map[string]string{}, - }, - matchesVariantKey: "boz", - wantMatch: true, - }, - { - name: "match no value - variant 2", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "01", - Context: map[string]string{}, - }, - matchesVariantKey: "moz", - wantMatch: true, - }, - { - name: "match string value - variant 2", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "01", - Context: map[string]string{ - "bar": "boz", - }, - }, - matchesVariantKey: "moz", - wantMatch: true, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - matchesVariantKey = tt.matchesVariantKey - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Equal(t, matchesVariantKey, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} - -// Since we skip rollout buckets that have 0% distribution, ensure that things still work -// when a 0% distribution is the first available one. -func TestEvaluate_FirstRolloutRuleIsZero(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - // constraint: bar (string) == baz - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - }, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "4", - RuleID: "1", - VariantID: "5", - Rollout: 0, - VariantKey: "boz", - }, - { - ID: "6", - RuleID: "1", - VariantID: "7", - Rollout: 100, - VariantKey: "booz", - }, - }, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - matchesVariantKey string - wantMatch bool - }{ - { - name: "match string value - variant 1", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - }, - }, - matchesVariantKey: "booz", - wantMatch: true, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - matchesVariantKey = tt.matchesVariantKey - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Equal(t, matchesVariantKey, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} - -// Ensure things work properly when many rollout distributions have a 0% value. -func TestEvaluate_MultipleZeroRolloutDistributions(t *testing.T) { - var ( - store = &storeMock{} - logger = zaptest.NewLogger(t) - s = New(logger, store) - ) - - store.On("GetFlag", mock.Anything, mock.Anything, "foo").Return(enabledFlag, nil) - - store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( - []*storage.EvaluationRule{ - { - ID: "1", - FlagKey: "foo", - SegmentKey: "bar", - SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, - Rank: 0, - Constraints: []storage.EvaluationConstraint{ - // constraint: bar (string) == baz - { - ID: "2", - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "bar", - Operator: flipt.OpEQ, - Value: "baz", - }, - }, - }, - }, nil) - - store.On("GetEvaluationDistributions", mock.Anything, "1").Return( - []*storage.EvaluationDistribution{ - { - ID: "1", - RuleID: "1", - VariantID: "1", - VariantKey: "1", - Rollout: 0, - }, - { - ID: "2", - RuleID: "1", - VariantID: "2", - VariantKey: "2", - Rollout: 0, - }, - { - ID: "3", - RuleID: "1", - VariantID: "3", - VariantKey: "3", - Rollout: 50, - }, - { - ID: "4", - RuleID: "4", - VariantID: "4", - VariantKey: "4", - Rollout: 0, - }, - { - ID: "5", - RuleID: "1", - VariantID: "5", - VariantKey: "5", - Rollout: 0, - }, - { - ID: "6", - RuleID: "1", - VariantID: "6", - VariantKey: "6", - Rollout: 50, - }, - }, nil) - - tests := []struct { - name string - req *flipt.EvaluationRequest - matchesVariantKey string - wantMatch bool - }{ - { - name: "match string value - variant 1", - req: &flipt.EvaluationRequest{ - FlagKey: "foo", - EntityId: "1", - Context: map[string]string{ - "bar": "baz", - }, - }, - matchesVariantKey: "3", - wantMatch: true, - }, - } - - for _, tt := range tests { - var ( - req = tt.req - matchesVariantKey = tt.matchesVariantKey - wantMatch = tt.wantMatch - ) - - t.Run(tt.name, func(t *testing.T) { - resp, err := s.Evaluate(context.TODO(), req) - require.NoError(t, err) - assert.NotNil(t, resp) - assert.Equal(t, "foo", resp.FlagKey) - assert.Equal(t, req.Context, resp.RequestContext) - - if !wantMatch { - assert.False(t, resp.Match) - assert.Empty(t, resp.SegmentKey) - return - } - - assert.True(t, resp.Match) - assert.Equal(t, "bar", resp.SegmentKey) - assert.Equal(t, matchesVariantKey, resp.Value) - assert.Equal(t, flipt.EvaluationReason_MATCH_EVALUATION_REASON, resp.Reason) - }) - } -} From 9341a8bd2bac3d5708cf2c3ab3fa51d55206903d Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Wed, 28 Jun 2023 11:49:51 -0500 Subject: [PATCH 04/12] chore: DRY up middleware and add extra field to otel metrics --- internal/server/evaluation/evaluation.go | 1 + .../evaluation/evaluation_store_mock.go | 1 - internal/server/middleware/grpc/middleware.go | 37 ++++++------------- 3 files changed, 13 insertions(+), 26 deletions(-) diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index 8d6c9207cd..eb571e3f28 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -42,6 +42,7 @@ func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.Evaluatio fliptotel.AttributeMatch.Bool(resp.Match), fliptotel.AttributeValue.String(resp.Value), fliptotel.AttributeReason.String(resp.Reason.String()), + fliptotel.AttributeSegment.String(resp.SegmentKey), ) } diff --git a/internal/server/evaluation/evaluation_store_mock.go b/internal/server/evaluation/evaluation_store_mock.go index efe11318d7..cd8f5fa626 100644 --- a/internal/server/evaluation/evaluation_store_mock.go +++ b/internal/server/evaluation/evaluation_store_mock.go @@ -12,7 +12,6 @@ var _ EvaluationStorer = &evaluationStoreMock{} type evaluationStoreMock struct { mock.Mock - storage.Store } func (e *evaluationStoreMock) String() string { diff --git a/internal/server/middleware/grpc/middleware.go b/internal/server/middleware/grpc/middleware.go index 6ecabec629..d223a52df4 100644 --- a/internal/server/middleware/grpc/middleware.go +++ b/internal/server/middleware/grpc/middleware.go @@ -74,34 +74,18 @@ func ErrorUnaryInterceptor(ctx context.Context, req interface{}, _ *grpc.UnarySe // Note: this should be added before any caching interceptor to ensure the request id/response fields are unique. func EvaluationUnaryInterceptor(ctx context.Context, req interface{}, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (resp interface{}, err error) { switch r := req.(type) { - case *evaluation.EvaluationRequest: + case *flipt.EvaluationRequest, *evaluation.EvaluationRequest: startTime := time.Now() // set request ID if not present - if r.RequestId == "" { - r.RequestId = uuid.Must(uuid.NewV4()).String() - } - - resp, err = handler(ctx, req) - if err != nil { - return resp, err - } - - // set response fields - if resp != nil { - if rr, ok := resp.(*evaluation.VariantEvaluationResponse); ok { - rr.Timestamp = timestamp.New(time.Now().UTC()) - rr.RequestDurationMillis = float64(time.Since(startTime)) / float64(time.Millisecond) + if re, ok := r.(*flipt.EvaluationRequest); ok { + if re.RequestId == "" { + re.RequestId = uuid.Must(uuid.NewV4()).String() + } + } else if re, ok := r.(*evaluation.EvaluationRequest); ok { + if re.RequestId == "" { + re.RequestId = uuid.Must(uuid.NewV4()).String() } - return resp, nil - } - - case *flipt.EvaluationRequest: - startTime := time.Now() - - // set request ID if not present - if r.RequestId == "" { - r.RequestId = uuid.Must(uuid.NewV4()).String() } resp, err = handler(ctx, req) @@ -112,10 +96,13 @@ func EvaluationUnaryInterceptor(ctx context.Context, req interface{}, _ *grpc.Un // set response fields if resp != nil { if rr, ok := resp.(*flipt.EvaluationResponse); ok { - rr.RequestId = r.RequestId + rr.Timestamp = timestamp.New(time.Now().UTC()) + rr.RequestDurationMillis = float64(time.Since(startTime)) / float64(time.Millisecond) + } else if rr, ok := resp.(*evaluation.VariantEvaluationResponse); ok { rr.Timestamp = timestamp.New(time.Now().UTC()) rr.RequestDurationMillis = float64(time.Since(startTime)) / float64(time.Millisecond) } + return resp, nil } From 68c0dd307f556e2a52bc0e6a1c1b14d1d6aa112b Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Wed, 28 Jun 2023 18:37:50 -0500 Subject: [PATCH 05/12] feat: add boolean evaluation logic for the store --- internal/server/evaluation/evaluation.go | 56 +++++++ .../evaluation/evaluation_store_mock.go | 5 + internal/server/evaluation/evaluator.go | 158 ++++++++++-------- .../server/middleware/grpc/support_test.go | 5 + internal/server/support_test.go | 5 + internal/storage/fs/snapshot.go | 4 + internal/storage/sql/common/evaluation.go | 107 ++++++++++++ internal/storage/storage.go | 24 +++ 8 files changed, 290 insertions(+), 74 deletions(-) diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index eb571e3f28..2e461fc0c6 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -2,6 +2,7 @@ package evaluation import ( "context" + "hash/crc32" fliptotel "go.flipt.io/flipt/internal/server/otel" "go.flipt.io/flipt/internal/storage" @@ -61,3 +62,58 @@ func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.Evaluatio e.logger.Debug("evaluate", zap.Stringer("response", resp)) return ver, nil } + +// Boolean evaluates a request for a boolean flag and entity. +func (e *EvaluateServer) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.BooleanEvaluationResponse, error) { + rollouts, err := e.store.GetEvaluationRollouts(ctx, r.FlagKey, r.NamespaceKey) + if err != nil { + return nil, err + } + + resp := &rpcEvaluation.BooleanEvaluationResponse{} + + for _, rollout := range rollouts { + if rollout.Percentage != nil { + // consistent hashing based on the entity id and flag key. + hash := crc32.ChecksumIEEE([]byte(r.EntityId + r.FlagKey)) + + normalizedValue := float32(int(hash) % 100) + + // if this case does not hold, fall through to the next rollout. + if normalizedValue < rollout.Percentage.Percentage { + resp.Value = rollout.Percentage.Value + resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON + + return resp, nil + } + } else if rollout.Segment != nil { + matched, err := doConstraintsMatch(e.logger, r.Context, rollout.Segment.Constraints, rollout.Segment.SegmentMatchType) + if err != nil { + resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON + return resp, err + } + + // if we don't match the segment, fall through to the next rollout. + if !matched { + continue + } + + resp.Value = rollout.Segment.Value + resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON + + return resp, nil + } + } + + f, err := e.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) + if err != nil { + resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON + return resp, err + } + + // If we have exhausted all rollouts and we still don't have a match, return the default value. + resp.Reason = rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON + resp.Value = f.Enabled + + return resp, nil +} diff --git a/internal/server/evaluation/evaluation_store_mock.go b/internal/server/evaluation/evaluation_store_mock.go index cd8f5fa626..0e29301920 100644 --- a/internal/server/evaluation/evaluation_store_mock.go +++ b/internal/server/evaluation/evaluation_store_mock.go @@ -32,3 +32,8 @@ func (e *evaluationStoreMock) GetEvaluationDistributions(ctx context.Context, ru args := e.Called(ctx, ruleID) return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1) } + +func (e *evaluationStoreMock) GetEvaluationRollouts(ctx context.Context, flagKey string, namespaceKey string) ([]*storage.EvaluationRollout, error) { + args := e.Called(ctx, flagKey, namespaceKey) + return args.Get(0).([]*storage.EvaluationRollout), args.Error(1) +} diff --git a/internal/server/evaluation/evaluator.go b/internal/server/evaluation/evaluator.go index 351faf01e4..638f336b73 100644 --- a/internal/server/evaluation/evaluator.go +++ b/internal/server/evaluation/evaluator.go @@ -23,6 +23,7 @@ type EvaluationStorer interface { GetFlag(ctx context.Context, namespaceKey, key string) (*flipt.Flag, error) GetEvaluationRules(ctx context.Context, namespaceKey string, flagKey string) ([]*storage.EvaluationRule, error) GetEvaluationDistributions(ctx context.Context, ruleID string) ([]*storage.EvaluationDistribution, error) + GetEvaluationRollouts(ctx context.Context, flagKey string, namespaceKey string) ([]*storage.EvaluationRollout, error) } // Evaluator is an implementation of the MultiVariateEvaluator. @@ -131,81 +132,13 @@ func (e *Evaluator) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (r lastRank = rule.Rank - constraintMatches := 0 - - // constraint loop - for _, c := range rule.Constraints { - v := r.Context[c.Property] - - var ( - match bool - err error - ) - - switch c.Type { - case flipt.ComparisonType_STRING_COMPARISON_TYPE: - match = matchesString(c, v) - case flipt.ComparisonType_NUMBER_COMPARISON_TYPE: - match, err = matchesNumber(c, v) - case flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE: - match, err = matchesBool(c, v) - case flipt.ComparisonType_DATETIME_COMPARISON_TYPE: - match, err = matchesDateTime(c, v) - default: - resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON - return resp, errs.ErrInvalid("unknown constraint type") - } - - if err != nil { - resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON - return resp, err - } - - if match { - e.logger.Debug("constraint matches", zap.Reflect("constraint", c)) - - // increase the matchCount - constraintMatches++ - - switch rule.SegmentMatchType { - case flipt.MatchType_ANY_MATCH_TYPE: - // can short circuit here since we had at least one match - break - default: - // keep looping as we need to match all constraints - continue - } - } else { - // no match - e.logger.Debug("constraint does not match", zap.Reflect("constraint", c)) - - switch rule.SegmentMatchType { - case flipt.MatchType_ALL_MATCH_TYPE: - // we can short circuit because we must match all constraints - break - default: - // keep looping to see if we match the next constraint - continue - } - } - } // end constraint loop - - switch rule.SegmentMatchType { - case flipt.MatchType_ALL_MATCH_TYPE: - if len(rule.Constraints) != constraintMatches { - // all constraints did not match, continue to next rule - e.logger.Debug("did not match ALL constraints") - continue - } + matched, err := doConstraintsMatch(e.logger, r.Context, rule.Constraints, rule.SegmentMatchType) + if err != nil { + resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON + return resp, err + } - case flipt.MatchType_ANY_MATCH_TYPE: - if len(rule.Constraints) > 0 && constraintMatches == 0 { - // no constraints matched, continue to next rule - e.logger.Debug("did not match ANY constraints") - continue - } - default: - e.logger.Error("unknown match type", zap.Int32("match_type", int32(rule.SegmentMatchType))) + if !matched { continue } @@ -275,6 +208,83 @@ func (e *Evaluator) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (r return resp, nil } +// doConstraintsMatch is a utility function that will return if all or any constraints have matched for a segment depending +// on the match type. +func doConstraintsMatch(logger *zap.Logger, evalCtx map[string]string, constraints []storage.EvaluationConstraint, segmentMatchType flipt.MatchType) (bool, error) { + constraintMatches := 0 + + for _, c := range constraints { + v := evalCtx[c.Property] + + var ( + match bool + err error + ) + + switch c.Type { + case flipt.ComparisonType_STRING_COMPARISON_TYPE: + match = matchesString(c, v) + case flipt.ComparisonType_NUMBER_COMPARISON_TYPE: + match, err = matchesNumber(c, v) + case flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE: + match, err = matchesBool(c, v) + case flipt.ComparisonType_DATETIME_COMPARISON_TYPE: + match, err = matchesDateTime(c, v) + default: + return false, errs.ErrInvalid("unknown constraint type") + } + + if err != nil { + return false, err + } + + if match { + // increase the matchCount + constraintMatches++ + + switch segmentMatchType { + case flipt.MatchType_ANY_MATCH_TYPE: + // can short circuit here since we had at least one match + break + default: + // keep looping as we need to match all constraints + continue + } + } else { + // no match + switch segmentMatchType { + case flipt.MatchType_ALL_MATCH_TYPE: + // we can short circuit because we must match all constraints + break + default: + // keep looping to see if we match the next constraint + continue + } + } + } + + var matched bool = true + + switch segmentMatchType { + case flipt.MatchType_ALL_MATCH_TYPE: + if len(constraints) == constraintMatches { + logger.Debug("did not match ALL constraints") + matched = false + } + + case flipt.MatchType_ANY_MATCH_TYPE: + if len(constraints) > 0 && constraintMatches == 0 { + logger.Debug("did not match ANY constraints") + matched = false + } + default: + logger.Error("unknown match type", zap.Int32("match_type", int32(segmentMatchType))) + matched = false + } + + return matched, nil +} + func crc32Num(entityID string, salt string) uint { return uint(crc32.ChecksumIEEE([]byte(salt+entityID))) % totalBucketNum } diff --git a/internal/server/middleware/grpc/support_test.go b/internal/server/middleware/grpc/support_test.go index b427b03dec..8e6202bb04 100644 --- a/internal/server/middleware/grpc/support_test.go +++ b/internal/server/middleware/grpc/support_test.go @@ -240,6 +240,11 @@ func (m *storeMock) GetEvaluationDistributions(ctx context.Context, ruleID strin return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1) } +func (m *storeMock) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey string) ([]*storage.EvaluationRollout, error) { + args := m.Called(ctx, flagKey, namespaceKey) + return args.Get(0).([]*storage.EvaluationRollout), args.Error(1) +} + var _ storageauth.Store = &authStoreMock{} type authStoreMock struct { diff --git a/internal/server/support_test.go b/internal/server/support_test.go index 5cc11981ad..cea02250b8 100644 --- a/internal/server/support_test.go +++ b/internal/server/support_test.go @@ -232,3 +232,8 @@ func (m *storeMock) GetEvaluationDistributions(ctx context.Context, ruleID strin args := m.Called(ctx, ruleID) return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1) } + +func (m *storeMock) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey string) ([]*storage.EvaluationRollout, error) { + args := m.Called(ctx, flagKey, namespaceKey) + return args.Get(0).([]*storage.EvaluationRollout), args.Error(1) +} diff --git a/internal/storage/fs/snapshot.go b/internal/storage/fs/snapshot.go index 2c9c6eace0..55a288609a 100644 --- a/internal/storage/fs/snapshot.go +++ b/internal/storage/fs/snapshot.go @@ -625,6 +625,10 @@ func (ss *storeSnapshot) GetEvaluationDistributions(ctx context.Context, ruleID return dists, nil } +func (ss *storeSnapshot) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey string) ([]*storage.EvaluationRollout, error) { + panic("not implemented") +} + func (ss *storeSnapshot) GetRollout(ctx context.Context, namespaceKey, id string) (*flipt.Rollout, error) { panic("not implemented") } diff --git a/internal/storage/sql/common/evaluation.go b/internal/storage/sql/common/evaluation.go index 91877f8794..60c131100d 100644 --- a/internal/storage/sql/common/evaluation.go +++ b/internal/storage/sql/common/evaluation.go @@ -160,3 +160,110 @@ func (s *Store) GetEvaluationDistributions(ctx context.Context, ruleID string) ( return distributions, nil } + +func (s *Store) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey string) ([]*storage.EvaluationRollout, error) { + rows, err := s.db.QueryContext(ctx, ` + SELECT r.id, r.namespace_key, r."type", r."rank", rp.percentage, rp.value, rss.segment_key, rss.match_type, rss.constraint_type, rss.constraint_property, rss.constraint_operator, rss.constraint_value + FROM rollouts r + LEFT JOIN rollout_percentages rp ON (r.id = rp.rollout_id) + LEFT JOIN ( + SELECT rs.rollout_id, rs.segment_key, s.match_type, rs.value AS rollout_segment_value, c."type" AS constraint_type, c.property AS constraint_property, c.operator AS constraint_operator, c.value AS constraint_value + FROM rollout_segments rs + JOIN segments s ON (rs.segment_key = s."key") + JOIN constraints c ON (rs.segment_key = c.segment_key) + ) rss ON (r.id = rss.rollout_id) + WHERE r.flag_key = $1 AND r.namespace_key = $2 + ORDER BY r."rank" ASC + `, flagKey, namespaceKey) + if err != nil { + return nil, err + } + + defer func() { + if cerr := rows.Close(); cerr != nil && err == nil { + err = cerr + } + }() + + var ( + uniqueSegmentedRollouts = make(map[string]*storage.EvaluationRollout) + rollouts = []*storage.EvaluationRollout{} + ) + + for rows.Next() { + var ( + rolloutId string + evaluationRollout storage.EvaluationRollout + rpPercentageNumber sql.NullFloat64 + rpPercentageValue sql.NullBool + rsSegmentKey sql.NullString + rsMatchType sql.NullInt32 + rsConstraintType sql.NullInt32 + rsConstraintProperty sql.NullString + rsConstraintOperator sql.NullString + rsConstraintValue sql.NullString + ) + + if err := rows.Scan( + &rolloutId, + &evaluationRollout.NamespaceKey, + &evaluationRollout.RolloutType, + &evaluationRollout.Rank, + &rpPercentageNumber, + &rpPercentageValue, + &rsSegmentKey, + &rsMatchType, + &rsConstraintType, + &rsConstraintProperty, + &rsConstraintOperator, + &rsConstraintValue, + ); err != nil { + return rollouts, err + } + + if rpPercentageNumber.Valid && rpPercentageValue.Valid { + storagePercentage := &storage.RolloutPercentage{ + Percentage: float32(rpPercentageNumber.Float64), + Value: rpPercentageValue.Bool, + } + + evaluationRollout.Percentage = storagePercentage + } + + if rsSegmentKey.Valid && + rsMatchType.Valid && + rsConstraintType.Valid && + rsConstraintProperty.Valid && + rsConstraintOperator.Valid && rsConstraintValue.Valid { + c := storage.EvaluationConstraint{ + Type: flipt.ComparisonType(rsConstraintType.Int32), + Property: rsConstraintProperty.String, + Operator: rsConstraintOperator.String, + Value: rsConstraintValue.String, + } + + if existingSegment, ok := uniqueSegmentedRollouts[rolloutId]; ok { + existingSegment.Segment.Constraints = append(existingSegment.Segment.Constraints, c) + continue + } + + storageSegment := &storage.RolloutSegment{ + SegmentKey: rsSegmentKey.String, + SegmentMatchType: flipt.MatchType(rsMatchType.Int32), + } + + storageSegment.Constraints = append(storageSegment.Constraints, c) + + evaluationRollout.Segment = storageSegment + uniqueSegmentedRollouts[rolloutId] = &evaluationRollout + } + + rollouts = append(rollouts, &evaluationRollout) + } + + if err := rows.Err(); err != nil { + return rollouts, err + } + + return rollouts, nil +} diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 9f9b6b8574..6b36977920 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -27,6 +27,29 @@ type EvaluationRule struct { Constraints []EvaluationConstraint } +// EvaluationRollout represents a rollout in the form that helps with evaluation. +type EvaluationRollout struct { + NamespaceKey string + RolloutType flipt.RolloutType + Rank int32 + Percentage *RolloutPercentage + Segment *RolloutSegment +} + +// RolloutPercentage represents Percentage(s) for use in evaluation. +type RolloutPercentage struct { + Percentage float32 + Value bool +} + +// RolloutSegment represents Segment(s) for use in evaluation. +type RolloutSegment struct { + SegmentKey string + SegmentMatchType flipt.MatchType + Value bool + Constraints []EvaluationConstraint +} + // EvaluationConstraint represents a segment constraint that is used for evaluation type EvaluationConstraint struct { ID string @@ -147,6 +170,7 @@ type EvaluationStore interface { // Note: Rules MUST be returned in order by Rank GetEvaluationRules(ctx context.Context, namespaceKey, flagKey string) ([]*EvaluationRule, error) GetEvaluationDistributions(ctx context.Context, ruleID string) ([]*EvaluationDistribution, error) + GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey string) ([]*EvaluationRollout, error) } // NamespaceStore stores and retrieves namespaces From f95e38e32151e84ee43d31b674711d3e9bc033da Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Thu, 29 Jun 2023 11:52:53 -0500 Subject: [PATCH 06/12] chore: add tests for boolean evaluate and address PR comments from upstream --- internal/cmd/grpc.go | 2 +- internal/server/evaluation/evaluation.go | 30 +- .../server/evaluation/evaluation_server.go | 30 -- .../evaluation/evaluation_store_mock.go | 2 +- internal/server/evaluation/evaluation_test.go | 290 ++++++++++++++++++ internal/server/evaluation/evaluator.go | 16 +- internal/server/evaluation/server.go | 41 +++ internal/server/middleware/grpc/middleware.go | 9 +- internal/storage/sql/common/evaluation.go | 6 +- 9 files changed, 371 insertions(+), 55 deletions(-) delete mode 100644 internal/server/evaluation/evaluation_server.go create mode 100644 internal/server/evaluation/evaluation_test.go create mode 100644 internal/server/evaluation/server.go diff --git a/internal/cmd/grpc.go b/internal/cmd/grpc.go index 460f5897c0..739f0778c6 100644 --- a/internal/cmd/grpc.go +++ b/internal/cmd/grpc.go @@ -344,7 +344,7 @@ func NewGRPCServer( register.Add(fliptserver.New(logger, store)) register.Add(metadata.NewServer(cfg, info)) - register.Add(evaluation.NewEvaluateServer(logger, store)) + register.Add(evaluation.New(logger, store)) // initialize grpc server server.Server = grpc.NewServer(grpcOpts...) diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index 2e461fc0c6..7715a34279 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -4,6 +4,7 @@ import ( "context" "hash/crc32" + "go.flipt.io/flipt/errors" fliptotel "go.flipt.io/flipt/internal/server/otel" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" @@ -14,13 +15,13 @@ import ( ) // Variant evaluates a request for a multi-variate flag and entity. -func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.VariantEvaluationResponse, error) { - e.logger.Debug("evaluate", zap.Stringer("request", v)) +func (s *Server) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.VariantEvaluationResponse, error) { + s.logger.Debug("evaluate", zap.Stringer("request", v)) if v.NamespaceKey == "" { v.NamespaceKey = storage.DefaultNamespace } - resp, err := e.evaluator.Evaluate(ctx, &flipt.EvaluationRequest{ + resp, err := s.evaluator.Evaluate(ctx, &flipt.EvaluationRequest{ RequestId: v.RequestId, FlagKey: v.FlagKey, EntityId: v.EntityId, @@ -59,20 +60,29 @@ func (e *EvaluateServer) Variant(ctx context.Context, v *rpcEvaluation.Evaluatio span := trace.SpanFromContext(ctx) span.SetAttributes(spanAttrs...) - e.logger.Debug("evaluate", zap.Stringer("response", resp)) + s.logger.Debug("evaluate", zap.Stringer("response", resp)) return ver, nil } // Boolean evaluates a request for a boolean flag and entity. -func (e *EvaluateServer) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.BooleanEvaluationResponse, error) { - rollouts, err := e.store.GetEvaluationRollouts(ctx, r.FlagKey, r.NamespaceKey) +func (s *Server) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.BooleanEvaluationResponse, error) { + rollouts, err := s.store.GetEvaluationRollouts(ctx, r.FlagKey, r.NamespaceKey) if err != nil { return nil, err } resp := &rpcEvaluation.BooleanEvaluationResponse{} + var lastRank int32 + for _, rollout := range rollouts { + if rollout.Rank < lastRank { + resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON + return resp, errors.ErrInvalidf("rollout rank: %d detected out of order", rollout.Rank) + } + + lastRank = rollout.Rank + if rollout.Percentage != nil { // consistent hashing based on the entity id and flag key. hash := crc32.ChecksumIEEE([]byte(r.EntityId + r.FlagKey)) @@ -83,11 +93,12 @@ func (e *EvaluateServer) Boolean(ctx context.Context, r *rpcEvaluation.Evaluatio if normalizedValue < rollout.Percentage.Percentage { resp.Value = rollout.Percentage.Value resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON + s.logger.Debug("percentage based matched", zap.Int("rank", int(rollout.Rank)), zap.String("rollout_type", "percentage")) return resp, nil } } else if rollout.Segment != nil { - matched, err := doConstraintsMatch(e.logger, r.Context, rollout.Segment.Constraints, rollout.Segment.SegmentMatchType) + matched, err := doConstraintsMatch(s.logger, r.Context, rollout.Segment.Constraints, rollout.Segment.SegmentMatchType) if err != nil { resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON return resp, err @@ -101,11 +112,13 @@ func (e *EvaluateServer) Boolean(ctx context.Context, r *rpcEvaluation.Evaluatio resp.Value = rollout.Segment.Value resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON + s.logger.Debug("segment based matched", zap.Int("rank", int(rollout.Rank)), zap.String("segment", rollout.Segment.SegmentKey)) + return resp, nil } } - f, err := e.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) + f, err := s.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) if err != nil { resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON return resp, err @@ -114,6 +127,7 @@ func (e *EvaluateServer) Boolean(ctx context.Context, r *rpcEvaluation.Evaluatio // If we have exhausted all rollouts and we still don't have a match, return the default value. resp.Reason = rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON resp.Value = f.Enabled + s.logger.Debug("default rollout matched", zap.Bool("value", f.Enabled)) return resp, nil } diff --git a/internal/server/evaluation/evaluation_server.go b/internal/server/evaluation/evaluation_server.go deleted file mode 100644 index 7f5de179b9..0000000000 --- a/internal/server/evaluation/evaluation_server.go +++ /dev/null @@ -1,30 +0,0 @@ -package evaluation - -import ( - "go.flipt.io/flipt/internal/storage" - rpcEvalution "go.flipt.io/flipt/rpc/flipt/evaluation" - "go.uber.org/zap" - "google.golang.org/grpc" -) - -// EvaluateServer serves the Flipt evaluate v2 gRPC Server. -type EvaluateServer struct { - logger *zap.Logger - store storage.Store - evaluator MultiVariateEvaluator - rpcEvalution.UnimplementedEvaluationServiceServer -} - -// NewEvaluateServer is constructs a new EvaluateServer. -func NewEvaluateServer(logger *zap.Logger, store storage.Store) *EvaluateServer { - return &EvaluateServer{ - logger: logger, - store: store, - evaluator: NewEvaluator(logger, store), - } -} - -// RegisterGRPC registers the EvaluateServer onto the provided gRPC Server. -func (e *EvaluateServer) RegisterGRPC(server *grpc.Server) { - rpcEvalution.RegisterEvaluationServiceServer(server, e) -} diff --git a/internal/server/evaluation/evaluation_store_mock.go b/internal/server/evaluation/evaluation_store_mock.go index 0e29301920..a71bf7f6e7 100644 --- a/internal/server/evaluation/evaluation_store_mock.go +++ b/internal/server/evaluation/evaluation_store_mock.go @@ -8,7 +8,7 @@ import ( flipt "go.flipt.io/flipt/rpc/flipt" ) -var _ EvaluationStorer = &evaluationStoreMock{} +var _ Storer = &evaluationStoreMock{} type evaluationStoreMock struct { mock.Mock diff --git a/internal/server/evaluation/evaluation_test.go b/internal/server/evaluation/evaluation_test.go new file mode 100644 index 0000000000..265c6bce7b --- /dev/null +++ b/internal/server/evaluation/evaluation_test.go @@ -0,0 +1,290 @@ +package evaluation + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "go.flipt.io/flipt/internal/storage" + "go.flipt.io/flipt/rpc/flipt" + rpcEvaluation "go.flipt.io/flipt/rpc/flipt/evaluation" + "go.uber.org/zap/zaptest" +) + +func TestBoolean_DefaultRule_NoRollouts(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) + + store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{}, nil) + + store.On("GetFlag", mock.Anything, mock.Anything, mock.Anything).Return(&flipt.Flag{ + NamespaceKey: "test-namespace", + Key: "test-flag", + Enabled: true, + }, nil) + + res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.NoError(t, err) + + assert.Equal(t, true, res.Value) + assert.Equal(t, rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON, res.Reason) +} + +func TestBoolean_DefaultRuleFallthrough_WithPercentageRollout(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) + + store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + { + NamespaceKey: namespaceKey, + Rank: 1, + RolloutType: flipt.RolloutType_PERCENTAGE_ROLLOUT_TYPE, + Percentage: &storage.RolloutPercentage{ + Percentage: 5, + Value: false, + }, + }, + }, nil) + + store.On("GetFlag", mock.Anything, mock.Anything, mock.Anything).Return(&flipt.Flag{ + NamespaceKey: "test-namespace", + Key: "test-flag", + Enabled: true, + }, nil) + + res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.NoError(t, err) + + assert.Equal(t, true, res.Value) + assert.Equal(t, rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON, res.Reason) +} + +func TestBoolean_PercentageRuleMatch(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) + + defer store.AssertNotCalled(t, "GetFlag", flagKey, namespaceKey) + + store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + { + NamespaceKey: namespaceKey, + Rank: 1, + RolloutType: flipt.RolloutType_PERCENTAGE_ROLLOUT_TYPE, + Percentage: &storage.RolloutPercentage{ + Percentage: 70, + Value: false, + }, + }, + }, nil) + + res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.NoError(t, err) + + assert.Equal(t, false, res.Value) + assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, res.Reason) +} + +func TestBoolean_PercentageRuleFallthrough_SegmentMatch(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) + + defer store.AssertNotCalled(t, "GetFlag", flagKey, namespaceKey) + + store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + { + NamespaceKey: namespaceKey, + Rank: 1, + RolloutType: flipt.RolloutType_PERCENTAGE_ROLLOUT_TYPE, + Percentage: &storage.RolloutPercentage{ + Percentage: 5, + Value: false, + }, + }, + { + NamespaceKey: namespaceKey, + RolloutType: flipt.RolloutType_SEGMENT_ROLLOUT_TYPE, + Rank: 2, + Segment: &storage.RolloutSegment{ + SegmentKey: "test-segment", + SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, + Value: true, + Constraints: []storage.EvaluationConstraint{ + { + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "hello", + Operator: flipt.OpEQ, + Value: "world", + }, + }, + }, + }, + }, nil) + + res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.NoError(t, err) + + assert.Equal(t, true, res.Value) + assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, res.Reason) +} + +func TestBoolean_SegmentMatch_MultipleConstraints(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) + + defer store.AssertNotCalled(t, "GetFlag", flagKey, namespaceKey) + + store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + { + NamespaceKey: namespaceKey, + RolloutType: flipt.RolloutType_SEGMENT_ROLLOUT_TYPE, + Rank: 1, + Segment: &storage.RolloutSegment{ + SegmentKey: "test-segment", + SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, + Value: true, + Constraints: []storage.EvaluationConstraint{ + { + Type: flipt.ComparisonType_NUMBER_COMPARISON_TYPE, + Property: "pitimes100", + Operator: flipt.OpEQ, + Value: "314", + }, + { + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "hello", + Operator: flipt.OpEQ, + Value: "world", + }, + }, + }, + }, + }, nil) + + res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.NoError(t, err) + + assert.Equal(t, true, res.Value) + assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, res.Reason) +} + +func TestBoolean_RulesOutOfOrder(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) + + defer store.AssertNotCalled(t, "GetFlag", flagKey, namespaceKey) + + store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + { + NamespaceKey: namespaceKey, + Rank: 1, + RolloutType: flipt.RolloutType_PERCENTAGE_ROLLOUT_TYPE, + Percentage: &storage.RolloutPercentage{ + Percentage: 5, + Value: false, + }, + }, + { + NamespaceKey: namespaceKey, + RolloutType: flipt.RolloutType_SEGMENT_ROLLOUT_TYPE, + Rank: 0, + Segment: &storage.RolloutSegment{ + SegmentKey: "test-segment", + SegmentMatchType: flipt.MatchType_ANY_MATCH_TYPE, + Value: true, + Constraints: []storage.EvaluationConstraint{ + { + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "hello", + Operator: flipt.OpEQ, + Value: "world", + }, + }, + }, + }, + }, nil) + + res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + assert.Error(t, err) + assert.EqualError(t, err, "rollout rank: 0 detected out of order") + assert.Equal(t, rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON, res.Reason) +} diff --git a/internal/server/evaluation/evaluator.go b/internal/server/evaluation/evaluator.go index 638f336b73..d71923e0bc 100644 --- a/internal/server/evaluation/evaluator.go +++ b/internal/server/evaluation/evaluator.go @@ -18,22 +18,14 @@ import ( "go.uber.org/zap" ) -// EvaluationStorer is the minimal abstraction for interacting with the storage layer for evaluation. -type EvaluationStorer interface { - GetFlag(ctx context.Context, namespaceKey, key string) (*flipt.Flag, error) - GetEvaluationRules(ctx context.Context, namespaceKey string, flagKey string) ([]*storage.EvaluationRule, error) - GetEvaluationDistributions(ctx context.Context, ruleID string) ([]*storage.EvaluationDistribution, error) - GetEvaluationRollouts(ctx context.Context, flagKey string, namespaceKey string) ([]*storage.EvaluationRollout, error) -} - // Evaluator is an implementation of the MultiVariateEvaluator. type Evaluator struct { logger *zap.Logger - store EvaluationStorer + store Storer } // NewEvaluator is the constructor for an Evaluator. -func NewEvaluator(logger *zap.Logger, store EvaluationStorer) *Evaluator { +func NewEvaluator(logger *zap.Logger, store Storer) *Evaluator { return &Evaluator{ logger: logger, store: store, @@ -263,11 +255,11 @@ func doConstraintsMatch(logger *zap.Logger, evalCtx map[string]string, constrain } } - var matched bool = true + var matched = true switch segmentMatchType { case flipt.MatchType_ALL_MATCH_TYPE: - if len(constraints) == constraintMatches { + if len(constraints) != constraintMatches { logger.Debug("did not match ALL constraints") matched = false } diff --git a/internal/server/evaluation/server.go b/internal/server/evaluation/server.go new file mode 100644 index 0000000000..dcd28f38fe --- /dev/null +++ b/internal/server/evaluation/server.go @@ -0,0 +1,41 @@ +package evaluation + +import ( + "context" + + "go.flipt.io/flipt/internal/storage" + "go.flipt.io/flipt/rpc/flipt" + rpcEvalution "go.flipt.io/flipt/rpc/flipt/evaluation" + "go.uber.org/zap" + "google.golang.org/grpc" +) + +// Storer is the minimal abstraction for interacting with the storage layer for evaluation. +type Storer interface { + GetFlag(ctx context.Context, namespaceKey, key string) (*flipt.Flag, error) + GetEvaluationRules(ctx context.Context, namespaceKey string, flagKey string) ([]*storage.EvaluationRule, error) + GetEvaluationDistributions(ctx context.Context, ruleID string) ([]*storage.EvaluationDistribution, error) + GetEvaluationRollouts(ctx context.Context, flagKey string, namespaceKey string) ([]*storage.EvaluationRollout, error) +} + +// Server serves the Flipt evaluate v2 gRPC Server. +type Server struct { + logger *zap.Logger + store Storer + evaluator MultiVariateEvaluator + rpcEvalution.UnimplementedEvaluationServiceServer +} + +// New is constructs a new Server. +func New(logger *zap.Logger, store Storer) *Server { + return &Server{ + logger: logger, + store: store, + evaluator: NewEvaluator(logger, store), + } +} + +// RegisterGRPC registers the EvaluateServer onto the provided gRPC Server. +func (s *Server) RegisterGRPC(server *grpc.Server) { + rpcEvalution.RegisterEvaluationServiceServer(server, s) +} diff --git a/internal/server/middleware/grpc/middleware.go b/internal/server/middleware/grpc/middleware.go index d223a52df4..8f1cd68be3 100644 --- a/internal/server/middleware/grpc/middleware.go +++ b/internal/server/middleware/grpc/middleware.go @@ -95,10 +95,15 @@ func EvaluationUnaryInterceptor(ctx context.Context, req interface{}, _ *grpc.Un // set response fields if resp != nil { - if rr, ok := resp.(*flipt.EvaluationResponse); ok { + switch rr := resp.(type) { + case *flipt.EvaluationResponse: + rr.RequestId = r.(*flipt.EvaluationRequest).RequestId rr.Timestamp = timestamp.New(time.Now().UTC()) rr.RequestDurationMillis = float64(time.Since(startTime)) / float64(time.Millisecond) - } else if rr, ok := resp.(*evaluation.VariantEvaluationResponse); ok { + case *evaluation.VariantEvaluationResponse: + rr.Timestamp = timestamp.New(time.Now().UTC()) + rr.RequestDurationMillis = float64(time.Since(startTime)) / float64(time.Millisecond) + case *evaluation.BooleanEvaluationResponse: rr.Timestamp = timestamp.New(time.Now().UTC()) rr.RequestDurationMillis = float64(time.Since(startTime)) / float64(time.Millisecond) } diff --git a/internal/storage/sql/common/evaluation.go b/internal/storage/sql/common/evaluation.go index 60c131100d..fab6ef99ae 100644 --- a/internal/storage/sql/common/evaluation.go +++ b/internal/storage/sql/common/evaluation.go @@ -163,7 +163,7 @@ func (s *Store) GetEvaluationDistributions(ctx context.Context, ruleID string) ( func (s *Store) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey string) ([]*storage.EvaluationRollout, error) { rows, err := s.db.QueryContext(ctx, ` - SELECT r.id, r.namespace_key, r."type", r."rank", rp.percentage, rp.value, rss.segment_key, rss.match_type, rss.constraint_type, rss.constraint_property, rss.constraint_operator, rss.constraint_value + SELECT r.id, r.namespace_key, r."type", r."rank", rp.percentage, rp.value, rss.segment_key, rss.rollout_segment_value, rss.match_type, rss.constraint_type, rss.constraint_property, rss.constraint_operator, rss.constraint_value FROM rollouts r LEFT JOIN rollout_percentages rp ON (r.id = rp.rollout_id) LEFT JOIN ( @@ -197,6 +197,7 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey rpPercentageNumber sql.NullFloat64 rpPercentageValue sql.NullBool rsSegmentKey sql.NullString + rsSegmentValue sql.NullBool rsMatchType sql.NullInt32 rsConstraintType sql.NullInt32 rsConstraintProperty sql.NullString @@ -212,6 +213,7 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey &rpPercentageNumber, &rpPercentageValue, &rsSegmentKey, + &rsSegmentValue, &rsMatchType, &rsConstraintType, &rsConstraintProperty, @@ -231,6 +233,7 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey } if rsSegmentKey.Valid && + rsSegmentValue.Valid && rsMatchType.Valid && rsConstraintType.Valid && rsConstraintProperty.Valid && @@ -249,6 +252,7 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey storageSegment := &storage.RolloutSegment{ SegmentKey: rsSegmentKey.String, + Value: rsSegmentValue.Bool, SegmentMatchType: flipt.MatchType(rsMatchType.Int32), } From 2b2ddcc10e28d5a8205cfc793416043d207c89ed Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Fri, 30 Jun 2023 01:15:50 -0500 Subject: [PATCH 07/12] chore: address comments and add more tests for behavior --- internal/server/evaluation/evaluation.go | 81 +++++++++---- .../evaluation/evaluation_store_mock.go | 4 +- internal/server/evaluation/evaluation_test.go | 114 +++++++++++++++--- internal/server/evaluation/evaluator.go | 20 ++- internal/server/evaluation/server.go | 4 +- .../server/middleware/grpc/support_test.go | 4 +- internal/server/server.go | 9 +- internal/server/support_test.go | 4 +- internal/storage/fs/snapshot.go | 2 +- internal/storage/sql/common/evaluation.go | 6 +- internal/storage/sql/evaluation_test.go | 80 ++++++++++++ 11 files changed, 262 insertions(+), 66 deletions(-) diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index 7715a34279..5b784e347c 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -2,9 +2,10 @@ package evaluation import ( "context" + "errors" "hash/crc32" - "go.flipt.io/flipt/errors" + errs "go.flipt.io/flipt/errors" fliptotel "go.flipt.io/flipt/internal/server/otel" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" @@ -16,11 +17,31 @@ import ( // Variant evaluates a request for a multi-variate flag and entity. func (s *Server) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.VariantEvaluationResponse, error) { - s.logger.Debug("evaluate", zap.Stringer("request", v)) + var ver = &rpcEvaluation.VariantEvaluationResponse{} + + s.logger.Debug("variant", zap.Stringer("request", v)) if v.NamespaceKey == "" { v.NamespaceKey = storage.DefaultNamespace } + flag, err := s.store.GetFlag(ctx, v.NamespaceKey, v.FlagKey) + if err != nil { + var errnf errs.ErrNotFound + + if errors.As(err, &errnf) { + ver.Reason = rpcEvaluation.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON + return ver, err + } + + ver.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON + return ver, err + } + + if flag.Type != flipt.FlagType_VARIANT_FLAG_TYPE { + ver.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON + return ver, err + } + resp, err := s.evaluator.Evaluate(ctx, &flipt.EvaluationRequest{ RequestId: v.RequestId, FlagKey: v.FlagKey, @@ -29,7 +50,8 @@ func (s *Server) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest NamespaceKey: v.NamespaceKey, }) if err != nil { - return nil, err + ver.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON + return ver, err } spanAttrs := []attribute.KeyValue{ @@ -48,37 +70,54 @@ func (s *Server) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest ) } - ver := &rpcEvaluation.VariantEvaluationResponse{ - Match: resp.Match, - SegmentKey: resp.SegmentKey, - Reason: rpcEvaluation.EvaluationReason(resp.Reason), - VariantKey: resp.Value, - VariantAttachment: resp.Attachment, - } + ver.Match = resp.Match + ver.SegmentKey = resp.SegmentKey + ver.Reason = rpcEvaluation.EvaluationReason(resp.Reason) + ver.VariantKey = resp.Value + ver.VariantAttachment = resp.Attachment // add otel attributes to span span := trace.SpanFromContext(ctx) span.SetAttributes(spanAttrs...) - s.logger.Debug("evaluate", zap.Stringer("response", resp)) + s.logger.Debug("variant", zap.Stringer("response", resp)) return ver, nil } // Boolean evaluates a request for a boolean flag and entity. func (s *Server) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.BooleanEvaluationResponse, error) { - rollouts, err := s.store.GetEvaluationRollouts(ctx, r.FlagKey, r.NamespaceKey) + resp := &rpcEvaluation.BooleanEvaluationResponse{} + + flag, err := s.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) if err != nil { - return nil, err + var errnf errs.ErrNotFound + + if errors.As(err, &errnf) { + resp.Reason = rpcEvaluation.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON + return resp, err + } + + resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON + return resp, err } - resp := &rpcEvaluation.BooleanEvaluationResponse{} + if flag.Type != flipt.FlagType_BOOLEAN_FLAG_TYPE { + resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON + return resp, errs.ErrInvalidf("flag type %s invalid", flipt.FlagType_name[int32(flag.Type)]) + } + + rollouts, err := s.store.GetEvaluationRollouts(ctx, r.NamespaceKey, r.FlagKey) + if err != nil { + resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON + return resp, err + } var lastRank int32 for _, rollout := range rollouts { if rollout.Rank < lastRank { resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON - return resp, errors.ErrInvalidf("rollout rank: %d detected out of order", rollout.Rank) + return resp, errs.ErrInvalidf("rollout rank: %d detected out of order", rollout.Rank) } lastRank = rollout.Rank @@ -98,7 +137,7 @@ func (s *Server) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest return resp, nil } } else if rollout.Segment != nil { - matched, err := doConstraintsMatch(s.logger, r.Context, rollout.Segment.Constraints, rollout.Segment.SegmentMatchType) + matched, err := s.evaluator.matchConstraints(r.Context, rollout.Segment.Constraints, rollout.Segment.SegmentMatchType) if err != nil { resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON return resp, err @@ -118,16 +157,10 @@ func (s *Server) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest } } - f, err := s.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) - if err != nil { - resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON - return resp, err - } - // If we have exhausted all rollouts and we still don't have a match, return the default value. resp.Reason = rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON - resp.Value = f.Enabled - s.logger.Debug("default rollout matched", zap.Bool("value", f.Enabled)) + resp.Value = flag.Enabled + s.logger.Debug("default rollout matched", zap.Bool("value", flag.Enabled)) return resp, nil } diff --git a/internal/server/evaluation/evaluation_store_mock.go b/internal/server/evaluation/evaluation_store_mock.go index a71bf7f6e7..02411fe81e 100644 --- a/internal/server/evaluation/evaluation_store_mock.go +++ b/internal/server/evaluation/evaluation_store_mock.go @@ -33,7 +33,7 @@ func (e *evaluationStoreMock) GetEvaluationDistributions(ctx context.Context, ru return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1) } -func (e *evaluationStoreMock) GetEvaluationRollouts(ctx context.Context, flagKey string, namespaceKey string) ([]*storage.EvaluationRollout, error) { - args := e.Called(ctx, flagKey, namespaceKey) +func (e *evaluationStoreMock) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey string) ([]*storage.EvaluationRollout, error) { + args := e.Called(ctx, namespaceKey, flagKey) return args.Get(0).([]*storage.EvaluationRollout), args.Error(1) } diff --git a/internal/server/evaluation/evaluation_test.go b/internal/server/evaluation/evaluation_test.go index 265c6bce7b..8cc68a0db1 100644 --- a/internal/server/evaluation/evaluation_test.go +++ b/internal/server/evaluation/evaluation_test.go @@ -7,13 +7,39 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + errs "go.flipt.io/flipt/errors" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" rpcEvaluation "go.flipt.io/flipt/rpc/flipt/evaluation" "go.uber.org/zap/zaptest" ) -func TestBoolean_DefaultRule_NoRollouts(t *testing.T) { +func TestBoolean_FlagNotFoundError(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) + defer store.AssertNotCalled(t, "GetEvaluationRollouts", mock.Anything, namespaceKey, flagKey) + + store.On("GetFlag", mock.Anything, mock.Anything, mock.Anything).Return(&flipt.Flag{}, errs.ErrNotFound("test-flag")) + + res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + assert.EqualError(t, err, "test-flag not found") + assert.Equal(t, rpcEvaluation.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON, res.Reason) +} + +func TestBoolean_NonBooleanFlagError(t *testing.T) { var ( flagKey = "test-flag" namespaceKey = "test-namespace" @@ -21,15 +47,46 @@ func TestBoolean_DefaultRule_NoRollouts(t *testing.T) { logger = zaptest.NewLogger(t) s = New(logger, store) ) + defer store.AssertNotCalled(t, "GetEvaluationRollouts", mock.Anything, namespaceKey, flagKey) + + store.On("GetFlag", mock.Anything, mock.Anything, mock.Anything).Return(&flipt.Flag{ + NamespaceKey: "test-namespace", + Key: "test-flag", + Enabled: true, + Type: flipt.FlagType_VARIANT_FLAG_TYPE, + }, nil) + + res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) - store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{}, nil) + assert.EqualError(t, err, "flag type VARIANT_FLAG_TYPE invalid") + assert.Equal(t, rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON, res.Reason) +} + +func TestBoolean_DefaultRule_NoRollouts(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + ) store.On("GetFlag", mock.Anything, mock.Anything, mock.Anything).Return(&flipt.Flag{ NamespaceKey: "test-namespace", Key: "test-flag", Enabled: true, + Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, }, nil) + store.On("GetEvaluationRollouts", mock.Anything, namespaceKey, flagKey).Return([]*storage.EvaluationRollout{}, nil) + res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", @@ -54,7 +111,14 @@ func TestBoolean_DefaultRuleFallthrough_WithPercentageRollout(t *testing.T) { s = New(logger, store) ) - store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + store.On("GetFlag", mock.Anything, mock.Anything, mock.Anything).Return(&flipt.Flag{ + NamespaceKey: "test-namespace", + Key: "test-flag", + Enabled: true, + Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, + }, nil) + + store.On("GetEvaluationRollouts", mock.Anything, namespaceKey, flagKey).Return([]*storage.EvaluationRollout{ { NamespaceKey: namespaceKey, Rank: 1, @@ -66,12 +130,6 @@ func TestBoolean_DefaultRuleFallthrough_WithPercentageRollout(t *testing.T) { }, }, nil) - store.On("GetFlag", mock.Anything, mock.Anything, mock.Anything).Return(&flipt.Flag{ - NamespaceKey: "test-namespace", - Key: "test-flag", - Enabled: true, - }, nil) - res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", @@ -96,9 +154,14 @@ func TestBoolean_PercentageRuleMatch(t *testing.T) { s = New(logger, store) ) - defer store.AssertNotCalled(t, "GetFlag", flagKey, namespaceKey) + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(&flipt.Flag{ + NamespaceKey: "test-namespace", + Key: "test-flag", + Enabled: true, + Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, + }, nil) - store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + store.On("GetEvaluationRollouts", mock.Anything, namespaceKey, flagKey).Return([]*storage.EvaluationRollout{ { NamespaceKey: namespaceKey, Rank: 1, @@ -134,9 +197,14 @@ func TestBoolean_PercentageRuleFallthrough_SegmentMatch(t *testing.T) { s = New(logger, store) ) - defer store.AssertNotCalled(t, "GetFlag", flagKey, namespaceKey) + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(&flipt.Flag{ + NamespaceKey: "test-namespace", + Key: "test-flag", + Enabled: true, + Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, + }, nil) - store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + store.On("GetEvaluationRollouts", mock.Anything, namespaceKey, flagKey).Return([]*storage.EvaluationRollout{ { NamespaceKey: namespaceKey, Rank: 1, @@ -190,9 +258,15 @@ func TestBoolean_SegmentMatch_MultipleConstraints(t *testing.T) { s = New(logger, store) ) - defer store.AssertNotCalled(t, "GetFlag", flagKey, namespaceKey) + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return( + &flipt.Flag{ + NamespaceKey: "test-namespace", + Key: "test-flag", + Enabled: true, + Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, + }, nil) - store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + store.On("GetEvaluationRollouts", mock.Anything, namespaceKey, flagKey).Return([]*storage.EvaluationRollout{ { NamespaceKey: namespaceKey, RolloutType: flipt.RolloutType_SEGMENT_ROLLOUT_TYPE, @@ -243,9 +317,15 @@ func TestBoolean_RulesOutOfOrder(t *testing.T) { s = New(logger, store) ) - defer store.AssertNotCalled(t, "GetFlag", flagKey, namespaceKey) + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return( + &flipt.Flag{ + NamespaceKey: "test-namespace", + Key: "test-flag", + Enabled: true, + Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, + }, nil) - store.On("GetEvaluationRollouts", mock.Anything, flagKey, namespaceKey).Return([]*storage.EvaluationRollout{ + store.On("GetEvaluationRollouts", mock.Anything, namespaceKey, flagKey).Return([]*storage.EvaluationRollout{ { NamespaceKey: namespaceKey, Rank: 1, diff --git a/internal/server/evaluation/evaluator.go b/internal/server/evaluation/evaluator.go index d71923e0bc..938048b4ef 100644 --- a/internal/server/evaluation/evaluator.go +++ b/internal/server/evaluation/evaluator.go @@ -18,7 +18,7 @@ import ( "go.uber.org/zap" ) -// Evaluator is an implementation of the MultiVariateEvaluator. +// Evaluator is an evaluator for legacy flag evaluations. type Evaluator struct { logger *zap.Logger store Storer @@ -32,11 +32,7 @@ func NewEvaluator(logger *zap.Logger, store Storer) *Evaluator { } } -// MultiVariateEvaluator is an abstraction for evaluating a flag against a set of rules for multi-variate flags. -type MultiVariateEvaluator interface { - Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (*flipt.EvaluationResponse, error) -} - +// Evaluate takes in a information for an evaluation request, and returns the appropriate response. func (e *Evaluator) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (resp *flipt.EvaluationResponse, err error) { var ( startTime = time.Now().UTC() @@ -124,7 +120,7 @@ func (e *Evaluator) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (r lastRank = rule.Rank - matched, err := doConstraintsMatch(e.logger, r.Context, rule.Constraints, rule.SegmentMatchType) + matched, err := e.matchConstraints(r.Context, rule.Constraints, rule.SegmentMatchType) if err != nil { resp.Reason = flipt.EvaluationReason_ERROR_EVALUATION_REASON return resp, err @@ -200,9 +196,9 @@ func (e *Evaluator) Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (r return resp, nil } -// doConstraintsMatch is a utility function that will return if all or any constraints have matched for a segment depending +// matchConstraints is a utility function that will return if all or any constraints have matched for a segment depending // on the match type. -func doConstraintsMatch(logger *zap.Logger, evalCtx map[string]string, constraints []storage.EvaluationConstraint, segmentMatchType flipt.MatchType) (bool, error) { +func (e *Evaluator) matchConstraints(evalCtx map[string]string, constraints []storage.EvaluationConstraint, segmentMatchType flipt.MatchType) (bool, error) { constraintMatches := 0 for _, c := range constraints { @@ -260,17 +256,17 @@ func doConstraintsMatch(logger *zap.Logger, evalCtx map[string]string, constrain switch segmentMatchType { case flipt.MatchType_ALL_MATCH_TYPE: if len(constraints) != constraintMatches { - logger.Debug("did not match ALL constraints") + e.logger.Debug("did not match ALL constraints") matched = false } case flipt.MatchType_ANY_MATCH_TYPE: if len(constraints) > 0 && constraintMatches == 0 { - logger.Debug("did not match ANY constraints") + e.logger.Debug("did not match ANY constraints") matched = false } default: - logger.Error("unknown match type", zap.Int32("match_type", int32(segmentMatchType))) + e.logger.Error("unknown match type", zap.Int32("match_type", int32(segmentMatchType))) matched = false } diff --git a/internal/server/evaluation/server.go b/internal/server/evaluation/server.go index dcd28f38fe..8012b31f1f 100644 --- a/internal/server/evaluation/server.go +++ b/internal/server/evaluation/server.go @@ -15,14 +15,14 @@ type Storer interface { GetFlag(ctx context.Context, namespaceKey, key string) (*flipt.Flag, error) GetEvaluationRules(ctx context.Context, namespaceKey string, flagKey string) ([]*storage.EvaluationRule, error) GetEvaluationDistributions(ctx context.Context, ruleID string) ([]*storage.EvaluationDistribution, error) - GetEvaluationRollouts(ctx context.Context, flagKey string, namespaceKey string) ([]*storage.EvaluationRollout, error) + GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey string) ([]*storage.EvaluationRollout, error) } // Server serves the Flipt evaluate v2 gRPC Server. type Server struct { logger *zap.Logger store Storer - evaluator MultiVariateEvaluator + evaluator *Evaluator rpcEvalution.UnimplementedEvaluationServiceServer } diff --git a/internal/server/middleware/grpc/support_test.go b/internal/server/middleware/grpc/support_test.go index 8e6202bb04..35d04daeaa 100644 --- a/internal/server/middleware/grpc/support_test.go +++ b/internal/server/middleware/grpc/support_test.go @@ -240,8 +240,8 @@ func (m *storeMock) GetEvaluationDistributions(ctx context.Context, ruleID strin return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1) } -func (m *storeMock) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey string) ([]*storage.EvaluationRollout, error) { - args := m.Called(ctx, flagKey, namespaceKey) +func (m *storeMock) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey string) ([]*storage.EvaluationRollout, error) { + args := m.Called(ctx, namespaceKey, flagKey) return args.Get(0).([]*storage.EvaluationRollout), args.Error(1) } diff --git a/internal/server/server.go b/internal/server/server.go index b7b293daca..66cf2d9385 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1,6 +1,8 @@ package server import ( + "context" + "go.flipt.io/flipt/internal/server/evaluation" "go.flipt.io/flipt/internal/storage" flipt "go.flipt.io/flipt/rpc/flipt" @@ -10,12 +12,17 @@ import ( var _ flipt.FliptServer = &Server{} +// MultiVariateEvaluator houses the minimal contract for legacy evaluations. +type MultiVariateEvaluator interface { + Evaluate(ctx context.Context, r *flipt.EvaluationRequest) (*flipt.EvaluationResponse, error) +} + // Server serves the Flipt backend type Server struct { logger *zap.Logger store storage.Store flipt.UnimplementedFliptServer - evaluator evaluation.MultiVariateEvaluator + evaluator MultiVariateEvaluator } // New creates a new Server diff --git a/internal/server/support_test.go b/internal/server/support_test.go index cea02250b8..cf64e3079b 100644 --- a/internal/server/support_test.go +++ b/internal/server/support_test.go @@ -233,7 +233,7 @@ func (m *storeMock) GetEvaluationDistributions(ctx context.Context, ruleID strin return args.Get(0).([]*storage.EvaluationDistribution), args.Error(1) } -func (m *storeMock) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey string) ([]*storage.EvaluationRollout, error) { - args := m.Called(ctx, flagKey, namespaceKey) +func (m *storeMock) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey string) ([]*storage.EvaluationRollout, error) { + args := m.Called(ctx, namespaceKey, flagKey) return args.Get(0).([]*storage.EvaluationRollout), args.Error(1) } diff --git a/internal/storage/fs/snapshot.go b/internal/storage/fs/snapshot.go index 55a288609a..83edbd6f0d 100644 --- a/internal/storage/fs/snapshot.go +++ b/internal/storage/fs/snapshot.go @@ -625,7 +625,7 @@ func (ss *storeSnapshot) GetEvaluationDistributions(ctx context.Context, ruleID return dists, nil } -func (ss *storeSnapshot) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey string) ([]*storage.EvaluationRollout, error) { +func (ss *storeSnapshot) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey string) ([]*storage.EvaluationRollout, error) { panic("not implemented") } diff --git a/internal/storage/sql/common/evaluation.go b/internal/storage/sql/common/evaluation.go index fab6ef99ae..a25c931542 100644 --- a/internal/storage/sql/common/evaluation.go +++ b/internal/storage/sql/common/evaluation.go @@ -161,7 +161,7 @@ func (s *Store) GetEvaluationDistributions(ctx context.Context, ruleID string) ( return distributions, nil } -func (s *Store) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey string) ([]*storage.EvaluationRollout, error) { +func (s *Store) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey string) ([]*storage.EvaluationRollout, error) { rows, err := s.db.QueryContext(ctx, ` SELECT r.id, r.namespace_key, r."type", r."rank", rp.percentage, rp.value, rss.segment_key, rss.rollout_segment_value, rss.match_type, rss.constraint_type, rss.constraint_property, rss.constraint_operator, rss.constraint_value FROM rollouts r @@ -172,9 +172,9 @@ func (s *Store) GetEvaluationRollouts(ctx context.Context, flagKey, namespaceKey JOIN segments s ON (rs.segment_key = s."key") JOIN constraints c ON (rs.segment_key = c.segment_key) ) rss ON (r.id = rss.rollout_id) - WHERE r.flag_key = $1 AND r.namespace_key = $2 + WHERE r.namespace_key = $1 AND r.flag_key = $2 ORDER BY r."rank" ASC - `, flagKey, namespaceKey) + `, namespaceKey, flagKey) if err != nil { return nil, err } diff --git a/internal/storage/sql/evaluation_test.go b/internal/storage/sql/evaluation_test.go index ba6b604d2c..356d138383 100644 --- a/internal/storage/sql/evaluation_test.go +++ b/internal/storage/sql/evaluation_test.go @@ -523,3 +523,83 @@ func (s *DBTestSuite) TestGetEvaluationDistributions_MaintainOrder() { assert.Equal(t, variant2.Key, evaluationDistributions[1].VariantKey) assert.Equal(t, float32(20.00), evaluationDistributions[1].Rollout) } + +func (s *DBTestSuite) TestGetEvaluationRollouts() { + t := s.T() + + flag, err := s.store.CreateFlag(context.TODO(), &flipt.CreateFlagRequest{ + Key: t.Name(), + Name: "foo", + Description: "bar", + Enabled: true, + Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, + }) + + require.NoError(t, err) + + segment, err := s.store.CreateSegment(context.TODO(), &flipt.CreateSegmentRequest{ + Key: t.Name(), + Name: "foo", + Description: "bar", + MatchType: flipt.MatchType_ANY_MATCH_TYPE, + }) + + require.NoError(t, err) + + _, err = s.store.CreateConstraint(context.TODO(), &flipt.CreateConstraintRequest{ + SegmentKey: segment.Key, + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "foo", + Operator: "EQ", + Value: "bar", + }) + + require.NoError(t, err) + + _, err = s.store.CreateRollout(context.TODO(), &flipt.CreateRolloutRequest{ + NamespaceKey: "default", + FlagKey: flag.Key, + Rank: 1, + Type: flipt.RolloutType_PERCENTAGE_ROLLOUT_TYPE, + Rule: &flipt.CreateRolloutRequest_Percentage{ + Percentage: &flipt.RolloutPercentage{ + Percentage: 50.0, + Value: false, + }, + }, + }) + + require.NoError(t, err) + + _, err = s.store.CreateRollout(context.TODO(), &flipt.CreateRolloutRequest{ + NamespaceKey: "default", + FlagKey: flag.Key, + Rank: 2, + Type: flipt.RolloutType_SEGMENT_ROLLOUT_TYPE, + Rule: &flipt.CreateRolloutRequest_Segment{ + Segment: &flipt.RolloutSegment{ + SegmentKey: segment.Key, + Value: true, + }, + }, + }) + + require.NoError(t, err) + + evaluationRollouts, err := s.store.GetEvaluationRollouts(context.TODO(), storage.DefaultNamespace, flag.Key) + require.NoError(t, err) + + assert.Equal(t, 2, len(evaluationRollouts)) + + assert.Equal(t, "default", evaluationRollouts[0].NamespaceKey) + assert.Equal(t, int32(1), evaluationRollouts[0].Rank) + assert.NotNil(t, evaluationRollouts[0].Percentage) + assert.Equal(t, float32(50.0), evaluationRollouts[0].Percentage.Percentage) + assert.False(t, evaluationRollouts[0].Percentage.Value, "percentage value is false") + + assert.Equal(t, "default", evaluationRollouts[1].NamespaceKey) + assert.Equal(t, int32(2), evaluationRollouts[1].Rank) + assert.NotNil(t, evaluationRollouts[1].Segment) + assert.Equal(t, segment.Key, evaluationRollouts[1].Segment.SegmentKey) + assert.True(t, evaluationRollouts[1].Segment.Value, "segment value is true") +} From 544b001db99505af3dd3b694652b59198830a740 Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Fri, 30 Jun 2023 13:45:07 -0500 Subject: [PATCH 08/12] chore: readd tests and other couple fixes --- internal/server/evaluation/evaluation.go | 23 +- internal/server/evaluation/evaluation_test.go | 197 +++++++++--------- internal/server/evaluation/evaluator_mock.go | 21 -- 3 files changed, 98 insertions(+), 143 deletions(-) delete mode 100644 internal/server/evaluation/evaluator_mock.go diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index 0827fa1b18..bc343d144b 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -2,7 +2,6 @@ package evaluation import ( "context" - "errors" "hash/crc32" errs "go.flipt.io/flipt/errors" @@ -76,34 +75,23 @@ func (s *Server) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest flag, err := s.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) if err != nil { - var errnf errs.ErrNotFound - - if errors.As(err, &errnf) { - resp.Reason = rpcEvaluation.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON - return resp, err - } - - resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON - return resp, err + return nil, err } if flag.Type != flipt.FlagType_BOOLEAN_FLAG_TYPE { - resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON - return resp, errs.ErrInvalidf("flag type %s invalid", flipt.FlagType_name[int32(flag.Type)]) + return nil, errs.ErrInvalidf("flag type %s invalid", flag.Type) } rollouts, err := s.store.GetEvaluationRollouts(ctx, r.NamespaceKey, r.FlagKey) if err != nil { - resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON - return resp, err + return nil, err } var lastRank int32 for _, rollout := range rollouts { if rollout.Rank < lastRank { - resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON - return resp, errs.ErrInvalidf("rollout rank: %d detected out of order", rollout.Rank) + return nil, errs.ErrInvalidf("rollout rank: %d detected out of order", rollout.Rank) } lastRank = rollout.Rank @@ -125,8 +113,7 @@ func (s *Server) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest } else if rollout.Segment != nil { matched, err := s.evaluator.matchConstraints(r.Context, rollout.Segment.Constraints, rollout.Segment.SegmentMatchType) if err != nil { - resp.Reason = rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON - return resp, err + return nil, err } // if we don't match the segment, fall through to the next rollout. diff --git a/internal/server/evaluation/evaluation_test.go b/internal/server/evaluation/evaluation_test.go index 455824427e..14ea5ab295 100644 --- a/internal/server/evaluation/evaluation_test.go +++ b/internal/server/evaluation/evaluation_test.go @@ -69,106 +69,93 @@ func TestVariant_NonVariantFlag(t *testing.T) { assert.EqualError(t, err, "flag type BOOLEAN_FLAG_TYPE invalid") } -// func TestVariant_EvaluateFailure(t *testing.T) { -// var ( -// flagKey = "test-flag" -// namespaceKey = "test-namespace" -// store = &evaluationStoreMock{} -// evaluator = &evaluatorMock{} -// logger = zaptest.NewLogger(t) -// s = &Server{ -// logger: logger, -// store: store, -// evaluator: evaluator, -// } -// flag = &flipt.Flag{ -// NamespaceKey: namespaceKey, -// Key: flagKey, -// Enabled: true, -// Type: flipt.FlagType_VARIANT_FLAG_TYPE, -// } -// ) - -// store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(flag, nil) - -// evaluator.On("Evaluate", mock.Anything, flag, &flipt.EvaluationRequest{ -// FlagKey: flagKey, -// NamespaceKey: namespaceKey, -// EntityId: "test-entity", -// Context: map[string]string{ -// "hello": "world", -// }, -// }).Return(&flipt.EvaluationResponse{}, errs.ErrInvalid("some error")) - -// v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ -// FlagKey: flagKey, -// EntityId: "test-entity", -// NamespaceKey: namespaceKey, -// Context: map[string]string{ -// "hello": "world", -// }, -// }) - -// require.Nil(t, v) - -// assert.EqualError(t, err, "some error") -// } - -// func TestVariant_Success(t *testing.T) { -// var ( -// flagKey = "test-flag" -// namespaceKey = "test-namespace" -// store = &evaluationStoreMock{} -// evaluator = &evaluatorMock{} -// logger = zaptest.NewLogger(t) -// s = &Server{ -// logger: logger, -// store: store, -// evaluator: evaluator, -// } -// flag = &flipt.Flag{ -// NamespaceKey: namespaceKey, -// Key: flagKey, -// Enabled: true, -// Type: flipt.FlagType_VARIANT_FLAG_TYPE, -// } -// ) - -// store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(flag, nil) - -// evaluator.On("Evaluate", mock.Anything, flag, &flipt.EvaluationRequest{ -// FlagKey: flagKey, -// NamespaceKey: namespaceKey, -// EntityId: "test-entity", -// Context: map[string]string{ -// "hello": "world", -// }, -// }).Return( -// &flipt.EvaluationResponse{ -// FlagKey: flagKey, -// NamespaceKey: namespaceKey, -// Value: "foo", -// Match: true, -// SegmentKey: "segment", -// Reason: flipt.EvaluationReason(rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON), -// }, nil) - -// v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ -// FlagKey: flagKey, -// EntityId: "test-entity", -// NamespaceKey: namespaceKey, -// Context: map[string]string{ -// "hello": "world", -// }, -// }) - -// require.NoError(t, err) - -// assert.Equal(t, true, v.Match) -// assert.Equal(t, "foo", v.VariantKey) -// assert.Equal(t, "segment", v.SegmentKey) -// assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, v.Reason) -// } +func TestVariant_EvaluateFailure_OnGetEvaluationRules(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + flag = &flipt.Flag{ + NamespaceKey: namespaceKey, + Key: flagKey, + Enabled: true, + Type: flipt.FlagType_VARIANT_FLAG_TYPE, + } + ) + + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(flag, nil) + + store.On("GetEvaluationRules", mock.Anything, namespaceKey, flagKey).Return([]*storage.EvaluationRule{}, errs.ErrInvalid("some invalid error")) + + v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.Nil(t, v) + + assert.EqualError(t, err, "some invalid error") +} + +func TestVariant_Success(t *testing.T) { + var ( + flagKey = "test-flag" + namespaceKey = "test-namespace" + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = New(logger, store) + flag = &flipt.Flag{ + NamespaceKey: namespaceKey, + Key: flagKey, + Enabled: true, + Type: flipt.FlagType_VARIANT_FLAG_TYPE, + } + ) + + store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(flag, nil) + + store.On("GetEvaluationRules", mock.Anything, namespaceKey, flagKey).Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: flagKey, + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "hello", + Operator: flipt.OpEQ, + Value: "world", + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return([]*storage.EvaluationDistribution{}, nil) + + v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + FlagKey: flagKey, + EntityId: "test-entity", + NamespaceKey: namespaceKey, + Context: map[string]string{ + "hello": "world", + }, + }) + + require.NoError(t, err) + + assert.Equal(t, true, v.Match) + assert.Equal(t, "bar", v.SegmentKey) + assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, v.Reason) +} func TestBoolean_FlagNotFoundError(t *testing.T) { var ( @@ -191,8 +178,9 @@ func TestBoolean_FlagNotFoundError(t *testing.T) { }, }) + require.Nil(t, res) + assert.EqualError(t, err, "test-flag not found") - assert.Equal(t, rpcEvaluation.EvaluationReason_FLAG_NOT_FOUND_EVALUATION_REASON, res.Reason) } func TestBoolean_NonBooleanFlagError(t *testing.T) { @@ -221,8 +209,9 @@ func TestBoolean_NonBooleanFlagError(t *testing.T) { }, }) + require.Nil(t, res) + assert.EqualError(t, err, "flag type VARIANT_FLAG_TYPE invalid") - assert.Equal(t, rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON, res.Reason) } func TestBoolean_DefaultRule_NoRollouts(t *testing.T) { @@ -520,7 +509,7 @@ func TestBoolean_RulesOutOfOrder(t *testing.T) { }, }) - assert.Error(t, err) + require.Nil(t, res) + assert.EqualError(t, err, "rollout rank: 0 detected out of order") - assert.Equal(t, rpcEvaluation.EvaluationReason_ERROR_EVALUATION_REASON, res.Reason) } diff --git a/internal/server/evaluation/evaluator_mock.go b/internal/server/evaluation/evaluator_mock.go deleted file mode 100644 index f57e7464cf..0000000000 --- a/internal/server/evaluation/evaluator_mock.go +++ /dev/null @@ -1,21 +0,0 @@ -package evaluation - -import ( - "context" - - "github.com/stretchr/testify/mock" - "go.flipt.io/flipt/rpc/flipt" -) - -type evaluatorMock struct { - mock.Mock -} - -func (e *evaluatorMock) String() string { - return "mock" -} - -func (e *evaluatorMock) Evaluate(ctx context.Context, flag *flipt.Flag, er *flipt.EvaluationRequest) (*flipt.EvaluationResponse, error) { - args := e.Called(ctx, flag, er) - return args.Get(0).(*flipt.EvaluationResponse), args.Error(1) -} From 7c03b2eef875540a3c7b68761999dbde69fb808f Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Fri, 30 Jun 2023 14:14:07 -0500 Subject: [PATCH 09/12] chore: add middleware bits for timestamp and duration --- internal/server/evaluation/evaluation.go | 52 ++-------------------- internal/server/evaluation/evaluator.go | 55 ++++++++++++++++++++++++ rpc/flipt/evaluation/evaluation.go | 13 ++++++ 3 files changed, 71 insertions(+), 49 deletions(-) diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index bc343d144b..1b8912073a 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -2,7 +2,6 @@ package evaluation import ( "context" - "hash/crc32" errs "go.flipt.io/flipt/errors" fliptotel "go.flipt.io/flipt/internal/server/otel" @@ -71,8 +70,6 @@ func (s *Server) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest // Boolean evaluates a request for a boolean flag and entity. func (s *Server) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.BooleanEvaluationResponse, error) { - resp := &rpcEvaluation.BooleanEvaluationResponse{} - flag, err := s.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) if err != nil { return nil, err @@ -87,53 +84,10 @@ func (s *Server) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest return nil, err } - var lastRank int32 - - for _, rollout := range rollouts { - if rollout.Rank < lastRank { - return nil, errs.ErrInvalidf("rollout rank: %d detected out of order", rollout.Rank) - } - - lastRank = rollout.Rank - - if rollout.Percentage != nil { - // consistent hashing based on the entity id and flag key. - hash := crc32.ChecksumIEEE([]byte(r.EntityId + r.FlagKey)) - - normalizedValue := float32(int(hash) % 100) - - // if this case does not hold, fall through to the next rollout. - if normalizedValue < rollout.Percentage.Percentage { - resp.Value = rollout.Percentage.Value - resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON - s.logger.Debug("percentage based matched", zap.Int("rank", int(rollout.Rank)), zap.String("rollout_type", "percentage")) - - return resp, nil - } - } else if rollout.Segment != nil { - matched, err := s.evaluator.matchConstraints(r.Context, rollout.Segment.Constraints, rollout.Segment.SegmentMatchType) - if err != nil { - return nil, err - } - - // if we don't match the segment, fall through to the next rollout. - if !matched { - continue - } - - resp.Value = rollout.Segment.Value - resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON - - s.logger.Debug("segment based matched", zap.Int("rank", int(rollout.Rank)), zap.String("segment", rollout.Segment.SegmentKey)) - - return resp, nil - } + resp, err := s.evaluator.booleanMatch(r, flag.Enabled, rollouts) + if err != nil { + return nil, err } - // If we have exhausted all rollouts and we still don't have a match, return the default value. - resp.Reason = rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON - resp.Value = flag.Enabled - s.logger.Debug("default rollout matched", zap.Bool("value", flag.Enabled)) - return resp, nil } diff --git a/internal/server/evaluation/evaluator.go b/internal/server/evaluation/evaluator.go index 6bf6193f6d..457971bdd3 100644 --- a/internal/server/evaluation/evaluator.go +++ b/internal/server/evaluation/evaluator.go @@ -12,6 +12,7 @@ import ( "go.flipt.io/flipt/internal/server/metrics" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" + rpcEvaluation "go.flipt.io/flipt/rpc/flipt/evaluation" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.uber.org/zap" @@ -190,6 +191,60 @@ func (e *Evaluator) Evaluate(ctx context.Context, flag *flipt.Flag, r *flipt.Eva return resp, nil } +func (e *Evaluator) booleanMatch(r *rpcEvaluation.EvaluationRequest, flagValue bool, rollouts []*storage.EvaluationRollout) (*rpcEvaluation.BooleanEvaluationResponse, error) { + resp := &rpcEvaluation.BooleanEvaluationResponse{} + + var lastRank int32 + + for _, rollout := range rollouts { + if rollout.Rank < lastRank { + return &rpcEvaluation.BooleanEvaluationResponse{}, errs.ErrInvalidf("rollout rank: %d detected out of order", rollout.Rank) + } + + lastRank = rollout.Rank + + if rollout.Percentage != nil { + // consistent hashing based on the entity id and flag key. + hash := crc32.ChecksumIEEE([]byte(r.EntityId + r.FlagKey)) + + normalizedValue := float32(int(hash) % 100) + + // if this case does not hold, fall through to the next rollout. + if normalizedValue < rollout.Percentage.Percentage { + resp.Value = rollout.Percentage.Value + resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON + e.logger.Debug("percentage based matched", zap.Int("rank", int(rollout.Rank)), zap.String("rollout_type", "percentage")) + + return resp, nil + } + } else if rollout.Segment != nil { + matched, err := e.matchConstraints(r.Context, rollout.Segment.Constraints, rollout.Segment.SegmentMatchType) + if err != nil { + return resp, err + } + + // if we don't match the segment, fall through to the next rollout. + if !matched { + continue + } + + resp.Value = rollout.Segment.Value + resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON + + e.logger.Debug("segment based matched", zap.Int("rank", int(rollout.Rank)), zap.String("segment", rollout.Segment.SegmentKey)) + + return resp, nil + } + } + + // If we have exhausted all rollouts and we still don't have a match, return the default value. + resp.Reason = rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON + resp.Value = flagValue + e.logger.Debug("default rollout matched", zap.Bool("value", flagValue)) + + return resp, nil +} + // matchConstraints is a utility function that will return if all or any constraints have matched for a segment depending // on the match type. func (e *Evaluator) matchConstraints(evalCtx map[string]string, constraints []storage.EvaluationConstraint, segmentMatchType flipt.MatchType) (bool, error) { diff --git a/rpc/flipt/evaluation/evaluation.go b/rpc/flipt/evaluation/evaluation.go index 7720987308..df144c79ef 100644 --- a/rpc/flipt/evaluation/evaluation.go +++ b/rpc/flipt/evaluation/evaluation.go @@ -45,6 +45,14 @@ func (x *VariantEvaluationResponse) SetRequestIDIfNotBlank(id string) string { return x.RequestId } +func (x *BooleanEvaluationResponse) SetRequestIDIfNotBlank(id string) string { + if x.RequestId == "" { + x.RequestId = id + } + + return x.RequestId +} + // SetRequestIDIfNotBlank attempts to set the provided ID on the instance // If the ID was blank, it returns the ID provided to this call. // If the ID was not blank, it returns the ID found on the instance. @@ -106,6 +114,11 @@ func (x *VariantEvaluationResponse) SetTimestamps(start, end time.Time) { x.RequestDurationMillis = float64(end.Sub(start)) / float64(time.Millisecond) } +func (x *BooleanEvaluationResponse) SetTimestamps(start, end time.Time) { + x.Timestamp = timestamppb.New(end) + x.RequestDurationMillis = float64(end.Sub(start)) / float64(time.Millisecond) +} + // SetTimestamps records the start and end times on the target instance. func (x *BatchEvaluationResponse) SetTimestamps(start, end time.Time) { x.RequestDurationMillis = float64(end.Sub(start)) / float64(time.Millisecond) From 2c72d292e1b1d53ccb8652aff18cb710cb39920e Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Sat, 1 Jul 2023 10:37:35 -0500 Subject: [PATCH 10/12] chore: remove storage implementation of getting rollouts --- internal/storage/sql/common/evaluation.go | 109 +--------------------- internal/storage/sql/evaluation_test.go | 80 ---------------- 2 files changed, 1 insertion(+), 188 deletions(-) diff --git a/internal/storage/sql/common/evaluation.go b/internal/storage/sql/common/evaluation.go index a25c931542..a040cc897e 100644 --- a/internal/storage/sql/common/evaluation.go +++ b/internal/storage/sql/common/evaluation.go @@ -162,112 +162,5 @@ func (s *Store) GetEvaluationDistributions(ctx context.Context, ruleID string) ( } func (s *Store) GetEvaluationRollouts(ctx context.Context, namespaceKey, flagKey string) ([]*storage.EvaluationRollout, error) { - rows, err := s.db.QueryContext(ctx, ` - SELECT r.id, r.namespace_key, r."type", r."rank", rp.percentage, rp.value, rss.segment_key, rss.rollout_segment_value, rss.match_type, rss.constraint_type, rss.constraint_property, rss.constraint_operator, rss.constraint_value - FROM rollouts r - LEFT JOIN rollout_percentages rp ON (r.id = rp.rollout_id) - LEFT JOIN ( - SELECT rs.rollout_id, rs.segment_key, s.match_type, rs.value AS rollout_segment_value, c."type" AS constraint_type, c.property AS constraint_property, c.operator AS constraint_operator, c.value AS constraint_value - FROM rollout_segments rs - JOIN segments s ON (rs.segment_key = s."key") - JOIN constraints c ON (rs.segment_key = c.segment_key) - ) rss ON (r.id = rss.rollout_id) - WHERE r.namespace_key = $1 AND r.flag_key = $2 - ORDER BY r."rank" ASC - `, namespaceKey, flagKey) - if err != nil { - return nil, err - } - - defer func() { - if cerr := rows.Close(); cerr != nil && err == nil { - err = cerr - } - }() - - var ( - uniqueSegmentedRollouts = make(map[string]*storage.EvaluationRollout) - rollouts = []*storage.EvaluationRollout{} - ) - - for rows.Next() { - var ( - rolloutId string - evaluationRollout storage.EvaluationRollout - rpPercentageNumber sql.NullFloat64 - rpPercentageValue sql.NullBool - rsSegmentKey sql.NullString - rsSegmentValue sql.NullBool - rsMatchType sql.NullInt32 - rsConstraintType sql.NullInt32 - rsConstraintProperty sql.NullString - rsConstraintOperator sql.NullString - rsConstraintValue sql.NullString - ) - - if err := rows.Scan( - &rolloutId, - &evaluationRollout.NamespaceKey, - &evaluationRollout.RolloutType, - &evaluationRollout.Rank, - &rpPercentageNumber, - &rpPercentageValue, - &rsSegmentKey, - &rsSegmentValue, - &rsMatchType, - &rsConstraintType, - &rsConstraintProperty, - &rsConstraintOperator, - &rsConstraintValue, - ); err != nil { - return rollouts, err - } - - if rpPercentageNumber.Valid && rpPercentageValue.Valid { - storagePercentage := &storage.RolloutPercentage{ - Percentage: float32(rpPercentageNumber.Float64), - Value: rpPercentageValue.Bool, - } - - evaluationRollout.Percentage = storagePercentage - } - - if rsSegmentKey.Valid && - rsSegmentValue.Valid && - rsMatchType.Valid && - rsConstraintType.Valid && - rsConstraintProperty.Valid && - rsConstraintOperator.Valid && rsConstraintValue.Valid { - c := storage.EvaluationConstraint{ - Type: flipt.ComparisonType(rsConstraintType.Int32), - Property: rsConstraintProperty.String, - Operator: rsConstraintOperator.String, - Value: rsConstraintValue.String, - } - - if existingSegment, ok := uniqueSegmentedRollouts[rolloutId]; ok { - existingSegment.Segment.Constraints = append(existingSegment.Segment.Constraints, c) - continue - } - - storageSegment := &storage.RolloutSegment{ - SegmentKey: rsSegmentKey.String, - Value: rsSegmentValue.Bool, - SegmentMatchType: flipt.MatchType(rsMatchType.Int32), - } - - storageSegment.Constraints = append(storageSegment.Constraints, c) - - evaluationRollout.Segment = storageSegment - uniqueSegmentedRollouts[rolloutId] = &evaluationRollout - } - - rollouts = append(rollouts, &evaluationRollout) - } - - if err := rows.Err(); err != nil { - return rollouts, err - } - - return rollouts, nil + return []*storage.EvaluationRollout{}, nil } diff --git a/internal/storage/sql/evaluation_test.go b/internal/storage/sql/evaluation_test.go index 356d138383..ba6b604d2c 100644 --- a/internal/storage/sql/evaluation_test.go +++ b/internal/storage/sql/evaluation_test.go @@ -523,83 +523,3 @@ func (s *DBTestSuite) TestGetEvaluationDistributions_MaintainOrder() { assert.Equal(t, variant2.Key, evaluationDistributions[1].VariantKey) assert.Equal(t, float32(20.00), evaluationDistributions[1].Rollout) } - -func (s *DBTestSuite) TestGetEvaluationRollouts() { - t := s.T() - - flag, err := s.store.CreateFlag(context.TODO(), &flipt.CreateFlagRequest{ - Key: t.Name(), - Name: "foo", - Description: "bar", - Enabled: true, - Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, - }) - - require.NoError(t, err) - - segment, err := s.store.CreateSegment(context.TODO(), &flipt.CreateSegmentRequest{ - Key: t.Name(), - Name: "foo", - Description: "bar", - MatchType: flipt.MatchType_ANY_MATCH_TYPE, - }) - - require.NoError(t, err) - - _, err = s.store.CreateConstraint(context.TODO(), &flipt.CreateConstraintRequest{ - SegmentKey: segment.Key, - Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, - Property: "foo", - Operator: "EQ", - Value: "bar", - }) - - require.NoError(t, err) - - _, err = s.store.CreateRollout(context.TODO(), &flipt.CreateRolloutRequest{ - NamespaceKey: "default", - FlagKey: flag.Key, - Rank: 1, - Type: flipt.RolloutType_PERCENTAGE_ROLLOUT_TYPE, - Rule: &flipt.CreateRolloutRequest_Percentage{ - Percentage: &flipt.RolloutPercentage{ - Percentage: 50.0, - Value: false, - }, - }, - }) - - require.NoError(t, err) - - _, err = s.store.CreateRollout(context.TODO(), &flipt.CreateRolloutRequest{ - NamespaceKey: "default", - FlagKey: flag.Key, - Rank: 2, - Type: flipt.RolloutType_SEGMENT_ROLLOUT_TYPE, - Rule: &flipt.CreateRolloutRequest_Segment{ - Segment: &flipt.RolloutSegment{ - SegmentKey: segment.Key, - Value: true, - }, - }, - }) - - require.NoError(t, err) - - evaluationRollouts, err := s.store.GetEvaluationRollouts(context.TODO(), storage.DefaultNamespace, flag.Key) - require.NoError(t, err) - - assert.Equal(t, 2, len(evaluationRollouts)) - - assert.Equal(t, "default", evaluationRollouts[0].NamespaceKey) - assert.Equal(t, int32(1), evaluationRollouts[0].Rank) - assert.NotNil(t, evaluationRollouts[0].Percentage) - assert.Equal(t, float32(50.0), evaluationRollouts[0].Percentage.Percentage) - assert.False(t, evaluationRollouts[0].Percentage.Value, "percentage value is false") - - assert.Equal(t, "default", evaluationRollouts[1].NamespaceKey) - assert.Equal(t, int32(2), evaluationRollouts[1].Rank) - assert.NotNil(t, evaluationRollouts[1].Segment) - assert.Equal(t, segment.Key, evaluationRollouts[1].Segment.SegmentKey) - assert.True(t, evaluationRollouts[1].Segment.Value, "segment value is true") -} From 703fddfb428f10392727b6d892daf0b9f8b1165f Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Mon, 3 Jul 2023 08:56:55 -0500 Subject: [PATCH 11/12] chore: use lowercase package names, and return nil instead of real value --- internal/server/evaluation/evaluation.go | 10 ++--- internal/server/evaluation/evaluation_test.go | 38 +++++++++---------- internal/server/evaluation/evaluator.go | 17 +++++---- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/internal/server/evaluation/evaluation.go b/internal/server/evaluation/evaluation.go index 1b8912073a..2f5e81f2f8 100644 --- a/internal/server/evaluation/evaluation.go +++ b/internal/server/evaluation/evaluation.go @@ -7,15 +7,15 @@ import ( fliptotel "go.flipt.io/flipt/internal/server/otel" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" - rpcEvaluation "go.flipt.io/flipt/rpc/flipt/evaluation" + rpcevaluation "go.flipt.io/flipt/rpc/flipt/evaluation" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" ) // Variant evaluates a request for a multi-variate flag and entity. -func (s *Server) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.VariantEvaluationResponse, error) { - var ver = &rpcEvaluation.VariantEvaluationResponse{} +func (s *Server) Variant(ctx context.Context, v *rpcevaluation.EvaluationRequest) (*rpcevaluation.VariantEvaluationResponse, error) { + var ver = &rpcevaluation.VariantEvaluationResponse{} flag, err := s.store.GetFlag(ctx, v.NamespaceKey, v.FlagKey) if err != nil { @@ -56,7 +56,7 @@ func (s *Server) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest ver.Match = resp.Match ver.SegmentKey = resp.SegmentKey - ver.Reason = rpcEvaluation.EvaluationReason(resp.Reason) + ver.Reason = rpcevaluation.EvaluationReason(resp.Reason) ver.VariantKey = resp.Value ver.VariantAttachment = resp.Attachment @@ -69,7 +69,7 @@ func (s *Server) Variant(ctx context.Context, v *rpcEvaluation.EvaluationRequest } // Boolean evaluates a request for a boolean flag and entity. -func (s *Server) Boolean(ctx context.Context, r *rpcEvaluation.EvaluationRequest) (*rpcEvaluation.BooleanEvaluationResponse, error) { +func (s *Server) Boolean(ctx context.Context, r *rpcevaluation.EvaluationRequest) (*rpcevaluation.BooleanEvaluationResponse, error) { flag, err := s.store.GetFlag(ctx, r.NamespaceKey, r.FlagKey) if err != nil { return nil, err diff --git a/internal/server/evaluation/evaluation_test.go b/internal/server/evaluation/evaluation_test.go index 14ea5ab295..43e252b4d8 100644 --- a/internal/server/evaluation/evaluation_test.go +++ b/internal/server/evaluation/evaluation_test.go @@ -10,7 +10,7 @@ import ( errs "go.flipt.io/flipt/errors" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" - rpcEvaluation "go.flipt.io/flipt/rpc/flipt/evaluation" + rpcevaluation "go.flipt.io/flipt/rpc/flipt/evaluation" "go.uber.org/zap/zaptest" ) @@ -25,7 +25,7 @@ func TestVariant_FlagNotFound(t *testing.T) { store.On("GetFlag", mock.Anything, namespaceKey, flagKey).Return(&flipt.Flag{}, errs.ErrNotFound("test-flag")) - v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + v, err := s.Variant(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -55,7 +55,7 @@ func TestVariant_NonVariantFlag(t *testing.T) { Type: flipt.FlagType_BOOLEAN_FLAG_TYPE, }, nil) - v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + v, err := s.Variant(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -88,7 +88,7 @@ func TestVariant_EvaluateFailure_OnGetEvaluationRules(t *testing.T) { store.On("GetEvaluationRules", mock.Anything, namespaceKey, flagKey).Return([]*storage.EvaluationRule{}, errs.ErrInvalid("some invalid error")) - v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + v, err := s.Variant(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -141,7 +141,7 @@ func TestVariant_Success(t *testing.T) { store.On("GetEvaluationDistributions", mock.Anything, "1").Return([]*storage.EvaluationDistribution{}, nil) - v, err := s.Variant(context.TODO(), &rpcEvaluation.EvaluationRequest{ + v, err := s.Variant(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -154,7 +154,7 @@ func TestVariant_Success(t *testing.T) { assert.Equal(t, true, v.Match) assert.Equal(t, "bar", v.SegmentKey) - assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, v.Reason) + assert.Equal(t, rpcevaluation.EvaluationReason_MATCH_EVALUATION_REASON, v.Reason) } func TestBoolean_FlagNotFoundError(t *testing.T) { @@ -169,7 +169,7 @@ func TestBoolean_FlagNotFoundError(t *testing.T) { store.On("GetFlag", mock.Anything, mock.Anything, mock.Anything).Return(&flipt.Flag{}, errs.ErrNotFound("test-flag")) - res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + res, err := s.Boolean(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -200,7 +200,7 @@ func TestBoolean_NonBooleanFlagError(t *testing.T) { Type: flipt.FlagType_VARIANT_FLAG_TYPE, }, nil) - res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + res, err := s.Boolean(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -232,7 +232,7 @@ func TestBoolean_DefaultRule_NoRollouts(t *testing.T) { store.On("GetEvaluationRollouts", mock.Anything, namespaceKey, flagKey).Return([]*storage.EvaluationRollout{}, nil) - res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + res, err := s.Boolean(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -244,7 +244,7 @@ func TestBoolean_DefaultRule_NoRollouts(t *testing.T) { require.NoError(t, err) assert.Equal(t, true, res.Value) - assert.Equal(t, rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON, res.Reason) + assert.Equal(t, rpcevaluation.EvaluationReason_DEFAULT_EVALUATION_REASON, res.Reason) } func TestBoolean_DefaultRuleFallthrough_WithPercentageRollout(t *testing.T) { @@ -275,7 +275,7 @@ func TestBoolean_DefaultRuleFallthrough_WithPercentageRollout(t *testing.T) { }, }, nil) - res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + res, err := s.Boolean(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -287,7 +287,7 @@ func TestBoolean_DefaultRuleFallthrough_WithPercentageRollout(t *testing.T) { require.NoError(t, err) assert.Equal(t, true, res.Value) - assert.Equal(t, rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON, res.Reason) + assert.Equal(t, rpcevaluation.EvaluationReason_DEFAULT_EVALUATION_REASON, res.Reason) } func TestBoolean_PercentageRuleMatch(t *testing.T) { @@ -318,7 +318,7 @@ func TestBoolean_PercentageRuleMatch(t *testing.T) { }, }, nil) - res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + res, err := s.Boolean(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -330,7 +330,7 @@ func TestBoolean_PercentageRuleMatch(t *testing.T) { require.NoError(t, err) assert.Equal(t, false, res.Value) - assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, res.Reason) + assert.Equal(t, rpcevaluation.EvaluationReason_MATCH_EVALUATION_REASON, res.Reason) } func TestBoolean_PercentageRuleFallthrough_SegmentMatch(t *testing.T) { @@ -379,7 +379,7 @@ func TestBoolean_PercentageRuleFallthrough_SegmentMatch(t *testing.T) { }, }, nil) - res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + res, err := s.Boolean(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -391,7 +391,7 @@ func TestBoolean_PercentageRuleFallthrough_SegmentMatch(t *testing.T) { require.NoError(t, err) assert.Equal(t, true, res.Value) - assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, res.Reason) + assert.Equal(t, rpcevaluation.EvaluationReason_MATCH_EVALUATION_REASON, res.Reason) } func TestBoolean_SegmentMatch_MultipleConstraints(t *testing.T) { @@ -438,7 +438,7 @@ func TestBoolean_SegmentMatch_MultipleConstraints(t *testing.T) { }, }, nil) - res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + res, err := s.Boolean(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, @@ -450,7 +450,7 @@ func TestBoolean_SegmentMatch_MultipleConstraints(t *testing.T) { require.NoError(t, err) assert.Equal(t, true, res.Value) - assert.Equal(t, rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON, res.Reason) + assert.Equal(t, rpcevaluation.EvaluationReason_MATCH_EVALUATION_REASON, res.Reason) } func TestBoolean_RulesOutOfOrder(t *testing.T) { @@ -500,7 +500,7 @@ func TestBoolean_RulesOutOfOrder(t *testing.T) { }, }, nil) - res, err := s.Boolean(context.TODO(), &rpcEvaluation.EvaluationRequest{ + res, err := s.Boolean(context.TODO(), &rpcevaluation.EvaluationRequest{ FlagKey: flagKey, EntityId: "test-entity", NamespaceKey: namespaceKey, diff --git a/internal/server/evaluation/evaluator.go b/internal/server/evaluation/evaluator.go index 457971bdd3..0e927eddc6 100644 --- a/internal/server/evaluation/evaluator.go +++ b/internal/server/evaluation/evaluator.go @@ -2,6 +2,7 @@ package evaluation import ( "context" + "fmt" "hash/crc32" "sort" "strconv" @@ -12,7 +13,7 @@ import ( "go.flipt.io/flipt/internal/server/metrics" "go.flipt.io/flipt/internal/storage" "go.flipt.io/flipt/rpc/flipt" - rpcEvaluation "go.flipt.io/flipt/rpc/flipt/evaluation" + rpcevaluation "go.flipt.io/flipt/rpc/flipt/evaluation" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.uber.org/zap" @@ -191,14 +192,14 @@ func (e *Evaluator) Evaluate(ctx context.Context, flag *flipt.Flag, r *flipt.Eva return resp, nil } -func (e *Evaluator) booleanMatch(r *rpcEvaluation.EvaluationRequest, flagValue bool, rollouts []*storage.EvaluationRollout) (*rpcEvaluation.BooleanEvaluationResponse, error) { - resp := &rpcEvaluation.BooleanEvaluationResponse{} +func (e *Evaluator) booleanMatch(r *rpcevaluation.EvaluationRequest, flagValue bool, rollouts []*storage.EvaluationRollout) (*rpcevaluation.BooleanEvaluationResponse, error) { + resp := &rpcevaluation.BooleanEvaluationResponse{} var lastRank int32 for _, rollout := range rollouts { if rollout.Rank < lastRank { - return &rpcEvaluation.BooleanEvaluationResponse{}, errs.ErrInvalidf("rollout rank: %d detected out of order", rollout.Rank) + return nil, fmt.Errorf("rollout rank: %d detected out of order", rollout.Rank) } lastRank = rollout.Rank @@ -212,7 +213,7 @@ func (e *Evaluator) booleanMatch(r *rpcEvaluation.EvaluationRequest, flagValue b // if this case does not hold, fall through to the next rollout. if normalizedValue < rollout.Percentage.Percentage { resp.Value = rollout.Percentage.Value - resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON + resp.Reason = rpcevaluation.EvaluationReason_MATCH_EVALUATION_REASON e.logger.Debug("percentage based matched", zap.Int("rank", int(rollout.Rank)), zap.String("rollout_type", "percentage")) return resp, nil @@ -220,7 +221,7 @@ func (e *Evaluator) booleanMatch(r *rpcEvaluation.EvaluationRequest, flagValue b } else if rollout.Segment != nil { matched, err := e.matchConstraints(r.Context, rollout.Segment.Constraints, rollout.Segment.SegmentMatchType) if err != nil { - return resp, err + return nil, err } // if we don't match the segment, fall through to the next rollout. @@ -229,7 +230,7 @@ func (e *Evaluator) booleanMatch(r *rpcEvaluation.EvaluationRequest, flagValue b } resp.Value = rollout.Segment.Value - resp.Reason = rpcEvaluation.EvaluationReason_MATCH_EVALUATION_REASON + resp.Reason = rpcevaluation.EvaluationReason_MATCH_EVALUATION_REASON e.logger.Debug("segment based matched", zap.Int("rank", int(rollout.Rank)), zap.String("segment", rollout.Segment.SegmentKey)) @@ -238,7 +239,7 @@ func (e *Evaluator) booleanMatch(r *rpcEvaluation.EvaluationRequest, flagValue b } // If we have exhausted all rollouts and we still don't have a match, return the default value. - resp.Reason = rpcEvaluation.EvaluationReason_DEFAULT_EVALUATION_REASON + resp.Reason = rpcevaluation.EvaluationReason_DEFAULT_EVALUATION_REASON resp.Value = flagValue e.logger.Debug("default rollout matched", zap.Bool("value", flagValue)) From 38604aca025ac5eb207193afbde1ab85b2a5b9e5 Mon Sep 17 00:00:00 2001 From: Yoofi Quansah Date: Mon, 3 Jul 2023 09:42:42 -0500 Subject: [PATCH 12/12] chore: add tests for coverage --- internal/server/evaluation/evaluator_test.go | 144 +++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/internal/server/evaluation/evaluator_test.go b/internal/server/evaluation/evaluator_test.go index 5bc9f535bc..902fb93d0e 100644 --- a/internal/server/evaluation/evaluator_test.go +++ b/internal/server/evaluation/evaluator_test.go @@ -896,6 +896,87 @@ func TestEvaluator_RulesOutOfOrder(t *testing.T) { assert.Equal(t, flipt.EvaluationReason_ERROR_EVALUATION_REASON, resp.Reason) } +func TestEvaluator_ErrorParsingNumber(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 1, + Constraints: []storage.EvaluationConstraint{ + { + ID: "2", + Type: flipt.ComparisonType_NUMBER_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "boz", + }, + }, + }, + }, nil) + + resp, err := s.Evaluate(context.TODO(), enabledFlag, &flipt.EvaluationRequest{ + EntityId: "1", + FlagKey: "foo", + Context: map[string]string{ + "bar": "baz", + }, + }) + + assert.Error(t, err) + assert.EqualError(t, err, "parsing number from \"baz\"") + assert.False(t, resp.Match) + assert.Equal(t, flipt.EvaluationReason_ERROR_EVALUATION_REASON, resp.Reason) +} +func TestEvaluator_ErrorParsingDateTime(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 1, + Constraints: []storage.EvaluationConstraint{ + { + ID: "2", + Type: flipt.ComparisonType_DATETIME_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "boz", + }, + }, + }, + }, nil) + + resp, err := s.Evaluate(context.TODO(), enabledFlag, &flipt.EvaluationRequest{ + EntityId: "1", + FlagKey: "foo", + Context: map[string]string{ + "bar": "baz", + }, + }) + + assert.Error(t, err) + assert.EqualError(t, err, "parsing datetime from \"baz\"") + assert.False(t, resp.Match) + assert.Equal(t, flipt.EvaluationReason_ERROR_EVALUATION_REASON, resp.Reason) +} + func TestEvaluator_ErrorGettingDistributions(t *testing.T) { var ( store = &evaluationStoreMock{} @@ -1023,6 +1104,69 @@ func TestEvaluator_MatchAll_NoVariants_NoDistributions(t *testing.T) { } } +func TestEvaluator_DistributionNotMatched(t *testing.T) { + var ( + store = &evaluationStoreMock{} + logger = zaptest.NewLogger(t) + s = NewEvaluator(logger, store) + ) + + store.On("GetEvaluationRules", mock.Anything, mock.Anything, "foo").Return( + []*storage.EvaluationRule{ + { + ID: "1", + FlagKey: "foo", + SegmentKey: "bar", + SegmentMatchType: flipt.MatchType_ALL_MATCH_TYPE, + Rank: 0, + Constraints: []storage.EvaluationConstraint{ + // constraint: bar (string) == baz + { + ID: "2", + Type: flipt.ComparisonType_STRING_COMPARISON_TYPE, + Property: "bar", + Operator: flipt.OpEQ, + Value: "baz", + }, + // constraint: admin (bool) == true + { + ID: "3", + Type: flipt.ComparisonType_BOOLEAN_COMPARISON_TYPE, + Property: "admin", + Operator: flipt.OpTrue, + }, + }, + }, + }, nil) + + store.On("GetEvaluationDistributions", mock.Anything, "1").Return( + []*storage.EvaluationDistribution{ + { + ID: "4", + RuleID: "1", + VariantID: "5", + Rollout: 10, + VariantKey: "boz", + VariantAttachment: `{"key":"value"}`, + }, + }, nil) + + resp, err := s.Evaluate(context.TODO(), enabledFlag, &flipt.EvaluationRequest{ + FlagKey: "foo", + EntityId: "123", + Context: map[string]string{ + "bar": "baz", + "admin": "true", + }, + }) + + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.Equal(t, "foo", resp.FlagKey) + + assert.False(t, resp.Match, "distribution not matched") +} + func TestEvaluator_MatchAll_SingleVariantDistribution(t *testing.T) { var ( store = &evaluationStoreMock{}