Skip to content

Commit

Permalink
feat: migrate configs and secrets to new extractor (#2058)
Browse files Browse the repository at this point in the history
  • Loading branch information
@worstell
worstell authored Jul 12, 2024
1 parent 43dd554 commit d737241
Show file tree
Hide file tree
Showing 9 changed files with 310 additions and 180 deletions.
154 changes: 0 additions & 154 deletions go-runtime/compile/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,13 @@ import (
var (
fset = token.NewFileSet()

ftlPkgPath = "github.com/TBD54566975/ftl/go-runtime/ftl"
ftlCallFuncPath = "github.com/TBD54566975/ftl/go-runtime/ftl.Call"
ftlFSMFuncPath = "github.com/TBD54566975/ftl/go-runtime/ftl.FSM"
ftlTransitionFuncPath = "github.com/TBD54566975/ftl/go-runtime/ftl.Transition"
ftlStartFuncPath = "github.com/TBD54566975/ftl/go-runtime/ftl.Start"
ftlConfigFuncPath = "github.com/TBD54566975/ftl/go-runtime/ftl.Config"
ftlSecretFuncPath = "github.com/TBD54566975/ftl/go-runtime/ftl.Secret" //nolint:gosec
ftlPostgresDBFuncPath = "github.com/TBD54566975/ftl/go-runtime/ftl.PostgresDatabase"
ftlTopicFuncPath = "github.com/TBD54566975/ftl/go-runtime/ftl.Topic"
ftlSubscriptionFuncPath = "github.com/TBD54566975/ftl/go-runtime/ftl.Subscription"
ftlTopicHandleTypeName = "TopicHandle"
aliasFieldTag = "json"
)

Expand Down Expand Up @@ -65,15 +61,6 @@ func errorf(node ast.Node, format string, args ...interface{}) *schema.Error {
return schema.Errorf(pos, endCol, format, args...)
}

func tokenWrapf(pos token.Pos, tokenText string, err error, format string, args ...interface{}) *schema.Error {
goPos := goPosToSchemaPos(pos)
endColumn := goPos.Column
if len(tokenText) > 0 {
endColumn += utf8.RuneCountInString(tokenText)
}
return schema.Wrapf(goPos, endColumn, err, format, args...)
}

//nolint:unparam
func wrapf(node ast.Node, err error, format string, args ...interface{}) *schema.Error {
pos, endCol := goNodePosToSchemaPos(node)
Expand Down Expand Up @@ -225,8 +212,6 @@ func extractTopicDecl(pctx *parseContext, node *ast.CallExpr, stack []ast.Node)
}

func visitCallExpr(pctx *parseContext, node *ast.CallExpr, stack []ast.Node) {
validateCallExpr(pctx, node)

_, fn := deref[*types.Func](pctx.pkg, node.Fun)
if fn == nil {
return
Expand All @@ -235,10 +220,6 @@ func visitCallExpr(pctx *parseContext, node *ast.CallExpr, stack []ast.Node) {
case ftlCallFuncPath:
parseCall(pctx, node, stack)

case ftlConfigFuncPath, ftlSecretFuncPath:
// Secret/config declaration: ftl.Config[<type>](<name>)
parseConfigDecl(pctx, node, fn)

case ftlFSMFuncPath:
parseFSMDecl(pctx, node, stack)

Expand All @@ -253,56 +234,6 @@ func visitCallExpr(pctx *parseContext, node *ast.CallExpr, stack []ast.Node) {
}
}

// validateCallExpr validates all function calls
// checks if the function call is:
// - a direct verb call to an external module
// - a direct publish call on an external module's topic
func validateCallExpr(pctx *parseContext, node *ast.CallExpr) {
selExpr, ok := node.Fun.(*ast.SelectorExpr)
if !ok {
return
}
var lhsIdent *ast.Ident
if expr, ok := selExpr.X.(*ast.SelectorExpr); ok {
lhsIdent = expr.Sel
} else if ident, ok := selExpr.X.(*ast.Ident); ok {
lhsIdent = ident
} else {
return
}
lhsObject := pctx.pkg.TypesInfo.ObjectOf(lhsIdent)
if lhsObject == nil {
return
}
var lhsPkgPath string
if pkgName, ok := lhsObject.(*types.PkgName); ok {
lhsPkgPath = pkgName.Imported().Path()
} else {
lhsPkgPath = lhsObject.Pkg().Path()
}
var lhsIsExternal bool
if !pctx.isPathInPkg(lhsPkgPath) {
lhsIsExternal = true
}

if lhsType, ok := pctx.pkg.TypesInfo.TypeOf(selExpr.X).(*types.Named); ok && lhsType.Obj().Pkg() != nil && lhsType.Obj().Pkg().Path() == ftlPkgPath {
// Calling a function on an FTL type
if lhsType.Obj().Name() == ftlTopicHandleTypeName && selExpr.Sel.Name == "Publish" {
if lhsIsExternal {
pctx.errors.add(errorf(node, "can not publish directly to topics in other modules"))
}
}
return
}

if lhsIsExternal && strings.HasPrefix(lhsPkgPath, "ftl/") {
if sig, ok := pctx.pkg.TypesInfo.TypeOf(selExpr.Sel).(*types.Signature); ok && sig.Recv() == nil {
// can not call functions in external modules directly
pctx.errors.add(errorf(node, "can not call verbs in other modules directly: use ftl.Call(…) instead"))
}
}
}

func parseCall(pctx *parseContext, node *ast.CallExpr, stack []ast.Node) {
var activeFuncDecl *ast.FuncDecl
for i := len(stack) - 1; i >= 0; i-- {
Expand Down Expand Up @@ -465,65 +396,6 @@ func parseFSMTransition(pctx *parseContext, node *ast.CallExpr, fn *types.Func,
}
}

func parseConfigDecl(pctx *parseContext, node *ast.CallExpr, fn *types.Func) {
name, schemaErr := extractStringLiteralArg(node, 0)
if schemaErr != nil {
pctx.errors.add(schemaErr)
return
}
if !schema.ValidateName(name) {
pctx.errors.add(errorf(node, "config and secret names must be valid identifiers"))
}

index := node.Fun.(*ast.IndexExpr) //nolint:forcetypeassert

// Type parameter
tp := pctx.pkg.TypesInfo.Types[index.Index].Type
st, ok := visitType(pctx, index.Index.Pos(), tp, false).Get()
if !ok {
pctx.errors.add(errorf(index.Index, "unsupported type %q", tp))
return
}

// Add the declaration to the module.
var decl schema.Decl
if fn.FullName() == ftlConfigFuncPath {
decl = &schema.Config{
Pos: goPosToSchemaPos(node.Pos()),
Name: name,
Type: st,
}
} else {
decl = &schema.Secret{
Pos: goPosToSchemaPos(node.Pos()),
Name: name,
Type: st,
}
}

// Check for duplicates
_, endCol := goNodePosToSchemaPos(node)
for _, d := range pctx.module.Decls {
switch fn.FullName() {
case ftlConfigFuncPath:
c, ok := d.(*schema.Config)
if ok && c.Name == name && c.Type.String() == st.String() {
pctx.errors.add(errorf(node, "duplicate config declaration at %d:%d-%d", c.Pos.Line, c.Pos.Column, endCol))
return
}
case ftlSecretFuncPath:
s, ok := d.(*schema.Secret)
if ok && s.Name == name && s.Type.String() == st.String() {
pctx.errors.add(errorf(node, "duplicate secret declaration at %d:%d-%d", s.Pos.Line, s.Pos.Column, endCol))
return
}
default:
}
}

pctx.module.Decls = append(pctx.module.Decls, decl)
}

func parseDatabaseDecl(pctx *parseContext, node *ast.CallExpr, dbType string) {
name, schemaErr := extractStringLiteralArg(node, 0)
if schemaErr != nil {
Expand Down Expand Up @@ -832,32 +704,6 @@ func visitStruct(pctx *parseContext, pos token.Pos, tnode types.Type, isExported
return optional.Some[*schema.Ref](dataRef)
}

func visitConst(pctx *parseContext, cnode *types.Const) optional.Option[schema.Value] {
if b, ok := cnode.Type().Underlying().(*types.Basic); ok {
switch b.Kind() {
case types.String:
value, err := strconv.Unquote(cnode.Val().String())
if err != nil {
pctx.errors.add(tokenWrapf(cnode.Pos(), cnode.Val().String(), err, "unsupported string constant"))
return optional.None[schema.Value]()
}
return optional.Some[schema.Value](&schema.StringValue{Pos: goPosToSchemaPos(cnode.Pos()), Value: value})

case types.Int:
value, err := strconv.ParseInt(cnode.Val().String(), 10, 64)
if err != nil {
pctx.errors.add(tokenWrapf(cnode.Pos(), cnode.Val().String(), err, "unsupported int constant"))
return optional.None[schema.Value]()
}
return optional.Some[schema.Value](&schema.IntValue{Pos: goPosToSchemaPos(cnode.Pos()), Value: int(value)})

default:
return optional.None[schema.Value]()
}
}
return optional.None[schema.Value]()
}

func visitType(pctx *parseContext, pos token.Pos, tnode types.Type, isExported bool) optional.Option[schema.Type] {
if tparam, ok := tnode.(*types.TypeParam); ok {
return optional.Some[schema.Type](&schema.Ref{Pos: goPosToSchemaPos(pos), Name: tparam.Obj().Id()})
Expand Down
47 changes: 30 additions & 17 deletions go-runtime/compile/schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,26 +543,28 @@ func TestErrorReporting(t *testing.T) {
r, err := ExtractModuleSchema("testdata/failing", &schema.Schema{})
assert.NoError(t, err)

var actualParent []string
var actualChild []string
filename := filepath.Join(pwd, `testdata/failing/failing.go`)
subFilename := filepath.Join(pwd, `testdata/failing/child/child.go`)
actual := slices.Map(r.Errors, func(e *schema.Error) string {
str := strings.ReplaceAll(e.Error(), filename+":", "")
str = strings.ReplaceAll(str, subFilename+":", "")
return str
})
expected := []string{
`6:2-6: unsupported type "uint64" for field "Body"`,
`11:2-2: unsupported external type "github.com/TBD54566975/ftl/go-runtime/compile/testdata.lib.NonFTLType"`,
`11:2-7: unsupported type "github.com/TBD54566975/ftl/go-runtime/compile/testdata.NonFTLType" for field "Field"`,
for _, e := range r.Errors {
str := strings.ReplaceAll(e.Error(), subFilename+":", "")
str = strings.ReplaceAll(str, filename+":", "")
if e.Pos.Filename == filename {
actualParent = append(actualParent, str)
} else {
actualChild = append(actualChild, str)
}
}

// failing/failing.go
expectedParent := []string{
`13:13-34: expected string literal for argument at index 0`,
`16: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`,
`16:18-52: duplicate config declaration at 15:18-52`,
`19:18-52: duplicate secret declaration at 18:18-52`,
`21:6-6: multiple Go type mappings found for "ftl/failing/child.MultipleMappings"`,
`16:18-18: duplicate config declaration for "failing.FTL_ENDPOINT"; already declared at "37:18"`,
`19:18-18: duplicate secret declaration for "failing.FTL_ENDPOINT"; already declared at "38:18"`,
`22:14-44: duplicate database declaration at 21:14-44`,
`25:2-10: unsupported type "error" for field "BadParam"`,
`28:2-17: unsupported type "uint64" for field "AnotherBadParam"`,
`31:2-13: enum variant "SameVariant" conflicts with existing enum variant of "EnumVariantConflictParent" at "196:2"`,
`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:50-50: unsupported response type "ftl/failing.Response"`,
Expand Down Expand Up @@ -593,7 +595,7 @@ func TestErrorReporting(t *testing.T) {
`90:3-3: unexpected directive "ftl:verb"`,
`99:6-18: "BadValueEnum" is a value enum and cannot be tagged as a variant of type enum "TypeEnum" directly`,
`108:6-35: "BadValueEnumOrderDoesntMatter" is a value enum and cannot be tagged as a variant of type enum "TypeEnum" directly`,
`124:21-60: config and secret names must be valid identifiers`,
`124:21-60: config names must be valid identifiers`,
`130:1-1: schema declaration contains conflicting directives`,
`130:1-26: only one directive expected when directive "ftl:enum" is present, found multiple`,
`152:6-45: enum discriminator "TypeEnum3" cannot contain exported methods`,
Expand All @@ -603,9 +605,20 @@ func TestErrorReporting(t *testing.T) {
`175:9-26: can not call verbs in other modules directly: use ftl.Call(…) instead`,
`180:2-12: struct field unexported must be exported by starting with an uppercase letter`,
`184:6-6: unsupported type "ftl/failing/child.BadChildStruct" for field "child"`,
`189:6-6: duplicate Data declaration for "failing.Redeclared" in "ftl/failing"; already declared in "ftl/failing/child"`,
`189:6-6: duplicate data declaration for "failing.Redeclared"; already declared at "27:6"`,
}
assert.Equal(t, expected, actual)

// 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"`,
`14:2-7: unsupported type "github.com/TBD54566975/ftl/go-runtime/compile/testdata.NonFTLType" for field "Field"`,
`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"`,
}
assert.Equal(t, expectedParent, actualParent)
assert.Equal(t, expectedChild, actualChild)
}

// Where parsing is correct but validation of the schema fails
Expand Down
8 changes: 7 additions & 1 deletion go-runtime/compile/testdata/failing/child/child.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package child

import lib "github.com/TBD54566975/ftl/go-runtime/compile/testdata"
import (
lib "github.com/TBD54566975/ftl/go-runtime/compile/testdata"
"github.com/TBD54566975/ftl/go-runtime/ftl"
)

type BadChildStruct struct {
Body uint64
Expand Down Expand Up @@ -30,3 +33,6 @@ type EnumVariantConflictChild int
const (
SameVariant EnumVariantConflictChild = iota
)

var duplConfig = ftl.Config[string]("FTL_ENDPOINT")
var duplSecret = ftl.Secret[string]("FTL_ENDPOINT")
4 changes: 2 additions & 2 deletions go-runtime/compile/testdata/failing/failing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import (

var empty = ftl.Config[string](1)

// var duplConfig = ftl.Config[string]("FTL_ENDPOINT")
var goodConfig = ftl.Config[string]("FTL_ENDPOINT")
var duplConfig = ftl.Config[string]("FTL_ENDPOINT")

// var duplSecret = ftl.Secret[string]("FTL_ENDPOINT")
var goodSecret = ftl.Secret[string]("FTL_ENDPOINT")
var duplSecret = ftl.Secret[string]("FTL_ENDPOINT")

var goodDB = ftl.PostgresDatabase("testDb")
var duplDB = ftl.PostgresDatabase("testDb")
Expand Down
Loading

0 comments on commit d737241

Please sign in to comment.