Skip to content

Commit

Permalink
fix: ensure topic name matches idiomatic variable name (#1958)
Browse files Browse the repository at this point in the history
Fixes #1919
  • Loading branch information
safeer authored Jul 23, 2024
1 parent 899be16 commit c3c7aa6
Show file tree
Hide file tree
Showing 16 changed files with 95 additions and 37 deletions.
12 changes: 6 additions & 6 deletions backend/controller/pubsub/testdata/go/publisher/publisher.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

//ftl:export
var topic = ftl.Topic[PubSubEvent]("test_topic")
var TestTopic = ftl.Topic[PubSubEvent]("test_topic")

type PubSubEvent struct {
Time time.Time
Expand All @@ -21,7 +21,7 @@ func PublishTen(ctx context.Context) error {
for i := 0; i < 10; i++ {
t := time.Now()
logger.Infof("Publishing %v", t)
err := topic.Publish(ctx, PubSubEvent{Time: t})
err := TestTopic.Publish(ctx, PubSubEvent{Time: t})
if err != nil {
return err
}
Expand All @@ -34,16 +34,16 @@ func PublishOne(ctx context.Context) error {
logger := ftl.LoggerFromContext(ctx)
t := time.Now()
logger.Infof("Publishing %v", t)
return topic.Publish(ctx, PubSubEvent{Time: t})
return TestTopic.Publish(ctx, PubSubEvent{Time: t})
}

//ftl:export
var topic2 = ftl.Topic[PubSubEvent]("topic2")
var Topic2 = ftl.Topic[PubSubEvent]("topic_2")

//ftl:verb
func PublishOneToTopic2(ctx context.Context) error {
logger := ftl.LoggerFromContext(ctx)
t := time.Now()
logger.Infof("Publishing to topic2 %v", t)
return topic2.Publish(ctx, PubSubEvent{Time: t})
logger.Infof("Publishing to topic_2 %v", t)
return Topic2.Publish(ctx, PubSubEvent{Time: t})
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/TBD54566975/ftl/go-runtime/ftl" // Import the FTL SDK.
)

var _ = ftl.Subscription(publisher.Test_topic, "test_subscription")
var _ = ftl.Subscription(publisher.TestTopic, "test_subscription")

//ftl:verb
//ftl:subscribe test_subscription
Expand All @@ -30,6 +30,6 @@ func ConsumeButFailAndRetry(ctx context.Context, req publisher.PubSubEvent) erro
//ftl:verb
func PublishToExternalModule(ctx context.Context) error {
// Get around compile-time checks
var topic = publisher.Test_topic
var topic = publisher.TestTopic
return topic.Publish(ctx, publisher.PubSubEvent{Time: time.Now()})
}
12 changes: 12 additions & 0 deletions backend/schema/strcase/case.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,18 @@ func ToUpperCamel(s string) string {
return strings.Join(parts, "")
}

func ToUpperStrippedCamel(s string) string {
parts := split(s)
out := make([]string, 0, len(parts)*2)
for i := range parts {
if parts[i] == "-" || parts[i] == "_" {
continue
}
out = append(out, title(parts[i]))
}
return strings.Join(out, "")
}

func ToLowerSnake(s string) string {
parts := split(s)
out := make([]string, 0, len(parts)*2)
Expand Down
28 changes: 28 additions & 0 deletions backend/schema/strcase/case_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,34 @@ func TestUpperCamelCase(t *testing.T) {
}
}

func TestUpperStrippedCamel(t *testing.T) {
for _, tt := range []struct {
input string
expected string
}{
{"lowercase", "Lowercase"},
{"Class", "Class"},
{"MyClass", "MyClass"},
{"MyC", "MyC"},
{"HTML", "Html"},
{"PDFLoader", "PdfLoader"},
{"AString", "AString"},
{"SimpleXMLParser", "SimpleXmlParser"},
{"vimRPCPlugin", "VimRpcPlugin"},
{"GL11Version", "Gl11Version"},
{"99Bottles", "99Bottles"},
{"May5", "May5"},
{"BFG9000", "Bfg9000"},
{"BöseÜberraschung", "BöseÜberraschung"},
{"snake_case", "SnakeCase"},
{"snake_Case_Caps", "SnakeCaseCaps"},
{"kebab-numbers99", "KebabNumbers99"},
} {
actual := ToUpperStrippedCamel(tt.input)
assert.Equal(t, tt.expected, actual, "UpperStrippedCamel(%q) = %v; want %v", tt.input, actual, tt.expected)
}
}

func TestLowerSnake(t *testing.T) {
for _, tt := range []struct {
input string
Expand Down
1 change: 1 addition & 0 deletions cmd/ftl/cmd_new.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ var scaffoldFuncs = template.FuncMap{
"screamingSnake": strcase.ToUpperSnake,
"camel": strcase.ToUpperCamel,
"lowerCamel": strcase.ToLowerCamel,
"strippedCamel": strcase.ToUpperStrippedCamel,
"kebab": strcase.ToLowerKebab,
"screamingKebab": strcase.ToUpperKebab,
"upper": strings.ToUpper,
Expand Down
6 changes: 3 additions & 3 deletions docs/content/docs/reference/pubsub.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ FTL has first-class support for PubSub, modelled on the concepts of topics (wher
First, declare a new topic:

```go
var invoicesTopic = ftl.Topic[Invoice]("invoices")
var Invoices = ftl.Topic[Invoice]("invoices")
```

Then declare each subscription on the topic:

```go
var _ = ftl.Subscription(invoicesTopic, "emailInvoices")
var _ = ftl.Subscription(Invoices, "emailInvoices")
```

And finally define a Sink to consume from the subscription:
Expand All @@ -39,7 +39,7 @@ func SendInvoiceEmail(ctx context.Context, in Invoice) error {
Events can be published to a topic like so:

```go
invoicesTopic.Publish(ctx, Invoice{...})
Invoices.Publish(ctx, Invoice{...})
```

> **NOTE!**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ var _ = context.Background
//
{{- end}}
{{- if is "Topic" .}}
var {{.Name|title}} = ftl.Topic[{{type $.Module .Event}}]("{{.Name}}")
var {{.Name|strippedCamel}} = ftl.Topic[{{type $.Module .Event}}]("{{.Name}}")
{{- else if and (is "Enum" .) .IsValueEnum}}
{{- $enumName := .Name}}
//ftl:enum
Expand Down
8 changes: 4 additions & 4 deletions go-runtime/compile/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,10 @@ func TestExtractModulePubSub(t *testing.T) {
actual := schema.Normalise(r.Module)
expected := `module pubsub {
topic payins pubsub.PayinEvent
// publicBroadcast is a topic that broadcasts payin events to the public.
// public_broadcast is a topic that broadcasts payin events to the public.
// out of order with subscription registration to test ordering doesn't matter.
export topic publicBroadcast pubsub.PayinEvent
subscription broadcastSubscription pubsub.publicBroadcast
export topic public_broadcast pubsub.PayinEvent
subscription broadcastSubscription pubsub.public_broadcast
subscription paymentProcessing pubsub.payins
export data PayinEvent {
Expand Down Expand Up @@ -439,7 +439,7 @@ func TestExtractModuleSubscriber(t *testing.T) {
assert.Equal(t, nil, r.Errors, "expected no schema errors")
actual := schema.Normalise(r.Module)
expected := `module subscriber {
subscription subscriptionToExternalTopic pubsub.publicBroadcast
subscription subscriptionToExternalTopic pubsub.public_broadcast
verb consumesSubscriptionFromExternalTopic(pubsub.PayinEvent) Unit
+subscribe subscriptionToExternalTopic
Expand Down
14 changes: 7 additions & 7 deletions go-runtime/compile/testdata/pubsub/pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type PayinEvent struct {

//ftl:verb
func Payin(ctx context.Context) error {
if err := payinsVar.Publish(ctx, PayinEvent{Name: "Test"}); err != nil {
if err := Payins.Publish(ctx, PayinEvent{Name: "Test"}); err != nil {
return fmt.Errorf("failed to publish event: %w", err)
}
return nil
Expand All @@ -26,21 +26,21 @@ func ProcessPayin(ctx context.Context, event PayinEvent) error {
return nil
}

var _ = ftl.Subscription(payinsVar, "paymentProcessing")
var _ = ftl.Subscription(Payins, "paymentProcessing")

var payinsVar = ftl.Topic[PayinEvent]("payins")
var Payins = ftl.Topic[PayinEvent]("payins")

var _ = ftl.Subscription(broadcast, "broadcastSubscription")
var _ = ftl.Subscription(PublicBroadcast, "broadcastSubscription")

// publicBroadcast is a topic that broadcasts payin events to the public.
// public_broadcast is a topic that broadcasts payin events to the public.
// out of order with subscription registration to test ordering doesn't matter.
//
//ftl:export
var broadcast = ftl.Topic[PayinEvent]("publicBroadcast")
var PublicBroadcast = ftl.Topic[PayinEvent]("public_broadcast")

//ftl:verb export
func Broadcast(ctx context.Context) error {
if err := broadcast.Publish(ctx, PayinEvent{Name: "Broadcast"}); err != nil {
if err := PublicBroadcast.Publish(ctx, PayinEvent{Name: "Broadcast"}); err != nil {
return fmt.Errorf("failed to publish broadcast event: %w", err)
}
return nil
Expand Down
6 changes: 3 additions & 3 deletions go-runtime/ftl/ftltest/testdata/go/pubsub/pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (
)

//ftl:export
var topic = ftl.Topic[Event]("topic")
var subscription = ftl.Subscription(topic, "subscription")
var Topic = ftl.Topic[Event]("topic")
var subscription = ftl.Subscription(Topic, "subscription")

//ftl:data
type Event struct {
Expand All @@ -20,7 +20,7 @@ type Event struct {

//ftl:verb
func PublishToTopicOne(ctx context.Context, event Event) error {
return topic.Publish(ctx, event)
return Topic.Publish(ctx, event)
}

//ftl:subscribe subscription
Expand Down
4 changes: 2 additions & 2 deletions go-runtime/ftl/ftltest/testdata/go/pubsub/pubsub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestSubscriberReturningErrors(t *testing.T) {
}
ftltest.WaitForSubscriptionsToComplete(ctx)
assert.Equal(t, count, len(ftltest.ErrorsForSubscription(ctx, subscription)))
assert.Equal(t, count, len(ftltest.EventsForTopic(ctx, topic)))
assert.Equal(t, count, len(ftltest.EventsForTopic(ctx, Topic)))
}

func TestMultipleMultipleFakeSubscribers(t *testing.T) {
Expand All @@ -50,6 +50,6 @@ func TestMultipleMultipleFakeSubscribers(t *testing.T) {
}
ftltest.WaitForSubscriptionsToComplete(ctx)
assert.Equal(t, 0, len(ftltest.ErrorsForSubscription(ctx, subscription)))
assert.Equal(t, count, len(ftltest.EventsForTopic(ctx, topic)))
assert.Equal(t, count, len(ftltest.EventsForTopic(ctx, Topic)))
assert.Equal(t, count, counter.Load())
}
7 changes: 2 additions & 5 deletions go-runtime/schema/subscription/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ package subscription
import (
"go/ast"
"go/types"
"strings"

"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/backend/schema/strcase"
"github.com/TBD54566975/ftl/go-runtime/schema/common"
"github.com/TBD54566975/golang-tools/go/analysis"
"github.com/TBD54566975/golang-tools/go/analysis/passes/inspect"
Expand Down Expand Up @@ -91,10 +91,7 @@ func extractSubscription(pass *analysis.Pass, obj types.Object, node *ast.CallEx
common.Errorf(pass, node, "subscription registration must have a topic")
return optional.None[*schema.Subscription]()
}
name := strings.ToLower(string(varName[0]))
if len(varName) > 1 {
name += varName[1:]
}
name := strcase.ToLowerSnake(varName)
topicRef = &schema.Ref{
Module: moduleIdent.Name,
Name: name,
Expand Down
23 changes: 21 additions & 2 deletions go-runtime/schema/topic/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"go/types"

"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/backend/schema/strcase"
"github.com/TBD54566975/ftl/go-runtime/schema/common"
"github.com/TBD54566975/golang-tools/go/analysis"
"github.com/TBD54566975/golang-tools/go/analysis/passes/inspect"
Expand Down Expand Up @@ -47,7 +48,7 @@ func Extract(pass *analysis.Pass) (interface{}, error) {
return common.NewExtractorResult(pass), nil
}

// expects: _ = ftl.Topic[EventType]("name_literal")
// expects: var NameLiteral = ftl.Topic[EventType]("name_literal")
func extractTopic(pass *analysis.Pass, node *ast.GenDecl, callExpr *ast.CallExpr, obj types.Object) optional.Option[*schema.Topic] {
indexExpr, ok := callExpr.Fun.(*ast.IndexExpr)
if !ok {
Expand All @@ -60,9 +61,27 @@ func extractTopic(pass *analysis.Pass, node *ast.GenDecl, callExpr *ast.CallExpr
return optional.None[*schema.Topic]()
}

topicName := common.ExtractStringLiteralArg(pass, callExpr, 0)
expTopicName := strcase.ToLowerSnake(topicName)
if topicName != expTopicName {
common.Errorf(pass, node, "unsupported topic name %q, did you mean to use %q?", topicName, expTopicName)
return optional.None[*schema.Topic]()
}

if len(node.Specs) > 0 {
if t, ok := node.Specs[0].(*ast.ValueSpec); ok {
varName := t.Names[0].Name
expVarName := strcase.ToUpperStrippedCamel(topicName)
if varName != expVarName {
common.Errorf(pass, node, "unexpected topic variable name %q, did you mean %q?", varName, expVarName)
return optional.None[*schema.Topic]()
}
}
}

topic := &schema.Topic{
Pos: common.GoPosToSchemaPos(pass.Fset, node.Pos()),
Name: common.ExtractStringLiteralArg(pass, callExpr, 0),
Name: topicName,
Event: typeParamType,
}
common.ApplyMetadata[*schema.Subscription](pass, obj, func(md *common.ExtractedMetadata) {
Expand Down
1 change: 1 addition & 0 deletions internal/scaffold.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var scaffoldFuncs = scaffolder.FuncMap{
"screamingSnake": strcase.ToUpperSnake,
"camel": strcase.ToUpperCamel,
"lowerCamel": strcase.ToLowerCamel,
"strippedCamel": strcase.ToUpperStrippedCamel,
"kebab": strcase.ToLowerKebab,
"screamingKebab": strcase.ToUpperKebab,
"upper": strings.ToUpper,
Expand Down
2 changes: 1 addition & 1 deletion lsp/hoveritems.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lsp/markdown/completion/pubSubTopic.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Declare a PubSub topic.

```go
var invoicesTopic = ftl.Topic[Invoice]("invoices")
var Invoices = ftl.Topic[Invoice]("invoices")
```

See https://tbd54566975.github.io/ftl/docs/reference/pubsub/
Expand Down

0 comments on commit c3c7aa6

Please sign in to comment.