Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Evaluation with FlagTagsOperator to match any or all tags #408

Merged
merged 1 commit into from
Oct 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions docs/api_docs/bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,17 @@ definitions:
x-omitempty: true
items:
type: string
flagTagsOperator:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, this doc is generated, you can move the change to https://github.com/checkr/flagr/blob/master/swagger/index.yaml#L518, and then run make gen to get the same result

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I wasn't sure how this work

description: >-
determine how flagTags is used to filter flags to be evaluated. OR
extends the evaluation to those which contains at least one of the
provided flagTags or AND limit the evaluation to those which contains
all the flagTags.
type: string
enum:
- ANY
- ALL
default: ANY
evalResult:
type: object
properties:
Expand Down Expand Up @@ -1403,6 +1414,17 @@ definitions:
type: string
minLength: 1
minItems: 1
flagTagsOperator:
description: >-
determine how flagTags is used to filter flags to be evaluated. OR
extends the evaluation to those which contains at least one of the
provided flagTags or AND limit the evaluation to those which contains
all the flagTags.
type: string
enum:
- ANY
- ALL
default: ANY
evaluationBatchResponse:
type: object
required:
Expand Down
24 changes: 24 additions & 0 deletions integration_tests/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,29 @@ step_11_test_tag_batch_evaluation()

}

step_12_test_tag_operator_batch_evaluation()
{
flagr_url=$1:18000/api/v1
sleep 5

shakedown POST $flagr_url/evaluation/batch -H 'Content-Type:application/json' -d '{"entities":[{ "entityType": "externalalert", "entityContext": {"property_1": "value_2"} }],"flagTags": ["value_1", "value_2"], "flagTagsOperator": "ALL", "enableDebug": false }'
status 200
matches "\"flagID\":1"
matches "\"variantKey\":\"key_1\""
matches "\"variantID\":1"

shakedown POST $flagr_url/evaluation/batch -H 'Content-Type:application/json' -d '{"entities":[{ "entityType": "externalalert", "entityContext": {"property_1": "value_2"} }],"flagTags": ["value_1", "value_3"], "flagTagsOperator": "ALL", "enableDebug": false }'
status 200
contains "\"evaluationResults\":null"

shakedown POST $flagr_url/evaluation/batch -H 'Content-Type:application/json' -d '{"entities":[{ "entityType": "externalalert", "entityContext": {"property_1": "value_2"} }],"flagTags": ["value_1", "value_3"], "flagTagsOperator": "ANY", "enableDebug": false }'
status 200
matches "\"flagID\":1"
matches "\"variantKey\":\"key_1\""
matches "\"variantID\":1"

}


start_test()
{
Expand All @@ -387,6 +410,7 @@ start_test()
step_9_test_export $flagr_host
step_10_test_crud_tag $flagr_host
step_11_test_tag_batch_evaluation $flagr_host
step_12_test_tag_operator_batch_evaluation $flagr_host
}

start(){
Expand Down
15 changes: 8 additions & 7 deletions pkg/handler/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,19 @@ func (e *eval) PostEvaluationBatch(params evaluation.PostEvaluationBatchParams)
flagIDs := params.Body.FlagIDs
flagKeys := params.Body.FlagKeys
flagTags := params.Body.FlagTags
flagTagsOperator := params.Body.FlagTagsOperator
results := &models.EvaluationBatchResponse{}

// TODO make it concurrent
for _, entity := range entities {
if len(flagTags) > 0 {
evalContext := models.EvalContext{
EnableDebug: params.Body.EnableDebug,
EntityContext: entity.EntityContext,
EntityID: entity.EntityID,
EntityType: entity.EntityType,
FlagTags: flagTags,
EnableDebug: params.Body.EnableDebug,
EntityContext: entity.EntityContext,
EntityID: entity.EntityID,
EntityType: entity.EntityType,
FlagTags: flagTags,
FlagTagsOperator: flagTagsOperator,
}
evalResults := EvalFlagsByTags(evalContext)
results.EvaluationResults = append(results.EvaluationResults, evalResults...)
Expand Down Expand Up @@ -133,8 +135,7 @@ var LookupFlag = func(evalContext models.EvalContext) *entity.Flag {

var EvalFlagsByTags = func(evalContext models.EvalContext) []*models.EvalResult {
cache := GetEvalCache()

fs := cache.GetByTags(evalContext.FlagTags)
fs := cache.GetByTags(evalContext.FlagTags, evalContext.FlagTagsOperator)
results := []*models.EvalResult{}
for _, f := range fs {
results = append(results, EvalFlagWithContext(f, evalContext))
Expand Down
59 changes: 54 additions & 5 deletions pkg/handler/eval_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"sync/atomic"
"time"

"github.com/checkr/flagr/swagger_gen/models"

"github.com/checkr/flagr/pkg/config"
"github.com/checkr/flagr/pkg/entity"
"github.com/checkr/flagr/pkg/util"
Expand Down Expand Up @@ -59,9 +61,29 @@ func (ec *EvalCache) Start() {
}()
}

func (ec *EvalCache) GetByTags(tags []string) []*entity.Flag {
func (ec *EvalCache) GetByTags(tags []string, operator *string) []*entity.Flag {
var results map[uint]*entity.Flag

if operator == nil || *operator == models.EvaluationBatchRequestFlagTagsOperatorANY {
results = ec.getByTagsANY(tags)
}

if operator != nil && *operator == models.EvaluationBatchRequestFlagTagsOperatorALL {
results = ec.getByTagsALL(tags)
}

values := make([]*entity.Flag, 0, len(results))
for _, f := range results {
values = append(values, f)
}

return values
}

func (ec *EvalCache) getByTagsANY(tags []string) map[uint]*entity.Flag {
results := map[uint]*entity.Flag{}
cache := ec.cache.Load().(*cacheContainer)

for _, t := range tags {
fSet, ok := cache.tagCache[t]
if ok {
Expand All @@ -70,13 +92,40 @@ func (ec *EvalCache) GetByTags(tags []string) []*entity.Flag {
}
}
}
return results
}

values := make([]*entity.Flag, 0, len(results))
for _, f := range results {
values = append(values, f)
func (ec *EvalCache) getByTagsALL(tags []string) map[uint]*entity.Flag {
results := map[uint]*entity.Flag{}
cache := ec.cache.Load().(*cacheContainer)

for i, t := range tags {
fSet, ok := cache.tagCache[t]
if !ok {
// no flags
return map[uint]*entity.Flag{}
}

if i == 0 {
// store all the flags
for fID, f := range fSet {
results[fID] = f
}
zhouzhuojie marked this conversation as resolved.
Show resolved Hide resolved
} else {
for fID := range results {
if _, ok := fSet[fID]; !ok {
delete(results, fID)
}
}

// no flags left
if len(results) == 0 {
return results
}
}
}

return values
return results
}

// GetByFlagKeyOrID gets the flag by Key or ID
Expand Down
21 changes: 20 additions & 1 deletion pkg/handler/eval_cache_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package handler

import (
"github.com/checkr/flagr/swagger_gen/models"
"testing"

"github.com/checkr/flagr/pkg/entity"
Expand Down Expand Up @@ -35,8 +36,26 @@ func TestGetByTags(t *testing.T) {
for i, s := range fixtureFlag.Tags {
tags[i] = s.Value
}
f := ec.GetByTags(tags)
any := models.EvalContextFlagTagsOperatorANY
all := models.EvalContextFlagTagsOperatorALL
f := ec.GetByTags(tags, &any)
assert.Len(t, f, 1)
assert.Equal(t, f[0].ID, fixtureFlag.ID)
assert.Equal(t, f[0].Tags[0].Value, fixtureFlag.Tags[0].Value)

tags = make([]string, len(fixtureFlag.Tags)+1)
for i, s := range fixtureFlag.Tags {
tags[i] = s.Value
}
tags[len(tags)-1] = "tag3"

f = ec.GetByTags(tags, &any)
assert.Len(t, f, 1)

var operator *string
f = ec.GetByTags(tags, operator)
assert.Len(t, f, 1)

f = ec.GetByTags(tags, &all)
assert.Len(t, f, 0)
}
20 changes: 20 additions & 0 deletions swagger/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,16 @@ definitions:
x-omitempty: true
items:
type: string
flagTagsOperator:
description: >-
determine how flagTags is used to filter flags to be evaluated. OR extends the evaluation to those which
contains at least one of the provided flagTags or AND limit the evaluation to those which contains all the
flagTags.
type: string
enum:
- "ANY"
- "ALL"
default: "ANY"
evalResult:
type: object
properties:
Expand Down Expand Up @@ -549,6 +559,16 @@ definitions:
type: string
minLength: 1
minItems: 1
flagTagsOperator:
description: >-
determine how flagTags is used to filter flags to be evaluated. OR extends the evaluation to those which
contains at least one of the provided flagTags or AND limit the evaluation to those which contains all the
flagTags.
type: string
enum:
- "ANY"
- "ALL"
default: "ANY"
evaluationBatchResponse:
type: object
required:
Expand Down
53 changes: 53 additions & 0 deletions swagger_gen/models/eval_context.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading