Skip to content

Commit 3f6b78c

Browse files
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>
1 parent 797b482 commit 3f6b78c

File tree

3 files changed

+130
-3
lines changed

3 files changed

+130
-3
lines changed

core/pkg/evaluator/json.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,12 @@ func (je *Resolver) evaluateVariant(ctx context.Context, reqID string, flagKey s
372372

373373
// check if string is "null" before we strip quotes, so we can differentiate between JSON null and "null"
374374
trimmed := strings.TrimSpace(result.String())
375+
375376
if trimmed == "null" {
377+
if flag.DefaultVariant == "" {
378+
return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.FlagNotFoundErrorCode)
379+
}
380+
376381
return flag.DefaultVariant, flag.Variants, model.DefaultReason, metadata, nil
377382
}
378383

@@ -387,6 +392,11 @@ func (je *Resolver) evaluateVariant(ctx context.Context, reqID string, flagKey s
387392
fmt.Sprintf("invalid or missing variant: %s for flagKey: %s, variant is not valid", variant, flagKey))
388393
return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.ParseErrorCode)
389394
}
395+
396+
if flag.DefaultVariant == "" {
397+
return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.FlagNotFoundErrorCode)
398+
}
399+
390400
return flag.DefaultVariant, flag.Variants, model.StaticReason, metadata, nil
391401
}
392402

@@ -479,6 +489,11 @@ func configToFlagDefinition(log *logger.Logger, config string, definition *Defin
479489
// validateDefaultVariants returns an error if any of the default variants aren't valid
480490
func validateDefaultVariants(flags *Definition) error {
481491
for name, flag := range flags.Flags {
492+
// Default Variant is not provided in the config
493+
if flag.DefaultVariant == "" {
494+
continue
495+
}
496+
482497
if _, ok := flag.Variants[flag.DefaultVariant]; !ok {
483498
return fmt.Errorf(
484499
"default variant: '%s' isn't a valid variant of flag: '%s'", flag.DefaultVariant, name,

core/pkg/evaluator/json_test.go

Lines changed: 114 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,90 @@ const ValidFlags = `{
4444
}
4545
}`
4646

47+
const NullDefault = `{
48+
"flags": {
49+
"validFlag": {
50+
"state": "ENABLED",
51+
"variants": {
52+
"on": true,
53+
"off": false
54+
},
55+
"defaultVariant": null
56+
}
57+
}
58+
}`
59+
60+
const UndefinedDefault = `{
61+
"flags": {
62+
"validFlag": {
63+
"state": "ENABLED",
64+
"variants": {
65+
"on": true,
66+
"off": false
67+
}
68+
}
69+
}
70+
}`
71+
72+
const NullDefaultWithTargetting = `{
73+
"flags": {
74+
"validFlag": {
75+
"state": "ENABLED",
76+
"variants": {
77+
"on": true,
78+
"off": false
79+
},
80+
"defaultVariant": null,
81+
"targeting": {
82+
"if": [
83+
{
84+
"==": [
85+
{
86+
"var": [
87+
"key"
88+
]
89+
},
90+
"value"
91+
]
92+
},
93+
"on"
94+
]
95+
}
96+
}
97+
}
98+
}`
99+
100+
const UndefinedDefaultWithTargetting = `{
101+
"flags": {
102+
"validFlag": {
103+
"state": "ENABLED",
104+
"variants": {
105+
"on": true,
106+
"off": false
107+
},
108+
"targeting": {
109+
"if": [
110+
{
111+
"==": [
112+
{
113+
"var": [
114+
"key"
115+
]
116+
},
117+
"value"
118+
]
119+
},
120+
"on"
121+
]
122+
}
123+
}
124+
}
125+
}`
126+
47127
const (
48128
FlagSetID = "testSetId"
49129
Version = "v33"
130+
ValidFlag = "validFlag"
50131
MissingFlag = "missingFlag"
51132
StaticBoolFlag = "staticBoolFlag"
52133
StaticBoolValue = true
@@ -325,8 +406,8 @@ func TestSetState_Invalid_Error(t *testing.T) {
325406

326407
// set state with an invalid flag definition
327408
_, _, err := evaluator.SetState(sync.DataSync{FlagData: InvalidFlags})
328-
if err == nil {
329-
t.Fatalf("expected error")
409+
if err != nil {
410+
t.Fatalf("unexpected error")
330411
}
331412
}
332413

@@ -873,6 +954,37 @@ func TestResolveAsAnyValue(t *testing.T) {
873954
}
874955
}
875956

957+
func TestResolve_DefaultVariant(t *testing.T) {
958+
tests := []struct {
959+
flags string
960+
flagKey string
961+
context map[string]interface{}
962+
reason string
963+
errorCode string
964+
}{
965+
{NullDefault, ValidFlag, nil, model.ErrorReason, model.FlagNotFoundErrorCode},
966+
{UndefinedDefault, ValidFlag, nil, model.ErrorReason, model.FlagNotFoundErrorCode},
967+
{NullDefaultWithTargetting, ValidFlag, nil, model.ErrorReason, model.FlagNotFoundErrorCode},
968+
{UndefinedDefaultWithTargetting, ValidFlag, nil, model.ErrorReason, model.FlagNotFoundErrorCode},
969+
}
970+
971+
for _, test := range tests {
972+
t.Run("", func(t *testing.T) {
973+
evaluator := evaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
974+
_, _, err := evaluator.SetState(sync.DataSync{FlagData: test.flags})
975+
976+
if err != nil {
977+
t.Fatalf("expected no error")
978+
}
979+
980+
anyResult := evaluator.ResolveAsAnyValue(context.TODO(), "", test.flagKey, test.context)
981+
982+
assert.Equal(t, model.ErrorReason, anyResult.Reason)
983+
assert.EqualError(t, anyResult.Error, test.errorCode)
984+
})
985+
}
986+
}
987+
876988
func TestSetState_DefaultVariantValidation(t *testing.T) {
877989
tests := map[string]struct {
878990
jsonFlags string

0 commit comments

Comments
 (0)