Skip to content

Commit

Permalink
chore: improve ingress schema validation (#1098)
Browse files Browse the repository at this point in the history
This vastly improves ingress schema validation, ensuring that the
request/response body types are valid, and that if path parameters are
used the request body is a data structure with a corresponding field.

cc @lifeizhou-ap
  • Loading branch information
alecthomas authored Mar 22, 2024
1 parent 6712969 commit 1b15592
Show file tree
Hide file tree
Showing 20 changed files with 391 additions and 163 deletions.
2 changes: 1 addition & 1 deletion backend/schema/any.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ func (*Any) schemaType() {}
func (*Any) schemaSymbol() {}
func (*Any) String() string { return "Any" }
func (a *Any) ToProto() proto.Message { return &schemapb.Any{Pos: posToProto(a.Pos)} }
func (*Any) GetName() string { return "" }
func (*Any) GetName() string { return "Any" }
4 changes: 3 additions & 1 deletion backend/schema/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,12 @@ type Array struct {
}

var _ Type = (*Array)(nil)
var _ Symbol = (*Array)(nil)

func (a *Array) Position() Position { return a.Pos }
func (a *Array) schemaChildren() []Node { return []Node{a.Element} }
func (*Array) schemaType() {}
func (a *Array) schemaType() {}
func (a *Array) schemaSymbol() {}
func (a *Array) String() string { return "[" + a.Element.String() + "]" }

func (a *Array) ToProto() proto.Message {
Expand Down
2 changes: 1 addition & 1 deletion backend/schema/bool.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ func (*Bool) schemaType() {}
func (*Bool) schemaSymbol() {}
func (*Bool) String() string { return "Bool" }
func (b *Bool) ToProto() proto.Message { return &schemapb.Bool{Pos: posToProto(b.Pos)} }
func (*Bool) GetName() string { return "" }
func (*Bool) GetName() string { return "Bool" }
2 changes: 1 addition & 1 deletion backend/schema/bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ func (*Bytes) schemaType() {}
func (*Bytes) schemaSymbol() {}
func (*Bytes) String() string { return "Bytes" }
func (b *Bytes) ToProto() proto.Message { return &schemapb.Bytes{Pos: posToProto(b.Pos)} }
func (*Bytes) GetName() string { return "" }
func (*Bytes) GetName() string { return "Bytes" }
2 changes: 1 addition & 1 deletion backend/schema/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (d *Data) Monomorphise(ref *Ref) (*Data, error) {
Metadata, *MetadataCalls, *MetadataDatabases, *MetadataIngress,
*MetadataAlias, *Module, *Schema, *String, *Time, Type, *TypeParameter,
*Unit, *Verb, *Enum, *EnumVariant,
Value, *IntValue, *StringValue, Symbol:
Value, *IntValue, *StringValue, Symbol, Named:
}
return next()
})
Expand Down
2 changes: 1 addition & 1 deletion backend/schema/float.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ func (*Float) schemaType() {}
func (*Float) schemaSymbol() {}
func (*Float) String() string { return "Float" }
func (f *Float) ToProto() proto.Message { return &schemapb.Float{Pos: posToProto(f.Pos)} }
func (*Float) GetName() string { return "" }
func (*Float) GetName() string { return "Float" }
2 changes: 1 addition & 1 deletion backend/schema/int.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ func (*Int) schemaChildren() []Node { return nil }
func (*Int) schemaType() {}
func (*Int) String() string { return "Int" }
func (i *Int) ToProto() proto.Message { return &schemapb.Int{Pos: posToProto(i.Pos)} }
func (*Int) GetName() string { return "" }
func (*Int) GetName() string { return "Int" }
2 changes: 1 addition & 1 deletion backend/schema/jsonschema.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func nodeToJSSchema(node Node, refs map[RefKey]*Ref) *jsonschema.Schema {
case Decl, *Field, Metadata, *MetadataCalls, *MetadataDatabases, *MetadataIngress,
*MetadataAlias, IngressPathComponent, *IngressPathLiteral, *IngressPathParameter, *Module,
*Schema, Type, *Database, *Verb, *EnumVariant,
Value, *StringValue, *IntValue, *Config, *Secret, Symbol:
Value, *StringValue, *IntValue, *Config, *Secret, Symbol, Named:
panic(fmt.Sprintf("unsupported node type %T", node))

default:
Expand Down
2 changes: 2 additions & 0 deletions backend/schema/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ type Map struct {
}

var _ Type = (*Map)(nil)
var _ Symbol = (*Map)(nil)

func (m *Map) Position() Position { return m.Pos }
func (m *Map) schemaChildren() []Node { return []Node{m.Key, m.Value} }
func (*Map) schemaType() {}
func (*Map) schemaSymbol() {}
func (m *Map) String() string { return fmt.Sprintf("{%s: %s}", m.Key.String(), m.Value.String()) }

func (m *Map) ToProto() proto.Message {
Expand Down
2 changes: 1 addition & 1 deletion backend/schema/normalise.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func Normalise[T Node](n T) T {
c.Pos = zero
c.Type = Normalise(c.Type)

case Symbol, Decl, Metadata, IngressPathComponent, Type, Value: // Can never occur in reality, but here to satisfy the sum-type check.
case Named, Symbol, Decl, Metadata, IngressPathComponent, Type, Value: // Can never occur in reality, but here to satisfy the sum-type check.
panic("??")
}
return ni.(T) //nolint:forcetypeassert
Expand Down
2 changes: 2 additions & 0 deletions backend/schema/optional.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ type Optional struct {
}

var _ Type = (*Optional)(nil)
var _ Symbol = (*Optional)(nil)

func (o *Optional) Position() Position { return o.Pos }
func (o *Optional) String() string { return o.Type.String() + "?" }
func (*Optional) schemaType() {}
func (*Optional) schemaSymbol() {}
func (o *Optional) schemaChildren() []Node { return []Node{o.Type} }
func (o *Optional) ToProto() proto.Message {
return &schemapb.Optional{Pos: posToProto(o.Pos), Type: typeToProto(o.Type)}
Expand Down
7 changes: 6 additions & 1 deletion backend/schema/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,15 @@ type Value interface {
//sumtype:decl
type Symbol interface {
Node
GetName() string
schemaSymbol()
}

// A Named symbol in the grammar.
type Named interface {
Symbol
GetName() string
}

// Decl represents user-defined data types in the schema grammar.
//
//sumtype:decl
Expand Down
116 changes: 3 additions & 113 deletions backend/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ module todo {
+calls todo.destroy +database calls todo.testdb
verb destroy(builtin.HttpRequest<todo.DestroyRequest>) builtin.HttpResponse<todo.DestroyResponse, String>
+ingress http GET /todo/destroy/{id}
+ingress http GET /todo/destroy/{name}
}
module foo {
Expand Down Expand Up @@ -389,7 +389,7 @@ module todo {
verb create(todo.CreateRequest) todo.CreateResponse
+calls todo.destroy +database calls todo.testdb
verb destroy(builtin.HttpRequest<todo.DestroyRequest>) builtin.HttpResponse<todo.DestroyResponse, String>
+ingress http GET /todo/destroy/{id}
+ingress http GET /todo/destroy/{name}
}
`
actual, err := ParseModuleString("", input)
Expand Down Expand Up @@ -480,7 +480,7 @@ var testSchema = MustValidate(&Schema{
Path: []IngressPathComponent{
&IngressPathLiteral{Text: "todo"},
&IngressPathLiteral{Text: "destroy"},
&IngressPathParameter{Name: "id"},
&IngressPathParameter{Name: "name"},
},
},
},
Expand Down Expand Up @@ -513,113 +513,3 @@ var testSchema = MustValidate(&Schema{
},
},
})

func TestValidateDependencies(t *testing.T) {
tests := []struct {
name string
schema string
err string
}{
{
// one <--> two, cyclical
name: "TwoModuleCycle",
schema: `
module one {
verb one(builtin.Empty) builtin.Empty
+calls two.two
}
module two {
verb two(builtin.Empty) builtin.Empty
+calls one.one
}
`,
err: "found cycle in dependencies: two -> one -> two",
},
{
// one --> two --> three, noncyclical
name: "ThreeModulesNoCycle",
schema: `
module one {
verb one(builtin.Empty) builtin.Empty
+calls two.two
}
module two {
verb two(builtin.Empty) builtin.Empty
+calls three.three
}
module three {
verb three(builtin.Empty) builtin.Empty
}
`,
err: "",
},
{
// one --> two --> three -> one, cyclical
name: "ThreeModulesCycle",
schema: `
module one {
verb one(builtin.Empty) builtin.Empty
+calls two.two
}
module two {
verb two(builtin.Empty) builtin.Empty
+calls three.three
}
module three {
verb three(builtin.Empty) builtin.Empty
+calls one.one
}
`,
err: "found cycle in dependencies: two -> three -> one -> two",
},
{
// one.a --> two.a
// one.b <---
// cyclical (does not depend on verbs used)
name: "TwoModuleCycleDiffVerbs",
schema: `
module one {
verb a(builtin.Empty) builtin.Empty
+calls two.a
verb b(builtin.Empty) builtin.Empty
}
module two {
verb a(builtin.Empty) builtin.Empty
+calls one.b
}
`,
err: "found cycle in dependencies: two -> one -> two",
},
{
// one --> one, this is allowed
name: "SelfReference",
schema: `
module one {
verb a(builtin.Empty) builtin.Empty
+calls one.b
verb b(builtin.Empty) builtin.Empty
+calls one.a
}
`,
err: "",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
_, err := ParseString("", test.schema)
if test.err == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, test.err)
}
})
}
}
2 changes: 1 addition & 1 deletion backend/schema/string.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ func (*String) schemaType() {}
func (*String) schemaSymbol() {}
func (*String) String() string { return "String" }
func (s *String) ToProto() proto.Message { return &schemapb.String{Pos: posToProto(s.Pos)} }
func (*String) GetName() string { return "" }
func (*String) GetName() string { return "String" }
2 changes: 1 addition & 1 deletion backend/schema/time.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ func (*Time) schemaType() {}
func (*Time) schemaSymbol() {}
func (*Time) String() string { return "Time" }
func (t *Time) ToProto() proto.Message { return &schemapb.Time{Pos: posToProto(t.Pos)} }
func (*Time) GetName() string { return "" }
func (*Time) GetName() string { return "Time" }
50 changes: 50 additions & 0 deletions backend/schema/typeresolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,34 @@ func (s Scope) String() string {
return out.String()
}

// ResolveTypeAs resolves a [Type] to a concrete symbol and declaration.
func ResolveTypeAs[S Symbol](scopes Scopes, t Type) (symbol S, decl *ModuleDecl) {
decl = scopes.ResolveType(t)
if decl == nil {
return
}
var ok bool
symbol, ok = decl.Symbol.(S)
if !ok {
return
}
return symbol, decl
}

// ResolveAs resolves a [Ref] to a concrete symbol and declaration.
func ResolveAs[S Symbol](scopes Scopes, ref Ref) (symbol S, decl *ModuleDecl) {
decl = scopes.Resolve(ref)
if decl == nil {
return
}
var ok bool
symbol, ok = decl.Symbol.(S)
if !ok {
return
}
return symbol, decl
}

// Scopes to search during type resolution.
type Scopes []Scope

Expand Down Expand Up @@ -95,6 +123,22 @@ func (s *Scopes) Add(owner *Module, name string, symbol Symbol) error {
return nil
}

func (s Scopes) ResolveType(t Type) *ModuleDecl {
switch t := t.(type) {
case Named:
return s.Resolve(Ref{Name: t.GetName()})

case *Ref:
return s.Resolve(*t)

case Symbol:
return &ModuleDecl{nil, t}

default:
return nil
}
}

// Resolve a reference to a symbol declaration or nil.
func (s Scopes) Resolve(ref Ref) *ModuleDecl {
if ref.Module == "" {
Expand All @@ -115,6 +159,12 @@ func (s Scopes) Resolve(ref Ref) *ModuleDecl {
// Holy nested if statement Batman.
return decl
}
} else {
for _, d := range mdecl.Module.Decls {
if d.GetName() == ref.Name {
return &ModuleDecl{mdecl.Module, d}
}
}
}
}
}
Expand Down
41 changes: 41 additions & 0 deletions backend/schema/typeresolver_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package schema

import (
"testing"

"github.com/alecthomas/assert/v2"
)

func TestTypeResolver(t *testing.T) {
module, err := ParseModuleString("", `
module test {
data Request<T> {
t T
}
verb test(Request<String>) Empty
}
`)
assert.NoError(t, err)
scopes := NewScopes()
scopes = scopes.PushScope(module.Scope())

// Resolve a builtin.
actualInt, _ := ResolveAs[*Int](scopes, Ref{Name: "Int"})
assert.NotZero(t, actualInt)
assert.Equal(t, &Int{}, actualInt, assert.Exclude[Position]())

// Resolve a generic data structure.
actualData, _ := ResolveAs[*Data](scopes, Ref{Module: "test", Name: "Request"})
assert.NotZero(t, actualData)
expectedData := &Data{
Name: "Request",
TypeParameters: []*TypeParameter{{Name: "T"}},
Fields: []*Field{{Name: "t", Type: &Ref{Name: "T"}}},
}
assert.Equal(t, expectedData, actualData, assert.Exclude[Position]())

// Resolve a type parameter.
scopes = scopes.PushScope(actualData.Scope())
actualTP, _ := ResolveAs[*TypeParameter](scopes, Ref{Name: "T"})
assert.Equal(t, actualTP, &TypeParameter{Name: "T"}, assert.Exclude[Position]())
}
2 changes: 1 addition & 1 deletion backend/schema/unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ func (u *Unit) schemaSymbol() {}
func (u *Unit) String() string { return "Unit" }
func (u *Unit) ToProto() protoreflect.ProtoMessage { return &schemapb.Unit{Pos: posToProto(u.Pos)} }
func (u *Unit) schemaChildren() []Node { return nil }
func (u *Unit) GetName() string { return "" }
func (u *Unit) GetName() string { return "Unit" }
Loading

0 comments on commit 1b15592

Please sign in to comment.