Skip to content

Commit

Permalink
fix: skip validation on external req/resp types
Browse files Browse the repository at this point in the history
We don't have access to the full schema at this point, so we can't validate that external types exist
  • Loading branch information
wesbillman committed Apr 18, 2024
1 parent 6207941 commit 3fb2f6e
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 13 deletions.
3 changes: 1 addition & 2 deletions backend/protos/xyz/block/ftl/v1/ftl.proto
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,7 @@ message StreamDeploymentLogsRequest {
}
message StreamDeploymentLogsResponse {}

message StatusRequest {
}
message StatusRequest {}
message StatusResponse {
message Controller {
string key = 1;
Expand Down
23 changes: 15 additions & 8 deletions backend/schema/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ func ValidateModule(module *Module) error {
merr = append(merr, errorf(n, "Verb name %q is a reserved word", n.Name))
}

merr = append(merr, validateVerbMetadata(scopes, n)...)
merr = append(merr, validateVerbMetadata(scopes, module, n)...)

case *Data:
if !ValidateName(n.Name) {
Expand Down Expand Up @@ -443,7 +443,7 @@ func errorf(pos interface{ Position() Position }, format string, args ...interfa
return Errorf(pos.Position(), pos.Position().Column, format, args...)
}

func validateVerbMetadata(scopes Scopes, n *Verb) (merr []error) {
func validateVerbMetadata(scopes Scopes, module *Module, n *Verb) (merr []error) {
// Validate metadata
metadataTypes := map[reflect.Type]bool{}
for _, md := range n.Metadata {
Expand All @@ -456,9 +456,9 @@ func validateVerbMetadata(scopes Scopes, n *Verb) (merr []error) {

switch md := md.(type) {
case *MetadataIngress:
reqBodyType, reqBody, errs := validateIngressRequestOrResponse(scopes, n, "request", n.Request)
reqBodyType, reqBody, errs := validateIngressRequestOrResponse(scopes, module, n, "request", n.Request)
merr = append(merr, errs...)
_, _, errs = validateIngressRequestOrResponse(scopes, n, "response", n.Response)
_, _, errs = validateIngressRequestOrResponse(scopes, module, n, "response", n.Response)
merr = append(merr, errs...)

// Validate path
Expand Down Expand Up @@ -492,11 +492,11 @@ func validateVerbMetadata(scopes Scopes, n *Verb) (merr []error) {
return
}

func validateIngressRequestOrResponse(scopes Scopes, n *Verb, reqOrResp string, r Type) (fieldType Type, body Symbol, 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)
module, _ := sym.Module.Get()
if sym == nil || module == nil || module.Name != "builtin" || resp.Name != "Http"+strings.Title(reqOrResp) {
m, _ := sym.Module.Get()
if sym == nil || m == nil || m.Name != "builtin" || resp.Name != "Http"+strings.Title(reqOrResp) {
merr = append(merr, errorf(r, "ingress verb %s: %s type %s must be builtin.HttpRequest", n.Name, reqOrResp, r))
return
}
Expand All @@ -512,9 +512,16 @@ func validateIngressRequestOrResponse(scopes Scopes, n *Verb, reqOrResp string,
if opt, ok := fieldType.(*Optional); ok {
fieldType = opt.Type
}

if ref, err := ParseRef(fieldType.String()); err == nil {
if ref.Module != "" && ref.Module != module.Name {
return // ignores references to other modules.
}
}

bodySym := scopes.ResolveType(fieldType)
if bodySym == nil {
merr = append(merr, errorf(resp, "ingress verb %s: couldn't resolve %s body type %s", n.Name, reqOrResp, fieldType))
merr = append(merr, errorf(r, "ingress verb %s: couldn't resolve %s body type %s", n.Name, reqOrResp, fieldType))
return
}
body = bodySym.Symbol
Expand Down
11 changes: 10 additions & 1 deletion backend/schema/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ func TestValidate(t *testing.T) {
"6:10-10: verb can not have multiple instances of ingress",
},
},

{name: "CronOnNonEmptyVerb",
schema: `
module one {
Expand All @@ -186,6 +185,16 @@ func TestValidate(t *testing.T) {
"6:7-7: verb verbWithWrongOutput: cron job can not have a response type",
},
},
{name: "IngressBodyExternalType",
schema: `
module two {
data Data {}
}
module one {
verb a(HttpRequest<two.Data>) HttpResponse<two.Data, Empty>
+ingress http GET /a
}
`},
}

for _, test := range tests {
Expand Down
6 changes: 4 additions & 2 deletions buildengine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,6 @@ func (e *Engine) buildWithCallback(ctx context.Context, callback buildCallback,
}

topology := TopologicalSort(graph)

errCh := make(chan error, 1024)
for _, group := range topology {
// Collect schemas to be inserted into "built" map for subsequent groups.
Expand All @@ -464,6 +463,8 @@ func (e *Engine) buildWithCallback(ctx context.Context, callback buildCallback,
key := ProjectKey(keyStr)

wg.Go(func() error {
logger := log.FromContext(ctx).Scope(string(key))
ctx = log.ContextWithLogger(ctx, logger)
err := e.tryBuild(ctx, mustBuild, key, builtModules, schemas, callback)
if err != nil {
errCh <- err
Expand Down Expand Up @@ -499,6 +500,7 @@ func (e *Engine) buildWithCallback(ctx context.Context, callback buildCallback,

func (e *Engine) tryBuild(ctx context.Context, mustBuild map[ProjectKey]bool, key ProjectKey, builtModules map[string]*schema.Module, schemas chan *schema.Module, callback buildCallback) error {
logger := log.FromContext(ctx)

if !mustBuild[key] {
return e.mustSchema(ctx, key, builtModules, schemas)
}
Expand All @@ -510,7 +512,7 @@ func (e *Engine) tryBuild(ctx context.Context, mustBuild map[ProjectKey]bool, ke

for _, dep := range meta.project.Config().Dependencies {
if _, ok := builtModules[dep]; !ok {
logger.Warnf("%q build skipped because its dependency %q failed to build", key, dep)
logger.Warnf("build skipped because dependency %q failed to build", dep)
return nil
}
}
Expand Down

0 comments on commit 3fb2f6e

Please sign in to comment.