Skip to content

Commit

Permalink
Introduce a flag for supporting 'null' as absent during optional fiel…
Browse files Browse the repository at this point in the history
…d selection (#938)
  • Loading branch information
TristonianJones committed Apr 29, 2024
1 parent 03048d0 commit 315a5cc
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 22 deletions.
65 changes: 65 additions & 0 deletions cel/cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
4 changes: 4 additions & 0 deletions cel/library.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions cel/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions cel/program.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions interpreter/attribute_patterns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
59 changes: 43 additions & 16 deletions interpreter/attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -226,6 +246,8 @@ type absoluteAttribute struct {
adapter types.Adapter
provider types.Provider
fac AttributeFactory

optionalFieldSelectionNoneIfNull bool
}

// ID implements the Attribute interface method.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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() {
Expand All @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions interpreter/interpreter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
},
}
}

Expand Down

0 comments on commit 315a5cc

Please sign in to comment.