Skip to content

Commit

Permalink
support http-valid characters in ingress paths
Browse files Browse the repository at this point in the history 
fixes #811
  • Loading branch information
@worstell
worstell committed Jan 20, 2024
1 parent 7f3a79e commit 8c7879a
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 106 deletions.
4 changes: 2 additions & 2 deletions backend/schema/metadataingress.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
type MetadataIngress struct {
Pos Position `parser:"" protobuf:"1,optional"`

Type string `parser:"'ingress' @('http' | 'ftl')" protobuf:"2"`
Type string `parser:"'ingress' @('http' | 'ftl')?" protobuf:"2"`
Method string `parser:"@('GET' | 'POST' | 'PUT' | 'DELETE')" protobuf:"3"`
Path []IngressPathComponent `parser:"('/' @@)+" protobuf:"4"`
}
Expand Down Expand Up @@ -79,7 +79,7 @@ type IngressPathComponent interface {
type IngressPathLiteral struct {
Pos Position `parser:"" protobuf:"1,optional"`

Text string `parser:"@Ident" protobuf:"2"`
Text string `parser:"@~(Whitespace | '/' | '{' | '}')+" protobuf:"2"`
}

var _ IngressPathComponent = (*IngressPathLiteral)(nil)
Expand Down
6 changes: 3 additions & 3 deletions backend/schema/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,17 @@ var (
reflect.TypeOf((*Decl)(nil)).Elem(): reflectUnion(declUnion...),
}

lex = lexer.MustSimple([]lexer.SimpleRule{
Lexer = lexer.MustSimple([]lexer.SimpleRule{
{Name: "Whitespace", Pattern: `\s+`},
{Name: "Ident", Pattern: `\b[a-zA-Z_][a-zA-Z0-9_]*\b`},
{Name: "Comment", Pattern: `//.*`},
{Name: "String", Pattern: `"(?:\\.|[^"])*"`},
{Name: "Number", Pattern: `[0-9]+(?:\.[0-9]+)?`},
{Name: "Punct", Pattern: `[/-:[\]{}<>()*+?.,\\^$|#]`},
{Name: "Punct", Pattern: `[%/\-\_:[\]{}<>()*+?.,\\^$|#~!\'@]`},
})

commonParserOptions = []participle.Option{
participle.Lexer(lex),
participle.Lexer(Lexer),
participle.Elide("Whitespace"),
participle.Unquote(),
participle.Map(func(token lexer.Token) (lexer.Token, error) {
Expand Down
29 changes: 5 additions & 24 deletions go-runtime/compile/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"go/token"
"strings"

"github.com/TBD54566975/ftl/backend/schema"
"github.com/alecthomas/participle/v2"
"github.com/alecthomas/participle/v2/lexer"
)
Expand All @@ -32,9 +33,7 @@ func (*directiveVerb) directive() {}
func (d *directiveVerb) String() string { return "ftl:verb" }

type directiveIngress struct {
Pos lexer.Position

Type directiveIngressType `parser:"'ingress' @@"`
schema.MetadataIngress
}

func (*directiveIngress) directive() {}
Expand All @@ -49,31 +48,13 @@ type directiveModule struct {
func (*directiveModule) directive() {}
func (d *directiveModule) String() string { return "ftl:module" }

//sumtype:decl
type directiveIngressType interface{ directiveIngressType() }

type directiveIngressHTTP struct {
Pos lexer.Position

Type string `parser:"@('http' | 'ftl')?"`
Method string `parser:"@('GET'|'POST'|'PUT'|'DELETE'|'PATCH')"`
Path string `parser:"@(('/' (('{' Ident '}') | Ident))+ '/'?)"`
}

func (d *directiveIngressHTTP) directiveIngressType() {}
func (d *directiveIngressHTTP) String() string {
typ := d.Type
if d.Type == "" {
typ = "ftl"
}
return fmt.Sprintf("%s %s %s", typ, d.Method, d.Path)
}

var directiveParser = participle.MustBuild[directiveWrapper](
participle.Lexer(schema.Lexer),
participle.Elide("Whitespace"),
participle.Unquote(),
participle.UseLookahead(2),
participle.Union[directive](&directiveVerb{}, &directiveIngress{}, &directiveModule{}),
participle.Union[directiveIngressType](&directiveIngressHTTP{}),
participle.Union[schema.IngressPathComponent](&schema.IngressPathLiteral{}, &schema.IngressPathParameter{}),
)

func parseDirectives(fset *token.FileSet, docs *ast.CommentGroup) ([]directive, error) {
Expand Down
22 changes: 6 additions & 16 deletions go-runtime/compile/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,22 +222,12 @@ func visitFuncDecl(pctx *parseContext, node *ast.FuncDecl) (verb *schema.Verb, e
isVerb = true

case *directiveIngress:
switch ingress := dir.Type.(type) {
case *directiveIngressHTTP:
ingressType := "ftl"
if ingress.Type != "" {
ingressType = ingress.Type
}
metadata = append(metadata, &schema.MetadataIngress{
Pos: schema.Position(dir.Pos),
Type: ingressType,
Method: ingress.Method,
Path: parsePathComponents(ingress.Path, schema.Position(dir.Pos)),
})

default:
panic(fmt.Sprintf("unsupported ingress type %T", ingress))
}
metadata = append(metadata, &schema.MetadataIngress{
Pos: dir.Pos,
Type: dir.Type,
Method: dir.Method,
Path: dir.Path,
})

default:
panic(fmt.Sprintf("unsupported directive %T", dir))
Expand Down
37 changes: 18 additions & 19 deletions go-runtime/compile/schema_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package compile

import (
"github.com/alecthomas/participle/v2/lexer"
"go/ast"
"go/types"
"testing"

"github.com/alecthomas/assert/v2"
"github.com/alecthomas/participle/v2/lexer"

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

func TestExtractModuleSchema(t *testing.T) {
Expand Down Expand Up @@ -50,24 +49,24 @@ func TestParseDirectives(t *testing.T) {
}{
{name: "Module", input: "ftl:module foo", expected: &directiveModule{Name: "foo"}},
{name: "Verb", input: "ftl:verb", expected: &directiveVerb{Verb: true}},
{name: "IngressImplicitFTL", input: `ftl:ingress GET /foo`, expected: &directiveIngress{
Type: &directiveIngressHTTP{
{name: "Ingress", input: `ftl:ingress GET /foo`, expected: &directiveIngress{
MetadataIngress: schema.MetadataIngress{
Method: "GET",
Path: "/foo",
},
}},
{name: "IngressFTL", input: `ftl:ingress ftl POST /bar`, expected: &directiveIngress{
Type: &directiveIngressHTTP{
Type: "ftl",
Method: "POST",
Path: "/bar",
Path: []schema.IngressPathComponent{
&schema.IngressPathLiteral{
Text: "foo",
},
},
},
}},
{name: "IngressHTTP", input: `ftl:ingress http POST /bar`, expected: &directiveIngress{
Type: &directiveIngressHTTP{
Type: "http",
Method: "POST",
Path: "/bar",
{name: "Ingress", input: `ftl:ingress GET /test_path/987-Your_File.txt%7E%21Misc%2A%28path%29info%40abc%3Fxyz`, expected: &directiveIngress{
MetadataIngress: schema.MetadataIngress{
Method: "GET",
Path: []schema.IngressPathComponent{
&schema.IngressPathLiteral{
Text: "/test_path/987-Your_File.txt%7E%21Misc%2A%28path%29info%40abc%3Fxyz",
},
},
},
}},
}
Expand All @@ -76,7 +75,7 @@ func TestParseDirectives(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
got, err := directiveParser.ParseString("", tt.input)
assert.NoError(t, err)
assert.Equal(t, tt.expected, got.Directive, assert.Exclude[lexer.Position]())
assert.Equal(t, tt.expected, got.Directive, assert.Exclude[lexer.Position](), assert.Exclude[schema.Position]())
})
}
}
Expand Down
6 changes: 6 additions & 0 deletions kotlin-runtime/ftl-runtime/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@
<version>2.2.0</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>3.25.1</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
48 changes: 6 additions & 42 deletions ...untime/ftl-runtime/src/test/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRuleTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package xyz.block.ftl.schemaextractor
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
import xyz.block.ftl.schemaextractor.ExtractSchemaRule.Companion.OUTPUT_FILENAME
Expand Down Expand Up @@ -82,11 +82,6 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) {
type = Type(string = xyz.block.ftl.v1.schema.String())
)
),
pos = Position(
filename = "Test.kt",
line = 14,
column = 9,
),
),
),
Decl(
Expand All @@ -105,22 +100,12 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) {
value_ = xyz.block.ftl.v1.schema.Type(
dataRef = DataRef(
name = "MapValue",
pos = Position(
filename = "Test.kt",
line = 15,
column = 67,
),
)
)
)
)
)
),
pos = Position(
filename = "Test.kt",
line = 15,
column = 9,
),
),
),
Decl(
Expand All @@ -141,11 +126,6 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) {
* Request to echo a message.
*/"""
),
pos = Position(
filename = "Test.kt",
line = 17,
column = 9,
),
),
),
Decl(
Expand All @@ -159,22 +139,12 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) {
element = xyz.block.ftl.v1.schema.Type(
dataRef = DataRef(
name = "EchoMessage",
pos = Position(
filename = "Test.kt",
line = 21,
column = 47,
)
)
)
)
)
)
),
pos = Position(
filename = "Test.kt",
line = 21,
column = 9,
),
),
),
Decl(
Expand All @@ -188,21 +158,11 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) {
request = Type(
dataRef = DataRef(
name = "EchoRequest",
pos = Position(
filename = "Test.kt",
line = 33,
column = 40,
),
)
),
response = Type(
dataRef = DataRef(
name = "EchoResponse",
pos = Position(
filename = "Test.kt",
line = 33,
column = 59,
),
)
),
metadata = listOf(
Expand Down Expand Up @@ -233,7 +193,11 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) {
)
)

assertEquals(expected, module)
assertThat(module)
.usingRecursiveComparison()
.withEqualsForType({ _, _ -> true }, Position::class.java)
.ignoringFieldsMatchingRegexes(".*hashCode\$")
.isEqualTo(expected)
}

@Test
Expand Down

0 comments on commit 8c7879a

Please sign in to comment.