feat: allowing null/missing defaultValue (#1659)
<!-- Please use this template for your pull request. --> <!-- Please use the sections that you need and delete other sections --> ## This PR - adds support for null/missing default values ### Related Issues Fixes #1647 ### Notes Points to be noted... - If there is no `defaultValue` and no targetting rules, then `FlagNotFound` is returned - If targetting doesn't resolve a variant, and there is no `defaultValue`, then `FlagNotFound` is returned --------- Signed-off-by: Rahul Baradol <rahul.baradol.14@gmail.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
This commit is contained in:
parent
797b48294b
commit
3f6b78c8cc
|
|
@ -372,7 +372,12 @@ func (je *Resolver) evaluateVariant(ctx context.Context, reqID string, flagKey s
|
||||||
|
|
||||||
// check if string is "null" before we strip quotes, so we can differentiate between JSON null and "null"
|
// check if string is "null" before we strip quotes, so we can differentiate between JSON null and "null"
|
||||||
trimmed := strings.TrimSpace(result.String())
|
trimmed := strings.TrimSpace(result.String())
|
||||||
|
|
||||||
if trimmed == "null" {
|
if trimmed == "null" {
|
||||||
|
if flag.DefaultVariant == "" {
|
||||||
|
return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.FlagNotFoundErrorCode)
|
||||||
|
}
|
||||||
|
|
||||||
return flag.DefaultVariant, flag.Variants, model.DefaultReason, metadata, nil
|
return flag.DefaultVariant, flag.Variants, model.DefaultReason, metadata, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -387,6 +392,11 @@ func (je *Resolver) evaluateVariant(ctx context.Context, reqID string, flagKey s
|
||||||
fmt.Sprintf("invalid or missing variant: %s for flagKey: %s, variant is not valid", variant, flagKey))
|
fmt.Sprintf("invalid or missing variant: %s for flagKey: %s, variant is not valid", variant, flagKey))
|
||||||
return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.ParseErrorCode)
|
return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.ParseErrorCode)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if flag.DefaultVariant == "" {
|
||||||
|
return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.FlagNotFoundErrorCode)
|
||||||
|
}
|
||||||
|
|
||||||
return flag.DefaultVariant, flag.Variants, model.StaticReason, metadata, nil
|
return flag.DefaultVariant, flag.Variants, model.StaticReason, metadata, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -479,6 +489,11 @@ func configToFlagDefinition(log *logger.Logger, config string, definition *Defin
|
||||||
// validateDefaultVariants returns an error if any of the default variants aren't valid
|
// validateDefaultVariants returns an error if any of the default variants aren't valid
|
||||||
func validateDefaultVariants(flags *Definition) error {
|
func validateDefaultVariants(flags *Definition) error {
|
||||||
for name, flag := range flags.Flags {
|
for name, flag := range flags.Flags {
|
||||||
|
// Default Variant is not provided in the config
|
||||||
|
if flag.DefaultVariant == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
if _, ok := flag.Variants[flag.DefaultVariant]; !ok {
|
if _, ok := flag.Variants[flag.DefaultVariant]; !ok {
|
||||||
return fmt.Errorf(
|
return fmt.Errorf(
|
||||||
"default variant: '%s' isn't a valid variant of flag: '%s'", flag.DefaultVariant, name,
|
"default variant: '%s' isn't a valid variant of flag: '%s'", flag.DefaultVariant, name,
|
||||||
|
|
|
||||||
|
|
@ -44,9 +44,90 @@ const ValidFlags = `{
|
||||||
}
|
}
|
||||||
}`
|
}`
|
||||||
|
|
||||||
|
const NullDefault = `{
|
||||||
|
"flags": {
|
||||||
|
"validFlag": {
|
||||||
|
"state": "ENABLED",
|
||||||
|
"variants": {
|
||||||
|
"on": true,
|
||||||
|
"off": false
|
||||||
|
},
|
||||||
|
"defaultVariant": null
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`
|
||||||
|
|
||||||
|
const UndefinedDefault = `{
|
||||||
|
"flags": {
|
||||||
|
"validFlag": {
|
||||||
|
"state": "ENABLED",
|
||||||
|
"variants": {
|
||||||
|
"on": true,
|
||||||
|
"off": false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`
|
||||||
|
|
||||||
|
const NullDefaultWithTargetting = `{
|
||||||
|
"flags": {
|
||||||
|
"validFlag": {
|
||||||
|
"state": "ENABLED",
|
||||||
|
"variants": {
|
||||||
|
"on": true,
|
||||||
|
"off": false
|
||||||
|
},
|
||||||
|
"defaultVariant": null,
|
||||||
|
"targeting": {
|
||||||
|
"if": [
|
||||||
|
{
|
||||||
|
"==": [
|
||||||
|
{
|
||||||
|
"var": [
|
||||||
|
"key"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"value"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"on"
|
||||||
|
]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`
|
||||||
|
|
||||||
|
const UndefinedDefaultWithTargetting = `{
|
||||||
|
"flags": {
|
||||||
|
"validFlag": {
|
||||||
|
"state": "ENABLED",
|
||||||
|
"variants": {
|
||||||
|
"on": true,
|
||||||
|
"off": false
|
||||||
|
},
|
||||||
|
"targeting": {
|
||||||
|
"if": [
|
||||||
|
{
|
||||||
|
"==": [
|
||||||
|
{
|
||||||
|
"var": [
|
||||||
|
"key"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"value"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"on"
|
||||||
|
]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}`
|
||||||
|
|
||||||
const (
|
const (
|
||||||
FlagSetID = "testSetId"
|
FlagSetID = "testSetId"
|
||||||
Version = "v33"
|
Version = "v33"
|
||||||
|
ValidFlag = "validFlag"
|
||||||
MissingFlag = "missingFlag"
|
MissingFlag = "missingFlag"
|
||||||
StaticBoolFlag = "staticBoolFlag"
|
StaticBoolFlag = "staticBoolFlag"
|
||||||
StaticBoolValue = true
|
StaticBoolValue = true
|
||||||
|
|
@ -325,8 +406,8 @@ func TestSetState_Invalid_Error(t *testing.T) {
|
||||||
|
|
||||||
// set state with an invalid flag definition
|
// set state with an invalid flag definition
|
||||||
_, _, err := evaluator.SetState(sync.DataSync{FlagData: InvalidFlags})
|
_, _, err := evaluator.SetState(sync.DataSync{FlagData: InvalidFlags})
|
||||||
if err == nil {
|
if err != nil {
|
||||||
t.Fatalf("expected error")
|
t.Fatalf("unexpected error")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -873,6 +954,37 @@ func TestResolveAsAnyValue(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestResolve_DefaultVariant(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
flags string
|
||||||
|
flagKey string
|
||||||
|
context map[string]interface{}
|
||||||
|
reason string
|
||||||
|
errorCode string
|
||||||
|
}{
|
||||||
|
{NullDefault, ValidFlag, nil, model.ErrorReason, model.FlagNotFoundErrorCode},
|
||||||
|
{UndefinedDefault, ValidFlag, nil, model.ErrorReason, model.FlagNotFoundErrorCode},
|
||||||
|
{NullDefaultWithTargetting, ValidFlag, nil, model.ErrorReason, model.FlagNotFoundErrorCode},
|
||||||
|
{UndefinedDefaultWithTargetting, ValidFlag, nil, model.ErrorReason, model.FlagNotFoundErrorCode},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, test := range tests {
|
||||||
|
t.Run("", func(t *testing.T) {
|
||||||
|
evaluator := evaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
|
||||||
|
_, _, err := evaluator.SetState(sync.DataSync{FlagData: test.flags})
|
||||||
|
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected no error")
|
||||||
|
}
|
||||||
|
|
||||||
|
anyResult := evaluator.ResolveAsAnyValue(context.TODO(), "", test.flagKey, test.context)
|
||||||
|
|
||||||
|
assert.Equal(t, model.ErrorReason, anyResult.Reason)
|
||||||
|
assert.EqualError(t, anyResult.Error, test.errorCode)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestSetState_DefaultVariantValidation(t *testing.T) {
|
func TestSetState_DefaultVariantValidation(t *testing.T) {
|
||||||
tests := map[string]struct {
|
tests := map[string]struct {
|
||||||
jsonFlags string
|
jsonFlags string
|
||||||
|
|
|
||||||
2
schemas
2
schemas
|
|
@ -1 +1 @@
|
||||||
Subproject commit 2852d7772e6b8674681a6ee6b88db10dbe3f6899
|
Subproject commit 08b4c52d3b86d686f11e74322dbfee775db91656
|
||||||
Loading…
Reference in New Issue