diff --git a/docs/provider_spec.md b/docs/provider_spec.md index b17b8f7..3342fc1 100644 --- a/docs/provider_spec.md +++ b/docs/provider_spec.md @@ -815,6 +815,21 @@ config: | `odata` | `$orderby=name desc` | | `prefix` | `sort=-name` | | `suffix` | `sort=name:desc` | +| `column_only` | `sort=name` | +| `direction_only` | `order=desc` | + +`direction_only` is for APIs that order by a fixed column and take only a +direction (e.g. `?order=desc`): it requires exactly one ORDER BY term whose +column is on an explicit `supportedColumns` allowlist. An ORDER BY the syntax +cannot express is not pushed and stays client-side, which remains +authoritative in every case. + +```yaml +orderBy: + paramName: order + syntax: direction_only + supportedColumns: ["created_at"] +``` --- diff --git a/internal/anysdk/query_param_pushdown.go b/internal/anysdk/query_param_pushdown.go index e8f7fc3..2fea07e 100644 --- a/internal/anysdk/query_param_pushdown.go +++ b/internal/anysdk/query_param_pushdown.go @@ -21,6 +21,12 @@ const ( DefaultCountParamName = "$count" DefaultCountParamValue = "true" DefaultCountResponseKey = "@odata.count" + // OrderBySyntax* name the non-OData ORDER BY renderings; see + // renderPushdownOrderBy. + OrderBySyntaxPrefix = "prefix" + OrderBySyntaxSuffix = "suffix" + OrderBySyntaxColumnOnly = "column_only" + OrderBySyntaxDirectionOnly = "direction_only" ) var ( diff --git a/internal/anysdk/query_param_pushdown_apply.go b/internal/anysdk/query_param_pushdown_apply.go index bdae2ad..025acb8 100644 --- a/internal/anysdk/query_param_pushdown_apply.go +++ b/internal/anysdk/query_param_pushdown_apply.go @@ -286,10 +286,6 @@ func applyPushdownOrderBy(qpp QueryParamPushdown, intent PushdownIntent, res *st if !ok { return } - // Only the OData syntax has a well-defined "col asc|desc" rendering here. - if !strings.EqualFold(ob.GetSyntax(), ODataDialect) { - return - } paramName := ob.GetParamName() if paramName == "" { return @@ -300,16 +296,60 @@ func applyPushdownOrderBy(qpp QueryParamPushdown, intent PushdownIntent, res *st return } } - parts := make([]string, 0, len(orderBy)) + value, ok := renderPushdownOrderBy(ob, orderBy) + if !ok { + return + } for _, o := range orderBy { - dir := "asc" + res.pushedOrderColumns = append(res.pushedOrderColumns, o.GetColumn()) + } + res.queryParams[paramName] = value +} + +// renderPushdownOrderBy renders the order terms in the configured syntax; false +// when the syntax is unknown or cannot express the terms, leaving ORDER BY +// client-side. +func renderPushdownOrderBy(ob OrderByPushdown, orderBy []PushdownOrder) (string, bool) { + syntax := strings.ToLower(ob.GetSyntax()) + switch syntax { + case OrderBySyntaxDirectionOnly: + // The API orders by a fixed column: exactly one term, on an explicit + // allowlist, rendered as its direction alone. + if len(orderBy) != 1 || len(ob.GetSupportedColumns()) == 0 { + return "", false + } + return pushdownOrderDirection(orderBy[0]), true + case ODataDialect, OrderBySyntaxPrefix, OrderBySyntaxSuffix, OrderBySyntaxColumnOnly: + parts := make([]string, 0, len(orderBy)) + for _, o := range orderBy { + parts = append(parts, renderPushdownOrderTerm(syntax, o)) + } + return strings.Join(parts, ","), true + } + return "", false +} + +func renderPushdownOrderTerm(syntax string, o PushdownOrder) string { + switch syntax { + case OrderBySyntaxPrefix: if o.IsDescending() { - dir = "desc" + return "-" + o.GetColumn() } - parts = append(parts, o.GetColumn()+" "+dir) - res.pushedOrderColumns = append(res.pushedOrderColumns, o.GetColumn()) + return o.GetColumn() + case OrderBySyntaxSuffix: + return o.GetColumn() + ":" + pushdownOrderDirection(o) + case OrderBySyntaxColumnOnly: + return o.GetColumn() + default: + return o.GetColumn() + " " + pushdownOrderDirection(o) + } +} + +func pushdownOrderDirection(o PushdownOrder) string { + if o.IsDescending() { + return "desc" } - res.queryParams[paramName] = strings.Join(parts, ",") + return "asc" } func applyPushdownTop(qpp QueryParamPushdown, intent PushdownIntent, res *standardPushdownResult) { diff --git a/internal/anysdk/query_param_pushdown_orderby_test.go b/internal/anysdk/query_param_pushdown_orderby_test.go new file mode 100644 index 0000000..dc97a34 --- /dev/null +++ b/internal/anysdk/query_param_pushdown_orderby_test.go @@ -0,0 +1,89 @@ +package anysdk + +import ( + "testing" +) + +func orderByTestIntent(orderBy ...PushdownOrder) PushdownIntent { + return NewPushdownIntent(nil, nil, orderBy, 0, false, 0, false, false) +} + +func orderByTestAssertNothingPushed(t *testing.T, res PushdownResult) { + t.Helper() + if len(res.QueryParams()) != 0 { + t.Fatalf("expected no query params, got %v", res.QueryParams()) + } +} + +const pushdownOrderByDirectionOnlyYaml = ` +orderBy: + paramName: order + syntax: direction_only + supportedColumns: + - created_at +` + +func TestApplyPushdown_OrderByDirectionOnly(t *testing.T) { + src := applyTestSource{qpp: applyTestBuildPushdown(t, pushdownOrderByDirectionOnlyYaml)} + rendered := map[string]struct { + order PushdownOrder + want string + }{ + "desc": {NewPushdownOrder("created_at", true), "desc"}, + "asc": {NewPushdownOrder("created_at", false), "asc"}, + } + for name, tc := range rendered { + t.Run(name, func(t *testing.T) { + res := ApplyPushdown(src, orderByTestIntent(tc.order)) + applyTestAssertParam(t, res.QueryParams(), "order", tc.want) + if len(res.QueryParams()) != 1 { + t.Fatalf("expected only the order param, got %v", res.QueryParams()) + } + }) + } + refused := map[string][]PushdownOrder{ + "unsupported column": {NewPushdownOrder("name", true)}, + "multiple terms": {NewPushdownOrder("created_at", true), NewPushdownOrder("name", false)}, + } + for name, orderBy := range refused { + t.Run(name, func(t *testing.T) { + orderByTestAssertNothingPushed(t, ApplyPushdown(src, orderByTestIntent(orderBy...))) + }) + } +} + +func TestApplyPushdown_OrderByDirectionOnlyRequiresAllowlist(t *testing.T) { + src := applyTestSource{qpp: applyTestBuildPushdown(t, ` +orderBy: + paramName: order + syntax: direction_only +`)} + orderByTestAssertNothingPushed(t, ApplyPushdown(src, orderByTestIntent(NewPushdownOrder("created_at", true)))) +} + +func TestApplyPushdown_OrderByCustomSyntaxes(t *testing.T) { + orderBy := []PushdownOrder{NewPushdownOrder("created_at", true), NewPushdownOrder("name", false)} + cases := map[string]string{ + OrderBySyntaxPrefix: "-created_at,name", + OrderBySyntaxSuffix: "created_at:desc,name:asc", + OrderBySyntaxColumnOnly: "created_at,name", + } + for syntax, want := range cases { + t.Run(syntax, func(t *testing.T) { + src := applyTestSource{qpp: applyTestBuildPushdown(t, "orderBy:\n paramName: sort\n syntax: "+syntax+"\n")} + res := ApplyPushdown(src, orderByTestIntent(orderBy...)) + applyTestAssertParam(t, res.QueryParams(), "sort", want) + }) + } +} + +func TestApplyPushdown_OrderByUnknownSyntaxEmitsNothing(t *testing.T) { + src := applyTestSource{qpp: applyTestBuildPushdown(t, "orderBy:\n paramName: sort\n syntax: bespoke\n")} + orderByTestAssertNothingPushed(t, ApplyPushdown(src, orderByTestIntent(NewPushdownOrder("created_at", true)))) +} + +func TestApplyPushdown_OrderByODataUnchanged(t *testing.T) { + src := applyTestSource{qpp: applyTestBuildPushdown(t, "orderBy:\n dialect: odata\n")} + res := ApplyPushdown(src, orderByTestIntent(NewPushdownOrder("created_at", true), NewPushdownOrder("name", false))) + applyTestAssertParam(t, res.QueryParams(), "$orderby", "created_at desc,name asc") +}