Skip to content

Commit

Permalink
fix: allow optional values in query params (#1694)
Browse files Browse the repository at this point in the history
Fixes #1686

Here's the test I used:

```go
type GetRequest struct {
	UserID string             `json:"userId"`
	PostID string             `json:"postId"`
	Tag    ftl.Option[string] `json:"tag"`
}

type GetResponse struct {
}

// Example:       curl -i http://localhost:8891/http/users/123/posts?postId=456&tag=foo
//
//ftl:ingress http GET /http/users/{userId}/posts
func Get(ctx context.Context, req builtin.HttpRequest[GetRequest]) (builtin.HttpResponse[GetResponse, string], error) {
	tag := req.Body.Tag.Default("No value")
	ftl.LoggerFromContext(ctx).Infof("Received request with tag: %s", tag)
	return builtin.HttpResponse[GetResponse, string]{
		Headers: map[string][]string{"Get": {"Header from FTL"}},
		Body:    ftl.Some(GetResponse{}),
	}, nil
}
```

```bash
curl -i http://localhost:8891/http/users/123/posts\?postId\=456\&tag\=foo

info:echo: Received request with tag: foo
```
  • Loading branch information
wesbillman authored Jun 7, 2024
1 parent 551eb48 commit 871271e
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 23 deletions.
10 changes: 10 additions & 0 deletions backend/controller/ingress/ingress_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand Down
4 changes: 2 additions & 2 deletions backend/controller/ingress/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
59 changes: 39 additions & 20 deletions backend/controller/ingress/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down
30 changes: 30 additions & 0 deletions backend/controller/ingress/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -53,6 +57,10 @@ func TestBuildRequestBody(t *testing.T) {
aliased String +alias json "alias"
}
data QueryParameterRequest {
foo String?
}
data PathParameterRequest {
username String
}
Expand All @@ -75,6 +83,9 @@ func TestBuildRequestBody(t *testing.T) {
export verb getPath(HttpRequest<test.PathParameterRequest>) HttpResponse<Empty, Empty>
+ingress http GET /getPath/{username}
export verb optionalQuery(HttpRequest<test.QueryParameterRequest>) HttpResponse<Empty, Empty>
+ingress http GET /optionalQuery
export verb postMissingTypes(HttpRequest<test.MissingTypes>) HttpResponse<Empty, Empty>
+ingress http POST /postMissingTypes
Expand Down Expand Up @@ -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",
Expand Down
11 changes: 11 additions & 0 deletions backend/controller/ingress/testdata/go/httpingress/httpingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion integration/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 871271e

Please sign in to comment.