From 315a5cc48bdcda667a650b29186e818e8decb5dc Mon Sep 17 00:00:00 2001 From: Tristan Swadell Date: Mon, 29 Apr 2024 10:35:18 -0700 Subject: [PATCH] Introduce a flag for supporting 'null' as absent during optional field selection (#938) --- cel/cel_test.go | 65 +++++++++++++++++++++++++++++++ cel/library.go | 4 ++ cel/options.go | 3 ++ cel/program.go | 7 +++- interpreter/attribute_patterns.go | 6 +-- interpreter/attributes.go | 59 ++++++++++++++++++++-------- interpreter/interpreter_test.go | 14 +++++++ 7 files changed, 136 insertions(+), 22 deletions(-) diff --git a/cel/cel_test.go b/cel/cel_test.go index f386b13b..def4e9e5 100644 --- a/cel/cel_test.go +++ b/cel/cel_test.go @@ -2659,6 +2659,71 @@ func TestOptionalValuesEval(t *testing.T) { } } +func TestOptionalValuesEvalNoneIfNull(t *testing.T) { + env := testEnv(t, + OptionalTypes(), + OptionalFieldSelectionNoneIfNull(true), + ) + adapter := env.TypeAdapter() + tests := []struct { + expr string + in map[string]any + out any + }{ + { + expr: `{}.?invalid`, + out: types.OptionalNone, + }, + { + expr: `{'null_field': dyn(null)}.?null_field`, + out: types.OptionalNone, + }, + { + expr: `{'null_field': dyn(null)}.?null_field.?nested`, + out: types.OptionalNone, + }, + { + expr: `{'zero_field': dyn(0)}.?zero_field.?invalid`, + out: "no such key: invalid", + }, + { + expr: `{0: dyn(0)}[?0].?invalid`, + out: "no such key: invalid", + }, + { + expr: `{true: dyn(0)}[?false].?invalid`, + out: types.OptionalNone, + }, + { + expr: `{true: dyn(0)}[?true].?invalid`, + out: "no such key: invalid", + }, + } + + for i, tst := range tests { + tc := tst + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + ast, iss := env.Compile(tc.expr) + if iss.Err() != nil { + t.Fatalf("%v failed: %v", tc.expr, iss.Err()) + } + prg, err := env.Program(ast) + if err != nil { + t.Errorf("env.Program() failed: %v", err) + } + out, _, err := prg.Eval(tc.in) + if err != nil && err.Error() != tc.out { + t.Errorf("prg.Eval() got %v, wanted %v", err, tc.out) + } + want := adapter.NativeToValue(tc.out) + if err == nil && out.Equal(want) != types.True { + t.Errorf("prg.Eval() got %v, wanted %v", out, want) + } + }) + } + +} + func TestOptionalMacroError(t *testing.T) { env := testEnv(t, OptionalTypes(), diff --git a/cel/library.go b/cel/library.go index 754d91b9..5fe9f707 100644 --- a/cel/library.go +++ b/cel/library.go @@ -446,6 +446,10 @@ func enableOptionalSyntax() EnvOption { } } +func OptionalFieldSelectionNoneIfNull(value bool) EnvOption { + return features(featureOptionalFieldSelectionNoneIfNull, value) +} + func decorateOptionalOr(i interpreter.Interpretable) (interpreter.Interpretable, error) { call, ok := i.(interpreter.InterpretableCall) if !ok { diff --git a/cel/options.go b/cel/options.go index f080e192..69600228 100644 --- a/cel/options.go +++ b/cel/options.go @@ -61,6 +61,9 @@ const ( // compressing the logic graph to a single call when multiple like-operator // expressions occur: e.g. a && b && c && d -> call(_&&_, [a, b, c, d]) featureVariadicLogicalASTs + + // Enable optional field selection to treat null values as optional.none() + featureOptionalFieldSelectionNoneIfNull ) // EnvOption is a functional interface for configuring the environment. diff --git a/cel/program.go b/cel/program.go index ece9fbda..bfa6da54 100644 --- a/cel/program.go +++ b/cel/program.go @@ -187,10 +187,13 @@ func newProgram(e *Env, a *Ast, opts []ProgramOption) (Program, error) { // Set the attribute factory after the options have been set. var attrFactory interpreter.AttributeFactory + attrFactorOpts := []interpreter.AttrFactoryOption{ + interpreter.OptionalFieldSelectionNoneIfNull(p.HasFeature(featureOptionalFieldSelectionNoneIfNull)), + } if p.evalOpts&OptPartialEval == OptPartialEval { - attrFactory = interpreter.NewPartialAttributeFactory(e.Container, e.adapter, e.provider) + attrFactory = interpreter.NewPartialAttributeFactory(e.Container, e.adapter, e.provider, attrFactorOpts...) } else { - attrFactory = interpreter.NewAttributeFactory(e.Container, e.adapter, e.provider) + attrFactory = interpreter.NewAttributeFactory(e.Container, e.adapter, e.provider, attrFactorOpts...) } interp := interpreter.NewInterpreter(disp, e.Container, e.provider, e.adapter, attrFactory) p.interpreter = interp diff --git a/interpreter/attribute_patterns.go b/interpreter/attribute_patterns.go index 1fbaaf17..8f19bde7 100644 --- a/interpreter/attribute_patterns.go +++ b/interpreter/attribute_patterns.go @@ -178,10 +178,8 @@ func numericValueEquals(value any, celValue ref.Val) bool { // NewPartialAttributeFactory returns an AttributeFactory implementation capable of performing // AttributePattern matches with PartialActivation inputs. -func NewPartialAttributeFactory(container *containers.Container, - adapter types.Adapter, - provider types.Provider) AttributeFactory { - fac := NewAttributeFactory(container, adapter, provider) +func NewPartialAttributeFactory(container *containers.Container, adapter types.Adapter, provider types.Provider, opts ...AttrFactoryOption) AttributeFactory { + fac := NewAttributeFactory(container, adapter, provider, opts...) return &partialAttributeFactory{ AttributeFactory: fac, container: container, diff --git a/interpreter/attributes.go b/interpreter/attributes.go index e505f85e..17daaebb 100644 --- a/interpreter/attributes.go +++ b/interpreter/attributes.go @@ -126,21 +126,39 @@ type NamespacedAttribute interface { Qualifiers() []Qualifier } +// AttrFactoryOption specifies a functional option for configuring an attribute factory. +type AttrFactoryOption func(*attrFactory) *attrFactory + +// OptionalFieldSelectionNoneIfNull indicates that when a null value is encountered during +// optional field selection that it is treated as optional.none() rather than as optional.of(null). +func OptionalFieldSelectionNoneIfNull(value bool) AttrFactoryOption { + return func(fac *attrFactory) *attrFactory { + fac.optionalFieldSelectionNoneIfNull = value + return fac + } +} + // NewAttributeFactory returns a default AttributeFactory which is produces Attribute values // capable of resolving types by simple names and qualify the values using the supported qualifier // types: bool, int, string, and uint. -func NewAttributeFactory(cont *containers.Container, a types.Adapter, p types.Provider) AttributeFactory { - return &attrFactory{ +func NewAttributeFactory(cont *containers.Container, a types.Adapter, p types.Provider, opts ...AttrFactoryOption) AttributeFactory { + fac := &attrFactory{ container: cont, adapter: a, provider: p, } + for _, o := range opts { + fac = o(fac) + } + return fac } type attrFactory struct { container *containers.Container adapter types.Adapter provider types.Provider + + optionalFieldSelectionNoneIfNull bool } // AbsoluteAttribute refers to a variable value and an optional qualifier path. @@ -149,12 +167,13 @@ type attrFactory struct { // resolution rules. func (r *attrFactory) AbsoluteAttribute(id int64, names ...string) NamespacedAttribute { return &absoluteAttribute{ - id: id, - namespaceNames: names, - qualifiers: []Qualifier{}, - adapter: r.adapter, - provider: r.provider, - fac: r, + id: id, + namespaceNames: names, + qualifiers: []Qualifier{}, + adapter: r.adapter, + provider: r.provider, + fac: r, + optionalFieldSelectionNoneIfNull: r.optionalFieldSelectionNoneIfNull, } } @@ -188,11 +207,12 @@ func (r *attrFactory) MaybeAttribute(id int64, name string) Attribute { // RelativeAttribute refers to an expression and an optional qualifier path. func (r *attrFactory) RelativeAttribute(id int64, operand Interpretable) Attribute { return &relativeAttribute{ - id: id, - operand: operand, - qualifiers: []Qualifier{}, - adapter: r.adapter, - fac: r, + id: id, + operand: operand, + qualifiers: []Qualifier{}, + adapter: r.adapter, + fac: r, + optionalFieldSelectionNoneIfNull: r.optionalFieldSelectionNoneIfNull, } } @@ -226,6 +246,8 @@ type absoluteAttribute struct { adapter types.Adapter provider types.Provider fac AttributeFactory + + optionalFieldSelectionNoneIfNull bool } // ID implements the Attribute interface method. @@ -290,7 +312,7 @@ func (a *absoluteAttribute) Resolve(vars Activation) (any, error) { if celErr, ok := obj.(*types.Err); ok { return nil, celErr.Unwrap() } - obj, isOpt, err := applyQualifiers(vars, obj, a.qualifiers) + obj, isOpt, err := applyQualifiers(vars, obj, a.qualifiers, a.optionalFieldSelectionNoneIfNull) if err != nil { return nil, err } @@ -514,6 +536,8 @@ type relativeAttribute struct { qualifiers []Qualifier adapter types.Adapter fac AttributeFactory + + optionalFieldSelectionNoneIfNull bool } // ID is an implementation of the Attribute interface method. @@ -558,7 +582,7 @@ func (a *relativeAttribute) Resolve(vars Activation) (any, error) { if types.IsUnknown(v) { return v, nil } - obj, isOpt, err := applyQualifiers(vars, v, a.qualifiers) + obj, isOpt, err := applyQualifiers(vars, v, a.qualifiers, a.optionalFieldSelectionNoneIfNull) if err != nil { return nil, err } @@ -1160,7 +1184,7 @@ func (q *unknownQualifier) Value() ref.Val { return q.value } -func applyQualifiers(vars Activation, obj any, qualifiers []Qualifier) (any, bool, error) { +func applyQualifiers(vars Activation, obj any, qualifiers []Qualifier, noneIfNull bool) (any, bool, error) { optObj, isOpt := obj.(*types.Optional) if isOpt { if !optObj.HasValue() { @@ -1185,6 +1209,9 @@ func applyQualifiers(vars Activation, obj any, qualifiers []Qualifier) (any, boo // of the qualifiers is optional. return types.OptionalNone, false, nil } + if noneIfNull && qualObj == types.NullValue { + return types.OptionalNone, false, nil + } } else { qualObj, err = qual.Qualify(vars, obj) if err != nil { diff --git a/interpreter/interpreter_test.go b/interpreter/interpreter_test.go index 1ca97e54..8fdfd4f1 100644 --- a/interpreter/interpreter_test.go +++ b/interpreter/interpreter_test.go @@ -1455,6 +1455,20 @@ func testData(t testing.TB) []testCase { expr: `has(dyn([]).invalid)`, err: "unsupported index type 'string' in list", }, + + { + name: "optional_select_on_undefined", + expr: `{}.?invalid`, + + out: types.OptionalNone, + }, + { + name: "optional_select_on_null_literal", + expr: `{"invalid": dyn(null)}.?invalid.?nested`, + out: types.OptionalNone, + attrs: NewAttributeFactory(testContainer(""), types.DefaultTypeAdapter, types.NewEmptyRegistry(), + OptionalFieldSelectionNoneIfNull(true)), + }, } }