Skip to content

Commit

Permalink
feat: extract type by node + add schema fuzz test (#2138) (#2154)
Browse files Browse the repository at this point in the history
user requested support for `type Foo = other.Bar` pattern when declaring
type aliases. this allows them to leverage member functions of the
underlying type.

after this change, the above pattern as well as the prior pattern, `type
Foo other.Bar` will be supported for type aliasing.

also fixes #2145
  • Loading branch information
worstell authored Jul 24, 2024
1 parent 4a34226 commit 304510c
Show file tree
Hide file tree
Showing 22 changed files with 1,166 additions and 375 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ examples/**/go.work
examples/**/go.work.sum
testdata/**/go.work
testdata/**/go.work.sum
go-runtime/schema/testdata/test/test.go

# Leaving old _ftl for now to avoid old stuff getting checked in
**/testdata/**/_ftl
Expand Down
5 changes: 5 additions & 0 deletions backend/controller/ingress/ingress_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ 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{"message": "hello"}), resp.BodyBytes)
}),
in.HttpCall(http.MethodGet, "/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
15 changes: 12 additions & 3 deletions backend/controller/ingress/testdata/go/httpingress/httpingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,18 @@ func TypeEnum(ctx context.Context, req builtin.HttpRequest[SumType]) (builtin.Ht
return builtin.HttpResponse[SumType, string]{Body: ftl.Some(req.Body)}, nil
}

type ExternalAlias lib.NonFTLType
// tests both supported patterns for aliasing an external type

type NewTypeAlias lib.NonFTLType

//ftl:ingress http GET /external
func External(ctx context.Context, req builtin.HttpRequest[ExternalAlias]) (builtin.HttpResponse[ExternalAlias, string], error) {
return builtin.HttpResponse[ExternalAlias, string]{Body: ftl.Some(req.Body)}, nil
func External(ctx context.Context, req builtin.HttpRequest[NewTypeAlias]) (builtin.HttpResponse[NewTypeAlias, string], error) {
return builtin.HttpResponse[NewTypeAlias, string]{Body: ftl.Some(req.Body)}, nil
}

type DirectTypeAlias = lib.NonFTLType

//ftl:ingress http GET /external2
func External2(ctx context.Context, req builtin.HttpRequest[DirectTypeAlias]) (builtin.HttpResponse[DirectTypeAlias, string], error) {
return builtin.HttpResponse[DirectTypeAlias, string]{Body: ftl.Some(req.Body)}, nil
}
65 changes: 51 additions & 14 deletions backend/schema/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,12 @@ func ValidateModule(module *Module) error {
})

merr = cleanErrors(merr)
SortModuleDecls(module)
return errors.Join(merr...)
}

// SortModuleDecls sorts the declarations in a module.
func SortModuleDecls(module *Module) {
sort.SliceStable(module.Decls, func(i, j int) bool {
iDecl := module.Decls[i]
jDecl := module.Decls[j]
Expand All @@ -382,7 +388,6 @@ func ValidateModule(module *Module) error {
}
return iPriority < jPriority
})
return errors.Join(merr...)
}

// getDeclSortingPriority (used for schema sorting) is pulled out into it's own switch so the Go sumtype check will fail
Expand Down Expand Up @@ -603,23 +608,16 @@ func validateVerbMetadata(scopes Scopes, module *Module, n *Verb) (merr []error)
}

func validateIngressRequestOrResponse(scopes Scopes, module *Module, n *Verb, reqOrResp string, r Type) (fieldType Type, body Symbol, merr []error) {
rref, _ := r.(*Ref)
resp, sym := ResolveTypeAs[*Data](scopes, r)
invalid := sym == nil
if !invalid {
m, _ := sym.Module.Get()
invalid = m == nil || m.Name != "builtin" || resp.Name != "Http"+strings.Title(reqOrResp)
}
if invalid {
merr = append(merr, errorf(r, "ingress verb %s: %s type %s must be builtin.HttpRequest", n.Name, reqOrResp, r))
return
}

resp, err := resp.Monomorphise(rref) //nolint:govet
data, err := resolveValidIngressReqResp(scopes, reqOrResp, optional.None[*ModuleDecl](), r, nil)
if err != nil {
merr = append(merr, errorf(r, "ingress verb %s: %s type %s: %v", n.Name, reqOrResp, r, err))
return
}
resp, ok := data.Get()
if !ok {
merr = append(merr, errorf(r, "ingress verb %s: %s type %s must be builtin.HttpRequest", n.Name, reqOrResp, r))
return
}

scopes = scopes.PushScope(resp.Scope())
fieldType = resp.FieldByName("body").Type
Expand All @@ -646,6 +644,45 @@ func validateIngressRequestOrResponse(scopes Scopes, module *Module, n *Verb, re
return
}

func resolveValidIngressReqResp(scopes Scopes, reqOrResp string, moduleDecl optional.Option[*ModuleDecl], n Node, parent Node) (optional.Option[*Data], error) {
switch t := n.(type) {
case *Ref:
m := scopes.Resolve(*t)
sym := m.Symbol
return resolveValidIngressReqResp(scopes, reqOrResp, optional.Some(m), sym, n)
case *Data:
md, ok := moduleDecl.Get()
if !ok {
return optional.None[*Data](), nil
}

m, ok := md.Module.Get()
if !ok {
return optional.None[*Data](), nil
}

if parent == nil || m.Name != "builtin" || t.Name != "Http"+strings.Title(reqOrResp) {
return optional.None[*Data](), nil
}

ref, ok := parent.(*Ref)
if !ok {
return optional.None[*Data](), nil
}

result, err := t.Monomorphise(ref)
if err != nil {
return optional.None[*Data](), err
}

return optional.Some(result), nil
case *TypeAlias:
return resolveValidIngressReqResp(scopes, reqOrResp, moduleDecl, t.Type, t)
default:
return optional.None[*Data](), nil
}
}

func validatePayloadType(n Node, r Type, v *Verb, reqOrResp string) error {
switch t := n.(type) {
case *Bytes, *String, *Data, *Unit, *Float, *Int, *Bool, *Map, *Array: // Valid HTTP response payload types.
Expand Down
2 changes: 1 addition & 1 deletion buildengine/build_go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ func TestExternalType(t *testing.T) {
}
testBuild(t, bctx, "", "unsupported external type", []assertion{
assertBuildProtoErrors(
`unsupported external type "time.Month"; see FTL docs on using external types: tbd54566975.github.io/ftl/docs/reference/externaltypes/`,
`unsupported type "time.Month" for field "Month"`,
`unsupported external type "time.Month"; see FTL docs on using external types: tbd54566975.github.io/ftl/docs/reference/externaltypes/`,
`unsupported response type "ftl/external.ExternalResponse"`,
),
})
Expand Down
103 changes: 56 additions & 47 deletions go-runtime/compile/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,57 +193,66 @@ func TestExtractModuleSchemaTwo(t *testing.T) {
}
actual := schema.Normalise(r.Module)
expected := `module two {
typealias ExternalAlias Any
+typemap kotlin "com.foo.bar.NonFTLType"
+typemap go "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"
typealias TransitiveAlias Any
+typemap go "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"
export enum TwoEnum: String {
Blue = "Blue"
Green = "Green"
Red = "Red"
}
typealias ExplicitAliasAlias Any
+typemap kotlin "com.foo.bar.NonFTLType"
+typemap go "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"
export enum TypeEnum {
Exported two.Exported
List [String]
Scalar String
WithoutDirective two.WithoutDirective
}
typealias ExplicitAliasType Any
+typemap kotlin "com.foo.bar.NonFTLType"
+typemap go "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"
export data Exported {
}
typealias TransitiveAliasAlias Any
+typemap go "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"
data NonFtlField {
field two.ExternalAlias
transitive two.TransitiveAlias
}
typealias TransitiveAliasType Any
+typemap go "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"
export data Payload<T> {
body T
}
export enum TwoEnum: String {
Blue = "Blue"
Green = "Green"
Red = "Red"
}
export data User {
name String
}
export enum TypeEnum {
Exported two.Exported
List [String]
Scalar String
WithoutDirective two.WithoutDirective
}
export data UserResponse {
user two.User
}
export data Exported {
}
export data WithoutDirective {
}
data NonFtlField {
explicitType two.ExplicitAliasType
explicitAlias two.ExplicitAliasAlias
transitiveType two.TransitiveAliasType
transitiveAlias two.TransitiveAliasAlias
}
export verb callsTwo(two.Payload<String>) two.Payload<String>
+calls two.two
export data Payload<T> {
body T
}
export verb returnsUser(Unit) two.UserResponse
export data User {
name String
}
export verb two(two.Payload<String>) two.Payload<String>
}
`
export data UserResponse {
user two.User
}
export data WithoutDirective {
}
export verb callsTwo(two.Payload<String>) two.Payload<String>
+calls two.two
export verb returnsUser(Unit) two.UserResponse
export verb two(two.Payload<String>) two.Payload<String>
}
`
assert.Equal(t, normaliseString(expected), normaliseString(actual.String()))
}

Expand Down Expand Up @@ -536,7 +545,7 @@ func TestErrorReporting(t *testing.T) {
`25:2-10: unsupported type "error" for field "BadParam"`,
`28:2-17: unsupported type "uint64" for field "AnotherBadParam"`,
`31:3-3: unexpected directive "ftl:export" attached for verb, did you mean to use '//ftl:verb export' instead?`,
`37:36-36: unsupported request type "ftl/failing.Request"`,
`37:40-40: unsupported request type "ftl/failing.Request"`,
`37:50-50: unsupported response type "ftl/failing.Response"`,
`38:16-29: call first argument must be a function but is an unresolved reference to lib.OtherFunc`,
`38:16-29: call first argument must be a function in an ftl module`,
Expand All @@ -551,12 +560,12 @@ func TestErrorReporting(t *testing.T) {
`60:1-2: first parameter must be context.Context`,
`60:18-18: unsupported response type "ftl/failing.Response"`,
`65:1-2: must have at most two results (<type>, error)`,
`65:41-41: unsupported request type "ftl/failing.Request"`,
`65:45-45: unsupported request type "ftl/failing.Request"`,
`70:1-2: must at least return an error`,
`70:36-36: unsupported request type "ftl/failing.Request"`,
`74:35-35: unsupported request type "ftl/failing.Request"`,
`70:40-40: unsupported request type "ftl/failing.Request"`,
`74:39-39: unsupported request type "ftl/failing.Request"`,
`74:48-48: must return an error but is ftl/failing.Response`,
`79:41-41: unsupported request type "ftl/failing.Request"`,
`79:45-45: unsupported request type "ftl/failing.Request"`,
`79:63-63: must return an error but is string`,
`79:63-63: second result must not be ftl.Unit`,
// `86:1-2: duplicate declaration of "WrongResponse" at 79:6`, TODO: fix this
Expand All @@ -580,8 +589,8 @@ func TestErrorReporting(t *testing.T) {
// failing/child/child.go
expectedChild := []string{
`9:2-6: unsupported type "uint64" for field "Body"`,
`14:2-2: unsupported external type "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"; see FTL docs on using external types: tbd54566975.github.io/ftl/docs/reference/externaltypes/`,
`14:2-7: unsupported type "github.com/TBD54566975/ftl/go-runtime/compile/testdata.NonFTLType" for field "Field"`,
`14:8-8: unsupported external type "github.com/TBD54566975/ftl/go-runtime/compile/testdata.NonFTLType"; see FTL docs on using external types: tbd54566975.github.io/ftl/docs/reference/externaltypes/`,
`19:6-41: declared type github.com/blah.lib.NonFTLType in typemap does not match native type github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType`,
`24:6-6: multiple Go type mappings found for "ftl/failing/child.MultipleMappings"`,
`34:2-13: enum variant "SameVariant" conflicts with existing enum variant of "EnumVariantConflictParent" at "196:2"`,
Expand Down
16 changes: 12 additions & 4 deletions go-runtime/compile/testdata/go/two/two.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,20 @@ func ReturnsUser(ctx context.Context) (UserResponse, error) {

//ftl:data
type NonFTLField struct {
Field ExternalAlias
Transitive TransitiveAlias
ExplicitType ExplicitAliasType
ExplicitAlias ExplicitAliasAlias
TransitiveType TransitiveAliasType
TransitiveAlias TransitiveAliasAlias
}

//ftl:typealias
//ftl:typemap kotlin "com.foo.bar.NonFTLType"
type ExternalAlias lib.NonFTLType
type ExplicitAliasType lib.NonFTLType

type TransitiveAlias lib.NonFTLType
//ftl:typealias
//ftl:typemap kotlin "com.foo.bar.NonFTLType"
type ExplicitAliasAlias = lib.NonFTLType

type TransitiveAliasType lib.NonFTLType

type TransitiveAliasAlias = lib.NonFTLType
Loading

0 comments on commit 304510c

Please sign in to comment.