diff --git a/backend/controller/ingress/ingress_integration_test.go b/backend/controller/ingress/ingress_integration_test.go index 66ce94b6f9..2db1cd0d47 100644 --- a/backend/controller/ingress/ingress_integration_test.go +++ b/backend/controller/ingress/ingress_integration_test.go @@ -53,6 +53,16 @@ func TestHttpIngress(t *testing.T) { assert.Equal(t, map[string]any{}, resp.JsonBody) }), + in.HttpCall(http.MethodGet, "/queryparams?foo=bar", in.JsonData(t, in.Obj{}), func(t testing.TB, resp *in.HTTPResponse) { + assert.Equal(t, 200, resp.Status) + assert.Equal(t, "bar", string(resp.BodyBytes)) + }), + + in.HttpCall(http.MethodGet, "/queryparams", in.JsonData(t, in.Obj{}), func(t testing.TB, resp *in.HTTPResponse) { + assert.Equal(t, 200, resp.Status) + assert.Equal(t, "No value", string(resp.BodyBytes)) + }), + in.HttpCall(http.MethodGet, "/html", in.JsonData(t, in.Obj{}), func(t testing.TB, resp *in.HTTPResponse) { assert.Equal(t, 200, resp.Status) assert.Equal(t, []string{"text/html; charset=utf-8"}, resp.Headers["Content-Type"]) diff --git a/backend/controller/ingress/ingress_test.go b/backend/controller/ingress/ingress_test.go index 34cef42233..1d502f2645 100644 --- a/backend/controller/ingress/ingress_test.go +++ b/backend/controller/ingress/ingress_test.go @@ -176,9 +176,9 @@ func TestParseQueryParams(t *testing.T) { {query: "array=2", request: obj{"array": []string{"2"}}}, {query: "array=10&array=11", request: obj{"array": []string{"10", "11"}}}, {query: "int=10&array=11&array=12", request: obj{"int": "10", "array": []string{"11", "12"}}}, - {query: "int=1&int=2", request: nil, err: "multiple values for \"int\" are not supported"}, + {query: "int=1&int=2", request: nil, err: `failed to parse query parameter "int": multiple values are not supported`}, {query: "[a,b]=c", request: nil, err: "complex key \"[a,b]\" is not supported, use '@json=' instead"}, - {query: "array=[1,2]", request: nil, err: "complex value \"[1,2]\" is not supported, use '@json=' instead"}, + {query: "array=[1,2]", request: nil, err: `failed to parse query parameter "array": complex value "[1,2]" is not supported, use '@json=' instead`}, } for _, test := range tests { diff --git a/backend/controller/ingress/request.go b/backend/controller/ingress/request.go index 6cee114bb1..4c8ed01e38 100644 --- a/backend/controller/ingress/request.go +++ b/backend/controller/ingress/request.go @@ -12,6 +12,7 @@ import ( "github.com/TBD54566975/ftl/backend/controller/dal" "github.com/TBD54566975/ftl/backend/schema" + "github.com/alecthomas/types/optional" ) // BuildRequestBody extracts the HttpRequest body from an HTTP request. @@ -311,33 +312,51 @@ func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, err continue } - switch field.Type.(type) { - case *schema.Bytes, *schema.Map, *schema.Optional, *schema.Time, - *schema.Unit, *schema.Ref, *schema.Any: + val, err := valueForField(field.Type, value) + if err != nil { + return nil, fmt.Errorf("failed to parse query parameter %q: %w", key, err) + } - case *schema.Int, *schema.Float, *schema.String, *schema.Bool: - if len(value) > 1 { - return nil, fmt.Errorf("multiple values for %q are not supported", key) - } - if hasInvalidQueryChars(value[0]) { - return nil, fmt.Errorf("complex value %q is not supported, use '@json=' instead", value[0]) - } - queryMap[key] = value[0] + if v, ok := val.Get(); ok { + queryMap[key] = v + } + } - case *schema.Array: - for _, v := range value { - if hasInvalidQueryChars(v) { - return nil, fmt.Errorf("complex value %q is not supported, use '@json=' instead", v) - } + return queryMap, nil +} + +func valueForField(typ schema.Type, value []string) (optional.Option[any], error) { + switch t := typ.(type) { + case *schema.Bytes, *schema.Map, *schema.Time, + *schema.Unit, *schema.Ref, *schema.Any: + + case *schema.Int, *schema.Float, *schema.String, *schema.Bool: + if len(value) > 1 { + return optional.None[any](), fmt.Errorf("multiple values are not supported") + } + if hasInvalidQueryChars(value[0]) { + return optional.None[any](), fmt.Errorf("complex value %q is not supported, use '@json=' instead", value[0]) + } + return optional.Some[any](value[0]), nil + + case *schema.Array: + for _, v := range value { + if hasInvalidQueryChars(v) { + return optional.None[any](), fmt.Errorf("complex value %q is not supported, use '@json=' instead", v) } - queryMap[key] = value + } + return optional.Some[any](value), nil - default: - panic(fmt.Sprintf("unsupported type %T for query parameter field %q", field.Type, key)) + case *schema.Optional: + if len(value) > 0 { + return valueForField(t.Type, value) } + + default: + panic(fmt.Sprintf("unsupported type %T", typ)) } - return queryMap, nil + return optional.Some[any](value), nil } func decodeQueryJSON(query string) (map[string]any, error) { diff --git a/backend/controller/ingress/request_test.go b/backend/controller/ingress/request_test.go index fab0664702..dec12964ef 100644 --- a/backend/controller/ingress/request_test.go +++ b/backend/controller/ingress/request_test.go @@ -24,6 +24,10 @@ type PathParameterRequest struct { Username string } +type QueryParameterRequest struct { + Foo ftl.Option[string] `json:"foo,omitempty"` +} + type MissingTypes struct { Optional ftl.Option[string] `json:"optional,omitempty"` Array []string `json:"array,omitempty"` @@ -53,6 +57,10 @@ func TestBuildRequestBody(t *testing.T) { aliased String +alias json "alias" } + data QueryParameterRequest { + foo String? + } + data PathParameterRequest { username String } @@ -75,6 +83,9 @@ func TestBuildRequestBody(t *testing.T) { export verb getPath(HttpRequest) HttpResponse +ingress http GET /getPath/{username} + export verb optionalQuery(HttpRequest) HttpResponse + +ingress http GET /optionalQuery + export verb postMissingTypes(HttpRequest) HttpResponse +ingress http POST /postMissingTypes @@ -142,6 +153,25 @@ func TestBuildRequestBody(t *testing.T) { Body: PostJSONPayload{Foo: "bar"}, }, }, + {name: "OptionalQueryParameter", + verb: "optionalQuery", + method: "GET", + path: "/optionalQuery", + routePath: "/optionalQuery", + query: map[string][]string{ + "foo": {"bar"}, + }, + expected: HTTPRequest[QueryParameterRequest]{ + Method: "GET", + Path: "/optionalQuery", + Query: map[string][]string{ + "foo": {"bar"}, + }, + Body: QueryParameterRequest{ + Foo: ftl.Some("bar"), + }, + }, + }, {name: "PathParameterDecoding", verb: "getPath", method: "GET", diff --git a/backend/controller/ingress/testdata/go/httpingress/httpingress.go b/backend/controller/ingress/testdata/go/httpingress/httpingress.go index 754abb273c..8085ddcdc2 100644 --- a/backend/controller/ingress/testdata/go/httpingress/httpingress.go +++ b/backend/controller/ingress/testdata/go/httpingress/httpingress.go @@ -97,6 +97,17 @@ func Delete(ctx context.Context, req builtin.HttpRequest[DeleteRequest]) (builti }, nil } +type QueryParamRequest struct { + Foo ftl.Option[string] `json:"foo"` +} + +//ftl:ingress http GET /queryparams +func Query(ctx context.Context, req builtin.HttpRequest[QueryParamRequest]) (builtin.HttpResponse[string, string], error) { + return builtin.HttpResponse[string, string]{ + Body: ftl.Some(req.Body.Foo.Default("No value")), + }, nil +} + type HtmlRequest struct{} //ftl:ingress http GET /html diff --git a/integration/actions.go b/integration/actions.go index d080a2d232..407a7e2f98 100644 --- a/integration/actions.go +++ b/integration/actions.go @@ -373,7 +373,9 @@ func HttpCall(method string, path string, body []byte, onResponse func(t testing baseURL, err := url.Parse(fmt.Sprintf("http://localhost:8891")) assert.NoError(t, err) - r, err := http.NewRequestWithContext(ic, method, baseURL.JoinPath(path).String(), bytes.NewReader(body)) + u, err := baseURL.Parse(path) + assert.NoError(t, err) + r, err := http.NewRequestWithContext(ic, method, u.String(), bytes.NewReader(body)) assert.NoError(t, err) r.Header.Add("Content-Type", "application/json")