Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: HttpRequest path and query mapping #2422

Merged
merged 1 commit into from
Aug 19, 2024
Merged

feat: HttpRequest path and query mapping #2422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions backend/controller/console/console_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ package console
import (
"testing"

"github.com/TBD54566975/ftl/backend/schema"
"github.com/alecthomas/assert/v2"

"github.com/TBD54566975/ftl/backend/schema"
)

func TestVerbSchemaString(t *testing.T) {
Expand All @@ -15,7 +16,7 @@ func TestVerbSchemaString(t *testing.T) {
}
ingressVerb := &schema.Verb{
Name: "Ingress",
Request: &schema.Ref{Module: "builtin", Name: "HttpRequest", TypeParameters: []schema.Type{&schema.String{}}},
Request: &schema.Ref{Module: "builtin", Name: "HttpRequest", TypeParameters: []schema.Type{&schema.String{}, &schema.Unit{}, &schema.Unit{}}},
Response: &schema.Ref{Module: "builtin", Name: "HttpResponse", TypeParameters: []schema.Type{&schema.String{}, &schema.String{}}},
Metadata: []schema.Metadata{
&schema.MetadataIngress{Type: "http", Method: "GET", Path: []schema.IngressPathComponent{&schema.IngressPathLiteral{Text: "test"}}},
Expand Down Expand Up @@ -107,7 +108,7 @@ verb Echo(foo.EchoRequest) foo.EchoResponse`
func TestVerbSchemaStringIngress(t *testing.T) {
verb := &schema.Verb{
Name: "Ingress",
Request: &schema.Ref{Module: "builtin", Name: "HttpRequest", TypeParameters: []schema.Type{&schema.Ref{Module: "foo", Name: "FooRequest"}}},
Request: &schema.Ref{Module: "builtin", Name: "HttpRequest", TypeParameters: []schema.Type{&schema.Ref{Module: "foo", Name: "FooRequest"}, &schema.Unit{}, &schema.Unit{}}},
Response: &schema.Ref{Module: "builtin", Name: "HttpResponse", TypeParameters: []schema.Type{&schema.Ref{Module: "foo", Name: "FooResponse"}, &schema.String{}}},
Metadata: []schema.Metadata{
&schema.MetadataIngress{Type: "http", Method: "GET", Path: []schema.IngressPathComponent{&schema.IngressPathLiteral{Text: "foo"}}},
Expand Down Expand Up @@ -135,11 +136,11 @@ func TestVerbSchemaStringIngress(t *testing.T) {
}

expected := `// HTTP request structure used for HTTP ingress verbs.
export data HttpRequest<Body> {
export data HttpRequest<Body, Path, Query> {
method String
path String
pathParameters {String: String}
query {String: [String]}
pathParameters Path
query Query
headers {String: [String]}
body Body
}
Expand All @@ -161,7 +162,7 @@ data FooResponse {
Message String
}

verb Ingress(builtin.HttpRequest<foo.FooRequest>) builtin.HttpResponse<foo.FooResponse, String>
verb Ingress(builtin.HttpRequest<foo.FooRequest, Unit, Unit>) builtin.HttpResponse<foo.FooResponse, String>
+ingress http GET /foo`

schemaString, err := verbSchemaString(sch, verb)
Expand Down
4 changes: 2 additions & 2 deletions backend/controller/console/testdata/go/console/console.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ type Response struct {
}

//ftl:ingress http GET /test
func Get(ctx context.Context, req builtin.HttpRequest[External]) (builtin.HttpResponse[Response, string], error) {
func Get(ctx context.Context, req builtin.HttpRequest[ftl.Unit, ftl.Unit, External]) (builtin.HttpResponse[Response, string], error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe later we should code-gen some type aliases to eradicate some of the boilerplate, it's pretty verbose.

Something like

type HttpRequestBody[T any] HttpRequest[Unit, Unit, T]

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same thought, we just need to figure out which ones we want to provide and what to name them.

return builtin.HttpResponse[Response, string]{
Body: ftl.Some(Response{
Message: fmt.Sprintf("Hello, %s", req.Body.Message),
Message: fmt.Sprintf("Hello, %s", req.Query.Message),
}),
}, nil
}
Expand Down
8 changes: 4 additions & 4 deletions backend/controller/ingress/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,16 @@ func TestIngress(t *testing.T) {
foo String
}

export verb getAlias(HttpRequest<test.AliasRequest>) HttpResponse<Empty, Empty>
export verb getAlias(HttpRequest<Unit, Unit, test.AliasRequest>) HttpResponse<Empty, Empty>
+ingress http GET /getAlias

export verb getPath(HttpRequest<test.PathParameterRequest>) HttpResponse<Empty, Empty>
export verb getPath(HttpRequest<Unit, test.PathParameterRequest, Unit>) HttpResponse<Empty, Empty>
+ingress http GET /getPath/{username}

export verb postMissingTypes(HttpRequest<test.MissingTypes>) HttpResponse<Empty, Empty>
export verb postMissingTypes(HttpRequest<test.MissingTypes, Unit, Unit>) HttpResponse<Empty, Empty>
+ingress http POST /postMissingTypes

export verb postJsonPayload(HttpRequest<test.JsonPayload>) HttpResponse<Empty, Empty>
export verb postJsonPayload(HttpRequest<test.JsonPayload, Unit, Unit>) HttpResponse<Empty, Empty>
+ingress http POST /postJsonPayload
}
`)
Expand Down
6 changes: 3 additions & 3 deletions backend/controller/ingress/ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,21 +67,21 @@ func ValidateCallBody(body []byte, verb *schema.Verb, sch *schema.Schema) error
return nil
}

func getBodyField(ref *schema.Ref, sch *schema.Schema) (*schema.Field, error) {
func getField(name string, ref *schema.Ref, sch *schema.Schema) (*schema.Field, error) {
data, err := sch.ResolveMonomorphised(ref)
if err != nil {
return nil, err
}
var bodyField *schema.Field
for _, field := range data.Fields {
if field.Name == "body" {
if field.Name == name {
bodyField = field
break
}
}

if bodyField == nil {
return nil, fmt.Errorf("verb %s must have a 'body' field", ref.Name)
return nil, fmt.Errorf("verb %s must have a %q field", ref.Name, name)
}

return bodyField, nil
Expand Down
18 changes: 9 additions & 9 deletions backend/controller/ingress/ingress_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestHttpIngress(t *testing.T) {
in.Run(t,
in.CopyModule("httpingress"),
in.Deploy("httpingress"),
in.HttpCall(http.MethodGet, "/users/123/posts/456", nil, in.JsonData(t, in.Obj{}), func(t testing.TB, resp *in.HTTPResponse) {
in.HttpCall(http.MethodGet, "/users/123/posts/456", nil, nil, func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"Header from FTL"}, resp.Headers["Get"])
assert.Equal(t, []string{"application/json; charset=utf-8"}, resp.Headers["Content-Type"])
Expand Down Expand Up @@ -82,23 +82,23 @@ func TestHttpIngress(t *testing.T) {
assert.Equal(t, nil, resp.BodyBytes)
}),

in.HttpCall(http.MethodGet, "/string", nil, []byte("Hello, World!"), func(t testing.TB, resp *in.HTTPResponse) {
in.HttpCall(http.MethodPost, "/string", nil, []byte("Hello, World!"), func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"text/plain; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, []byte("Hello, World!"), resp.BodyBytes)
}),

in.HttpCall(http.MethodGet, "/int", nil, []byte("1234"), func(t testing.TB, resp *in.HTTPResponse) {
in.HttpCall(http.MethodPost, "/int", nil, []byte("1234"), func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"text/plain; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, []byte("1234"), resp.BodyBytes)
}),
in.HttpCall(http.MethodGet, "/float", nil, []byte("1234.56789"), func(t testing.TB, resp *in.HTTPResponse) {
in.HttpCall(http.MethodPost, "/float", nil, []byte("1234.56789"), func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"text/plain; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, []byte("1234.56789"), resp.BodyBytes)
}),
in.HttpCall(http.MethodGet, "/bool", nil, []byte("true"), func(t testing.TB, resp *in.HTTPResponse) {
in.HttpCall(http.MethodPost, "/bool", nil, []byte("true"), func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"text/plain; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, []byte("true"), resp.BodyBytes)
Expand All @@ -108,7 +108,7 @@ func TestHttpIngress(t *testing.T) {
assert.Equal(t, []string{"text/plain; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, []byte("Error from FTL"), resp.BodyBytes)
}),
in.HttpCall(http.MethodGet, "/array/string", nil, in.JsonData(t, []string{"hello", "world"}), func(t testing.TB, resp *in.HTTPResponse) {
in.HttpCall(http.MethodPost, "/array/string", nil, in.JsonData(t, []string{"hello", "world"}), func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"application/json; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, in.JsonData(t, []string{"hello", "world"}), resp.BodyBytes)
Expand All @@ -118,7 +118,7 @@ func TestHttpIngress(t *testing.T) {
assert.Equal(t, []string{"application/json; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, in.JsonData(t, []in.Obj{{"item": "a"}, {"item": "b"}}), resp.BodyBytes)
}),
in.HttpCall(http.MethodGet, "/typeenum", nil, in.JsonData(t, in.Obj{"name": "A", "value": "hello"}), func(t testing.TB, resp *in.HTTPResponse) {
in.HttpCall(http.MethodPost, "/typeenum", nil, in.JsonData(t, in.Obj{"name": "A", "value": "hello"}), func(t testing.TB, resp *in.HTTPResponse) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the method changes?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah because of the request body in the test.

assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"application/json; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, in.JsonData(t, in.Obj{"name": "A", "value": "hello"}), resp.BodyBytes)
Expand All @@ -134,12 +134,12 @@ func TestHttpIngress(t *testing.T) {
assert.Equal(t, nil, resp.Headers["Access-Control-Allow-Methods"])
assert.Equal(t, nil, resp.Headers["Access-Control-Allow-Headers"])
}),
in.HttpCall(http.MethodGet, "/external", nil, in.JsonData(t, in.Obj{"message": "hello"}), func(t testing.TB, resp *in.HTTPResponse) {
in.HttpCall(http.MethodPost, "/external", nil, in.JsonData(t, in.Obj{"message": "hello"}), func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"application/json; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, in.JsonData(t, in.Obj{"message": "hello"}), resp.BodyBytes)
}),
in.HttpCall(http.MethodGet, "/external2", nil, in.JsonData(t, in.Obj{"message": "hello"}), func(t testing.TB, resp *in.HTTPResponse) {
in.HttpCall(http.MethodPost, "/external2", nil, in.JsonData(t, in.Obj{"message": "hello"}), func(t testing.TB, resp *in.HTTPResponse) {
assert.Equal(t, 200, resp.Status)
assert.Equal(t, []string{"application/json; charset=utf-8"}, resp.Headers["Content-Type"])
assert.Equal(t, in.JsonData(t, in.Obj{"Message": "hello"}), resp.BodyBytes)
Expand Down
128 changes: 89 additions & 39 deletions backend/controller/ingress/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,16 @@ func BuildRequestBody(route *dal.IngressRoute, r *http.Request, sch *schema.Sche
var requestMap map[string]any

if metadata, ok := verb.GetMetadataIngress().Get(); ok && metadata.Type == "http" {
pathParameters := map[string]any{}
pathParametersMap := map[string]string{}
matchSegments(route.Path, r.URL.Path, func(segment, value string) {
pathParameters[segment] = value
pathParametersMap[segment] = value
})
pathParameters, err := manglePathParameters(pathParametersMap, request, sch)
if err != nil {
return nil, err
}

httpRequestBody, err := extractHTTPRequestBody(route, r, request, sch)
httpRequestBody, err := extractHTTPRequestBody(r, request, sch)
if err != nil {
return nil, err
}
Expand All @@ -56,6 +60,10 @@ func BuildRequestBody(route *dal.IngressRoute, r *http.Request, sch *schema.Sche
queryMap[key] = valuesAny
}

finalQueryParams, err := mangleQueryParameters(queryMap, r.URL.Query(), request, sch)
if err != nil {
return nil, err
}
headerMap := make(map[string]any)
for key, values := range r.Header {
valuesAny := make([]any, len(values))
Expand All @@ -69,15 +77,11 @@ func BuildRequestBody(route *dal.IngressRoute, r *http.Request, sch *schema.Sche
requestMap["method"] = r.Method
requestMap["path"] = r.URL.Path
requestMap["pathParameters"] = pathParameters
requestMap["query"] = queryMap
requestMap["query"] = finalQueryParams
requestMap["headers"] = headerMap
requestMap["body"] = httpRequestBody
} else {
var err error
requestMap, err = buildRequestMap(route, r, request, sch)
if err != nil {
return nil, err
}
return nil, fmt.Errorf("no HTTP ingress metadata for verb %s", verb.Name)
}

requestMap, err = schema.TransformFromAliasedFields(request, sch, requestMap)
Expand All @@ -102,15 +106,15 @@ func BuildRequestBody(route *dal.IngressRoute, r *http.Request, sch *schema.Sche
return body, nil
}

func extractHTTPRequestBody(route *dal.IngressRoute, r *http.Request, ref *schema.Ref, sch *schema.Schema) (any, error) {
bodyField, err := getBodyField(ref, sch)
func extractHTTPRequestBody(r *http.Request, ref *schema.Ref, sch *schema.Schema) (any, error) {
bodyField, err := getField("body", ref, sch)
if err != nil {
return nil, err
}

if ref, ok := bodyField.Type.(*schema.Ref); ok {
if err := sch.ResolveToType(ref, &schema.Data{}); err == nil {
return buildRequestMap(route, r, ref, sch)
return buildRequestMap(r)
}
}

Expand All @@ -122,6 +126,76 @@ func extractHTTPRequestBody(route *dal.IngressRoute, r *http.Request, ref *schem
return valueForData(bodyField.Type, bodyData)
}

// Takes the map of path parameters and transforms them into the appropriate type
func manglePathParameters(params map[string]string, ref *schema.Ref, sch *schema.Schema) (any, error) {

paramsField, err := getField("pathParameters", ref, sch)
if err != nil {
return nil, err
}

switch paramsField.Type.(type) {
case *schema.Ref, *schema.Map:
ret := map[string]any{}
for k, v := range params {
ret[k] = v
}
return ret, nil
default:
}
// This is a scalar, there should only be a single param
// This is validated by the schema, we don't need extra validation here
for _, val := range params {

switch paramsField.Type.(type) {
case *schema.String:
return val, nil
case *schema.Int:
parsed, err := strconv.ParseInt(val, 10, 64)
if err != nil {
return nil, fmt.Errorf("failed to parse int from path parameter: %w", err)
}
return parsed, nil
case *schema.Float:
float, err := strconv.ParseFloat(val, 64)
if err != nil {
return nil, fmt.Errorf("failed to parse float from path parameter: %w", err)
}
return float, nil
case *schema.Bool:
// TODO: is anything else considered truthy?
return val == "true", nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use an LLM here to decide?

default:
return nil, fmt.Errorf("unsupported path parameter type %T", paramsField.Type)
}
}
// Empty map
return map[string]any{}, nil
}

// Takes the map of path parameters and transforms them into the appropriate type
func mangleQueryParameters(params map[string]any, underlying map[string][]string, ref *schema.Ref, sch *schema.Schema) (any, error) {

paramsField, err := getField("query", ref, sch)
if err != nil {
return nil, err
}

if m, ok := paramsField.Type.(*schema.Map); ok {
if _, ok := m.Value.(*schema.Array); ok {
return params, nil
}
}
// We need to turn them into straight strings
newParams := map[string]any{}
for k, v := range underlying {
if len(v) > 0 {
newParams[k] = v[0]
stuartwdouglas marked this conversation as resolved.
Show resolved Hide resolved
}
}
return newParams, nil
}

func valueForData(typ schema.Type, data []byte) (any, error) {
switch typ.(type) {
case *schema.Ref:
Expand Down Expand Up @@ -203,12 +277,7 @@ func readRequestBody(r *http.Request) ([]byte, error) {
return bodyData, nil
}

func buildRequestMap(route *dal.IngressRoute, r *http.Request, ref *schema.Ref, sch *schema.Schema) (map[string]any, error) {
requestMap := map[string]any{}
matchSegments(route.Path, r.URL.Path, func(segment, value string) {
requestMap[segment] = value
})

func buildRequestMap(r *http.Request) (map[string]any, error) {
switch r.Method {
case http.MethodPost, http.MethodPut:
var bodyMap map[string]any
Expand All @@ -217,29 +286,10 @@ func buildRequestMap(route *dal.IngressRoute, r *http.Request, ref *schema.Ref,
return nil, fmt.Errorf("HTTP request body is not valid JSON: %w", err)
}

// Merge bodyMap into params
for k, v := range bodyMap {
requestMap[k] = v
}
return bodyMap, nil
default:
symbol, err := sch.ResolveRequestResponseType(ref)
if err != nil {
return nil, err
}

if data, ok := symbol.(*schema.Data); ok {
queryMap, err := parseQueryParams(r.URL.Query(), data)
if err != nil {
return nil, fmt.Errorf("HTTP query params are not valid: %w", err)
}

for key, value := range queryMap {
requestMap[key] = value
}
}
return nil, nil
}

return requestMap, nil
}

func parseQueryParams(values url.Values, data *schema.Data) (map[string]any, error) {
Expand Down
Loading
Loading