Skip to content

Commit

Permalink
Remove registry from spec
Browse files Browse the repository at this point in the history 
This eliminates the global registry object in favor of a (possibly
user-created) registry instance. Users need this ability in order to add
custom functions without changing the behavior of JSONPath expressions
globally.

Replace `spec.Function` with an interface that registry.Function (added
in 701cf60) adheres to. Refactor `NewFunctionExpr` to simply create a
`FunctionExpr` with that interface and performs no validation.

Instead, teach the parser use a registry.Registry to look up functions
and validate arguments. This requires that the parser have a registry
instance, so add a `parser` struct that stores both a registry and a
lexer, and cease passing the lexer to every parsing function.

While at it, add the `Validator` and `Evaluator` interfaces to the
registry package and teach the `Register` method (now an instance
method) to return errors rather than panic. Also pass `Function` fields
to it rather than a `Function` instance, as it's a cleaner API. Then
make the `Function` fields private and add accessor methods to comply
with the `spec.Function` interface.

Other bits and bobs:

*   Rename `execute` to `evaluate` to be more consistent.
*   Remove the default function definitions from the spec package, as
    they now live in the registry package.
*   Add a mock function to properly test function expressions.
*   Add overlooked test case for a nil slice step.
  • Loading branch information
@theory
theory committed Sep 16, 2024
1 parent 701cf60 commit ef87c24
Show file tree
Hide file tree
Showing 12 changed files with 452 additions and 1,253 deletions.
148 changes: 80 additions & 68 deletions parser/parse.go
View file

Large diffs are not rendered by default.

102 changes: 56 additions & 46 deletions parser/parse_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,10 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/theory/jsonpath/registry"
"github.com/theory/jsonpath/spec"
)

func TestMain(m *testing.M) {
// Set up a function that returns an unknown return type.
spec.Register(&spec.Function{
Name: "__true",
ResultType: spec.FuncLogical,
Validate: func([]spec.FunctionExprArg) error { return nil },
Evaluate: func([]spec.JSONPathValue) spec.JSONPathValue {
return spec.LogicalTrue
},
})

m.Run()
}

func TestParseRoot(t *testing.T) {
t.Parallel()
a := assert.New(t)
Expand Down Expand Up @@ -225,19 +212,20 @@ func TestParseSimple(t *testing.T) {
}
}

func newFuncExpr(t *testing.T, name string, args []spec.FunctionExprArg) *spec.FunctionExpr {
t.Helper()
f, err := spec.NewFunctionExpr(name, args)
if err != nil {
t.Fatal(err.Error())
}
return f
}

func TestParseFilter(t *testing.T) {
t.Parallel()
a := assert.New(t)
r := require.New(t)
reg := registry.New()
_ = reg.Register(
"__true",
spec.FuncLogical,
func([]spec.FunctionExprArg) error { return nil },
func([]spec.JSONPathValue) spec.JSONPathValue {
return spec.LogicalTrue
},
)
trueFunc := reg.Get("__true")

for _, tc := range []struct {
name string
Expand Down Expand Up @@ -462,7 +450,7 @@ func TestParseFilter(t *testing.T) {
name: "function_current",
query: "__true(@)",
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
newFuncExpr(t, "__true", []spec.FunctionExprArg{
spec.NewFunctionExpr(trueFunc, []spec.FunctionExprArg{
spec.SingularQuery(false, []spec.Selector{}),
}),
}}),
Expand All @@ -471,7 +459,7 @@ func TestParseFilter(t *testing.T) {
name: "function_match_current_integer",
query: "match( @, 42 )",
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
newFuncExpr(t, "match", []spec.FunctionExprArg{
spec.NewFunctionExpr(reg.Get("match"), []spec.FunctionExprArg{
spec.SingularQuery(false, []spec.Selector{}),
spec.Literal(int64(42)),
}),
Expand All @@ -481,7 +469,7 @@ func TestParseFilter(t *testing.T) {
name: "function_search_two_queries",
query: "search( $.x, @[0] )",
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
newFuncExpr(t, "search", []spec.FunctionExprArg{
spec.NewFunctionExpr(reg.Get("search"), []spec.FunctionExprArg{
spec.SingularQuery(true, []spec.Selector{spec.Name("x")}),
spec.SingularQuery(false, []spec.Selector{spec.Index(0)}),
}),
Expand All @@ -492,7 +480,10 @@ func TestParseFilter(t *testing.T) {
query: `length("hi") == 2`,
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
spec.Comparison(
newFuncExpr(t, "length", []spec.FunctionExprArg{spec.Literal("hi")}),
spec.NewFunctionExpr(
reg.Get("length"),
[]spec.FunctionExprArg{spec.Literal("hi")},
),
spec.EqualTo,
spec.Literal(int64(2)),
),
Expand All @@ -503,7 +494,10 @@ func TestParseFilter(t *testing.T) {
query: `length(true) == 1`,
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
spec.Comparison(
newFuncExpr(t, "length", []spec.FunctionExprArg{spec.Literal(true)}),
spec.NewFunctionExpr(
reg.Get("length"),
[]spec.FunctionExprArg{spec.Literal(true)},
),
spec.EqualTo,
spec.Literal(int64(1)),
),
Expand All @@ -514,7 +508,10 @@ func TestParseFilter(t *testing.T) {
query: `length(false)==1`,
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
spec.Comparison(
newFuncExpr(t, "length", []spec.FunctionExprArg{spec.Literal(false)}),
spec.NewFunctionExpr(
reg.Get("length"),
[]spec.FunctionExprArg{spec.Literal(false)},
),
spec.EqualTo,
spec.Literal(int64(1)),
),
Expand All @@ -524,15 +521,17 @@ func TestParseFilter(t *testing.T) {
name: "function_value_null",
query: `__true(null)`, // defined in function_test.go
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
newFuncExpr(t, "__true", []spec.FunctionExprArg{spec.Literal(nil)}),
spec.NewFunctionExpr(trueFunc, []spec.FunctionExprArg{
spec.Literal(nil),
}),
}}),
},
{
name: "nested_function",
query: `__true(count(@))`, // defined in function_test.go
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
newFuncExpr(t, "__true", []spec.FunctionExprArg{
newFuncExpr(t, "count", []spec.FunctionExprArg{
spec.NewFunctionExpr(trueFunc, []spec.FunctionExprArg{
spec.NewFunctionExpr(reg.Get("count"), []spec.FunctionExprArg{
spec.SingularQuery(false, []spec.Selector{}),
}),
}),
Expand All @@ -542,7 +541,7 @@ func TestParseFilter(t *testing.T) {
name: "function_paren_logical_expr",
query: `__true((@.x))`, // defined in function_test.go
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
newFuncExpr(t, "__true", []spec.FunctionExprArg{
spec.NewFunctionExpr(trueFunc, []spec.FunctionExprArg{
spec.LogicalOr{spec.LogicalAnd{
spec.Existence(spec.Query(false, []*spec.Segment{
spec.Child(spec.Name("x")),
Expand All @@ -555,7 +554,7 @@ func TestParseFilter(t *testing.T) {
name: "function_paren_logical_not_expr",
query: `__true((!@.x))`, // defined in function_test.go
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
newFuncExpr(t, "__true", []spec.FunctionExprArg{
spec.NewFunctionExpr(trueFunc, []spec.FunctionExprArg{
spec.LogicalOr{spec.LogicalAnd{
spec.Nonexistence(spec.Query(false, []*spec.Segment{
spec.Child(spec.Name("x")),
Expand All @@ -568,7 +567,7 @@ func TestParseFilter(t *testing.T) {
name: "function_lots_of_literals",
query: `__true("hi", 42, true, false, null, 98.6)`,
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
newFuncExpr(t, "__true", []spec.FunctionExprArg{
spec.NewFunctionExpr(trueFunc, []spec.FunctionExprArg{
spec.Literal("hi"),
spec.Literal(int64(42)),
spec.Literal(true),
Expand All @@ -582,7 +581,7 @@ func TestParseFilter(t *testing.T) {
name: "function_no_args",
query: `__true()`,
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
newFuncExpr(t, "__true", []spec.FunctionExprArg{}),
spec.NewFunctionExpr(trueFunc, []spec.FunctionExprArg{}),
}}),
},
// ComparisonExpr
Expand Down Expand Up @@ -611,7 +610,9 @@ func TestParseFilter(t *testing.T) {
spec.Comparison(
spec.Literal(int64(42)),
spec.GreaterThan,
newFuncExpr(t, "length", []spec.FunctionExprArg{spec.Literal("hi")}),
spec.NewFunctionExpr(reg.Get("length"), []spec.FunctionExprArg{
spec.Literal("hi"),
}),
),
}}),
},
Expand All @@ -620,7 +621,10 @@ func TestParseFilter(t *testing.T) {
query: `length("hi") <= @[0]["a"]`,
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
spec.Comparison(
newFuncExpr(t, "length", []spec.FunctionExprArg{spec.Literal("hi")}),
spec.NewFunctionExpr(
reg.Get("length"),
[]spec.FunctionExprArg{spec.Literal("hi")},
),
spec.LessThanEqualTo,
spec.SingularQuery(false, []spec.Selector{spec.Index(0), spec.Name("a")}),
),
Expand All @@ -642,7 +646,10 @@ func TestParseFilter(t *testing.T) {
query: `length("hi") < 42 `,
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
spec.Comparison(
newFuncExpr(t, "length", []spec.FunctionExprArg{spec.Literal("hi")}),
spec.NewFunctionExpr(
reg.Get("length"),
[]spec.FunctionExprArg{spec.Literal("hi")},
),
spec.LessThan,
spec.Literal(int64(42)),
),
Expand All @@ -652,7 +659,9 @@ func TestParseFilter(t *testing.T) {
name: "not_function",
query: `!__true()`,
filter: spec.Filter(spec.LogicalOr{spec.LogicalAnd{
spec.NotFuncExpr{FunctionExpr: newFuncExpr(t, "__true", []spec.FunctionExprArg{})},
spec.NotFuncExpr{FunctionExpr: spec.NewFunctionExpr(
trueFunc, []spec.FunctionExprArg{},
)},
}}),
},
{
Expand Down Expand Up @@ -829,14 +838,15 @@ func TestParseFilter(t *testing.T) {
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
filter, err := parseFilter(newLexer(tc.query))
parser := &parser{lex: newLexer(tc.query), reg: reg}
filter, err := parser.parseFilter()
if tc.err == "" {
r.NoError(err)
a.Equal(tc.filter, filter)
r.NoError(err, tc.name)
a.Equal(tc.filter, filter, tc.name)
} else {
a.Nil(filter)
r.EqualError(err, tc.err)
r.ErrorIs(err, ErrPathParse)
a.Nil(filter, tc.name)
r.EqualError(err, tc.err, tc.name)
r.ErrorIs(err, ErrPathParse, tc.name)
}
})
}
Expand Down
11 changes: 5 additions & 6 deletions registry/funcs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ func TestLengthFunc(t *testing.T) {
func TestCheckSingularFuncArgs(t *testing.T) {
t.Parallel()
r := require.New(t)
reg := New()

for _, tc := range []struct {
name string
Expand Down Expand Up @@ -158,8 +159,7 @@ func TestCheckSingularFuncArgs(t *testing.T) {
},
{
name: "logical_function_expr",
expr: []spec.FunctionExprArg{newFuncExpr(
t, "match",
expr: []spec.FunctionExprArg{spec.NewFunctionExpr(reg.Get("match"),
[]spec.FunctionExprArg{
spec.FilterQuery(
spec.Query(true, []*spec.Segment{spec.Child(spec.Name("x"))}),
Expand Down Expand Up @@ -220,6 +220,7 @@ func TestCheckSingularFuncArgs(t *testing.T) {
func TestCheckRegexFuncArgs(t *testing.T) {
t.Parallel()
r := require.New(t)
reg := New()

for _, tc := range []struct {
name string
Expand Down Expand Up @@ -276,8 +277,7 @@ func TestCheckRegexFuncArgs(t *testing.T) {
{
name: "function_expr_1",
expr: []spec.FunctionExprArg{
newFuncExpr(
t, "match",
spec.NewFunctionExpr(reg.Get("match"),
[]spec.FunctionExprArg{
spec.FilterQuery(
spec.Query(true, []*spec.Segment{spec.Child(spec.Name("x"))}),
Expand All @@ -293,8 +293,7 @@ func TestCheckRegexFuncArgs(t *testing.T) {
name: "function_expr_2",
expr: []spec.FunctionExprArg{
spec.Literal("hi"),
newFuncExpr(
t, "match",
spec.NewFunctionExpr(reg.Get("match"),
[]spec.FunctionExprArg{
spec.FilterQuery(
spec.Query(true, []*spec.Segment{spec.Child(spec.Name("x"))}),
Expand Down
Loading

0 comments on commit ef87c24

Please sign in to comment.