Skip to content

Commit

Permalink
[ottl/pkg] Remove the ottlarg struct tag for the arguments and rely o…
Browse files Browse the repository at this point in the history
…n field ordering (#25705)

Remove the ottlarg struct tag and rely on field ordering
**Link to tracking Issue:**
#24874
  • Loading branch information
rnishtala-sumo authored Sep 13, 2023
1 parent 8eea663 commit faf9f20
Show file tree
Hide file tree
Showing 33 changed files with 57 additions and 202 deletions.
10 changes: 1 addition & 9 deletions pkg/ottl/expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,15 +280,7 @@ func (g StandardFunctionGetter[K]) Get(args Arguments) (Expr[K], error) {
}
for i := 0; i < fArgsVal.NumField(); i++ {
field := argsVal.Field(i)
argIndex, err := getArgumentIndex(i, argsVal)
if err != nil {
return Expr[K]{}, err
}
fArgIndex, err := getArgumentIndex(argIndex, fArgsVal)
if err != nil {
return Expr[K]{}, err
}
fArgsVal.Field(fArgIndex).Set(field)
fArgsVal.Field(i).Set(field)
}
fn, err := g.fact.CreateFunction(g.fCtx, fArgs)
if err != nil {
Expand Down
17 changes: 0 additions & 17 deletions pkg/ottl/expression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -690,11 +690,6 @@ func Test_FunctionGetter(t *testing.T) {
&multipleArgsArguments{},
functionWithStringGetter,
),
createFactory[any](
"no_struct_tag",
&noStructTagFunctionArguments{},
functionWithStringGetter,
),
NewFactory(
"cannot_create_function",
&stringGetterArguments{},
Expand Down Expand Up @@ -753,18 +748,6 @@ func Test_FunctionGetter(t *testing.T) {
valid: false,
expectedErrorMsg: "incorrect number of arguments. Expected: 4 Received: 1",
},
{
name: "Invalid Arguments struct tag",
getter: StandardStringGetter[interface{}]{
Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) {
return nil, nil
},
},
function: StandardFunctionGetter[any]{fCtx: FunctionContext{Set: componenttest.NewNopTelemetrySettings()}, fact: functions["no_struct_tag"]},
want: "anything",
valid: false,
expectedErrorMsg: "no `ottlarg` struct tag on Arguments field \"StringArg\"",
},
{
name: "Cannot create function",
getter: StandardStringGetter[interface{}]{
Expand Down
24 changes: 2 additions & 22 deletions pkg/ottl/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"errors"
"fmt"
"reflect"
"strconv"
"strings"
)

Expand Down Expand Up @@ -47,22 +46,6 @@ func (p *Parser[K]) newFunctionCall(ed editor) (Expr[K], error) {
return Expr[K]{exprFunc: fn}, err
}

func getArgumentIndex(index int, args reflect.Value) (int, error) {
argsType := args.Type()
fieldTag, ok := argsType.Field(index).Tag.Lookup("ottlarg")
if !ok {
return 0, fmt.Errorf("no `ottlarg` struct tag on Arguments field %q", argsType.Field(index).Name)
}
argNum, err := strconv.Atoi(fieldTag)
if err != nil {
return 0, fmt.Errorf("ottlarg struct tag on field %q is not a valid integer: %w", argsType.Field(index).Name, err)
}
if argNum < 0 || argNum >= args.NumField() {
return 0, fmt.Errorf("ottlarg struct tag on field %q has value %d, but must be between 0 and %d", argsType.Field(index).Name, argNum, args.NumField())
}
return argNum, nil
}

func (p *Parser[K]) buildArgs(ed editor, argsVal reflect.Value) error {
if len(ed.Arguments) != argsVal.NumField() {
return fmt.Errorf("incorrect number of arguments. Expected: %d Received: %d", argsVal.NumField(), len(ed.Arguments))
Expand All @@ -71,12 +54,9 @@ func (p *Parser[K]) buildArgs(ed editor, argsVal reflect.Value) error {
for i := 0; i < argsVal.NumField(); i++ {
field := argsVal.Field(i)
fieldType := field.Type()
argNum, err := getArgumentIndex(i, argsVal)
if err != nil {
return err
}
argVal := ed.Arguments[argNum]
argVal := ed.Arguments[i]
var val any
var err error
switch {
case strings.HasPrefix(fieldType.Name(), "FunctionGetter"):
var name string
Expand Down
100 changes: 0 additions & 100 deletions pkg/ottl/functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,31 +63,6 @@ func Test_NewFunctionCall_invalid(t *testing.T) {
errorFunctionArguments{},
functionThatHasAnError,
),
createFactory(
"no_struct_tag",
&noStructTagFunctionArguments{},
functionThatHasAnError,
),
createFactory(
"wrong_struct_tag",
&wrongTagFunctionArguments{},
functionThatHasAnError,
),
createFactory(
"bad_struct_tag",
&badStructTagFunctionArguments{},
functionThatHasAnError,
),
createFactory(
"negative_struct_tag",
&negativeStructTagFunctionArguments{},
functionThatHasAnError,
),
createFactory(
"out_of_bounds_struct_tag",
&outOfBoundsStructTagFunctionArguments{},
functionThatHasAnError,
),
createFactory(
"testing_unknown_function",
&functionGetterArguments{},
Expand Down Expand Up @@ -349,61 +324,6 @@ func Test_NewFunctionCall_invalid(t *testing.T) {
Function: "non_pointer",
},
},
{
name: "no struct tags",
inv: editor{
Function: "no_struct_tag",
Arguments: []value{
{
String: ottltest.Strp("str"),
},
},
},
},
{
name: "using the wrong struct tag",
inv: editor{
Function: "wrong_struct_tag",
Arguments: []value{
{
String: ottltest.Strp("str"),
},
},
},
},
{
name: "non-integer struct tags",
inv: editor{
Function: "bad_struct_tag",
Arguments: []value{
{
String: ottltest.Strp("str"),
},
},
},
},
{
name: "struct tag index too low",
inv: editor{
Function: "negative_struct_tag",
Arguments: []value{
{
String: ottltest.Strp("str"),
},
},
},
},
{
name: "struct tag index too high",
inv: editor{
Function: "out_of_bounds_struct_tag",
Arguments: []value{
{
String: ottltest.Strp("str"),
},
},
},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -1612,26 +1532,6 @@ func functionWithEnum(Enum) (ExprFunc[interface{}], error) {
}, nil
}

type noStructTagFunctionArguments struct {
StringArg string
}

type badStructTagFunctionArguments struct {
StringArg string `ottlarg:"a"`
}

type negativeStructTagFunctionArguments struct {
StringArg string `ottlarg:"-1"`
}

type outOfBoundsStructTagFunctionArguments struct {
StringArg string `ottlarg:"1"`
}

type wrongTagFunctionArguments struct {
StringArg string `argument:"1"`
}

func createFactory[A any](name string, args A, fn any) Factory[any] {
createFunction := func(fCtx FunctionContext, oArgs Arguments) (ExprFunc[any], error) {
fArgs, ok := oArgs.(A)
Expand Down
4 changes: 2 additions & 2 deletions pkg/ottl/ottlfuncs/func_concat.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
)

type ConcatArguments[K any] struct {
Vals []ottl.StringLikeGetter[K] `ottlarg:"0"`
Delimiter string `ottlarg:"1"`
Vals []ottl.StringLikeGetter[K]
Delimiter string
}

func NewConcatFactory[K any]() ottl.Factory[K] {
Expand Down
4 changes: 2 additions & 2 deletions pkg/ottl/ottlfuncs/func_convert_case.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import (
)

type ConvertCaseArguments[K any] struct {
Target ottl.StringGetter[K] `ottlarg:"0"`
ToCase string `ottlarg:"1"`
Target ottl.StringGetter[K]
ToCase string
}

func NewConvertCaseFactory[K any]() ottl.Factory[K] {
Expand Down
4 changes: 2 additions & 2 deletions pkg/ottl/ottlfuncs/func_delete_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (
)

type DeleteKeyArguments[K any] struct {
Target ottl.PMapGetter[K] `ottlarg:"0"`
Key string `ottlarg:"1"`
Target ottl.PMapGetter[K]
Key string
}

func NewDeleteKeyFactory[K any]() ottl.Factory[K] {
Expand Down
4 changes: 2 additions & 2 deletions pkg/ottl/ottlfuncs/func_delete_matching_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import (
)

type DeleteMatchingKeysArguments[K any] struct {
Target ottl.PMapGetter[K] `ottlarg:"0"`
Pattern string `ottlarg:"1"`
Target ottl.PMapGetter[K]
Pattern string
}

func NewDeleteMatchingKeysFactory[K any]() ottl.Factory[K] {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_duration.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

type DurationArguments[K any] struct {
Duration ottl.StringGetter[K] `ottlarg:"0"`
Duration ottl.StringGetter[K]
}

func NewDurationFactory[K any]() ottl.Factory[K] {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_fnv.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

type FnvArguments[K any] struct {
Target ottl.StringGetter[K] `ottlarg:"0"`
Target ottl.StringGetter[K]
}

func NewFnvFactory[K any]() ottl.Factory[K] {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_int.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type IntArguments[K any] struct {
Target ottl.IntLikeGetter[K] `ottlarg:"0"`
Target ottl.IntLikeGetter[K]
}

func NewIntFactory[K any]() ottl.Factory[K] {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_is_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type IsMapArguments[K any] struct {
Target ottl.PMapGetter[K] `ottlarg:"0"`
Target ottl.PMapGetter[K]
}

func NewIsMapFactory[K any]() ottl.Factory[K] {
Expand Down
4 changes: 2 additions & 2 deletions pkg/ottl/ottlfuncs/func_is_match.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
)

type IsMatchArguments[K any] struct {
Target ottl.StringLikeGetter[K] `ottlarg:"0"`
Pattern string `ottlarg:"1"`
Target ottl.StringLikeGetter[K]
Pattern string
}

func NewIsMatchFactory[K any]() ottl.Factory[K] {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_is_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type IsStringArguments[K any] struct {
Target ottl.StringGetter[K] `ottlarg:"0"`
Target ottl.StringGetter[K]
}

func NewIsStringFactory[K any]() ottl.Factory[K] {
Expand Down
4 changes: 2 additions & 2 deletions pkg/ottl/ottlfuncs/func_keep_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
)

type KeepKeysArguments[K any] struct {
Target ottl.PMapGetter[K] `ottlarg:"0"`
Keys []string `ottlarg:"1"`
Target ottl.PMapGetter[K]
Keys []string
}

func NewKeepKeysFactory[K any]() ottl.Factory[K] {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_len.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const (
)

type LenArguments[K any] struct {
Target ottl.Getter[K] `ottlarg:"0"`
Target ottl.Getter[K]
}

func NewLenFactory[K any]() ottl.Factory[K] {
Expand Down
6 changes: 3 additions & 3 deletions pkg/ottl/ottlfuncs/func_limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import (
)

type LimitArguments[K any] struct {
Target ottl.PMapGetter[K] `ottlarg:"0"`
Limit int64 `ottlarg:"1"`
PriorityKeys []string `ottlarg:"2"`
Target ottl.PMapGetter[K]
Limit int64
PriorityKeys []string
}

func NewLimitFactory[K any]() ottl.Factory[K] {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

type LogArguments[K any] struct {
Target ottl.FloatLikeGetter[K] `ottlarg:"0"`
Target ottl.FloatLikeGetter[K]
}

func NewLogFactory[K any]() ottl.Factory[K] {
Expand Down
6 changes: 3 additions & 3 deletions pkg/ottl/ottlfuncs/func_merge_maps.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ const (
)

type MergeMapsArguments[K any] struct {
Target ottl.PMapGetter[K] `ottlarg:"0"`
Source ottl.PMapGetter[K] `ottlarg:"1"`
Strategy string `ottlarg:"2"`
Target ottl.PMapGetter[K]
Source ottl.PMapGetter[K]
Strategy string
}

func NewMergeMapsFactory[K any]() ottl.Factory[K] {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ottl/ottlfuncs/func_parse_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

type ParseJSONArguments[K any] struct {
Target ottl.StringGetter[K] `ottlarg:"0"`
Target ottl.StringGetter[K]
}

func NewParseJSONFactory[K any]() ottl.Factory[K] {
Expand Down
6 changes: 3 additions & 3 deletions pkg/ottl/ottlfuncs/func_replace_all_matches.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ import (
)

type ReplaceAllMatchesArguments[K any] struct {
Target ottl.PMapGetter[K] `ottlarg:"0"`
Pattern string `ottlarg:"1"`
Replacement ottl.StringGetter[K] `ottlarg:"2"`
Target ottl.PMapGetter[K]
Pattern string
Replacement ottl.StringGetter[K]
}

func NewReplaceAllMatchesFactory[K any]() ottl.Factory[K] {
Expand Down
Loading

0 comments on commit faf9f20

Please sign in to comment.