From da693726b2110659e1f9d14eb74fcd9222bcf01d Mon Sep 17 00:00:00 2001
From: Stuart Douglas <stuart.w.douglas@gmail.com>
Date: Wed, 14 Aug 2024 11:54:23 +1000
Subject: [PATCH] fix: also require camel case subscription names (#2342)

This makes naming more consistent.
---
 backend/controller/pubsub/integration_test.go    |  8 ++++----
 .../pubsub/testdata/go/subscriber/subscriber.go  | 16 +++++++++-------
 go-runtime/schema/subscription/analyzer.go       |  8 +++++++-
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/backend/controller/pubsub/integration_test.go b/backend/controller/pubsub/integration_test.go
index 9438a3d2ba..bc8dbdd0b9 100644
--- a/backend/controller/pubsub/integration_test.go
+++ b/backend/controller/pubsub/integration_test.go
@@ -34,7 +34,7 @@ func TestPubSub(t *testing.T) {
 				WHERE
 					state = 'success'
 					AND origin = '%s'
-		`, dal.AsyncOriginPubSub{Subscription: schema.RefKey{Module: "subscriber", Name: "test_subscription"}}.String()),
+		`, dal.AsyncOriginPubSub{Subscription: schema.RefKey{Module: "subscriber", Name: "testSubscription"}}.String()),
 			events),
 	)
 }
@@ -103,7 +103,7 @@ func TestRetry(t *testing.T) {
 					state = 'error'
 					AND catching = false
 					AND origin = '%s'
-		`, dal.AsyncOriginPubSub{Subscription: schema.RefKey{Module: "subscriber", Name: "doomed_subscription"}}.String()),
+		`, dal.AsyncOriginPubSub{Subscription: schema.RefKey{Module: "subscriber", Name: "doomedSubscription"}}.String()),
 			1+retriesPerCall),
 
 		// check that there is one failed attempt to catch (we purposely fail the first one)
@@ -116,7 +116,7 @@ func TestRetry(t *testing.T) {
 				AND error = 'call to verb subscriber.catch failed: catching error'
 				AND catching = true
 				AND origin = '%s'
-	`, dal.AsyncOriginPubSub{Subscription: schema.RefKey{Module: "subscriber", Name: "doomed_subscription"}}.String()),
+	`, dal.AsyncOriginPubSub{Subscription: schema.RefKey{Module: "subscriber", Name: "doomedSubscription"}}.String()),
 			1),
 
 		// check that there is one successful attempt to catch (we succeed the second one as long as we receive the correct error in the request)
@@ -129,7 +129,7 @@ func TestRetry(t *testing.T) {
 			AND error IS NULL
 			AND catching = true
 			AND origin = '%s'
-`, dal.AsyncOriginPubSub{Subscription: schema.RefKey{Module: "subscriber", Name: "doomed_subscription"}}.String()),
+`, dal.AsyncOriginPubSub{Subscription: schema.RefKey{Module: "subscriber", Name: "doomedSubscription"}}.String()),
 			1),
 	)
 }
diff --git a/backend/controller/pubsub/testdata/go/subscriber/subscriber.go b/backend/controller/pubsub/testdata/go/subscriber/subscriber.go
index 068671661a..bc719e49e4 100644
--- a/backend/controller/pubsub/testdata/go/subscriber/subscriber.go
+++ b/backend/controller/pubsub/testdata/go/subscriber/subscriber.go
@@ -3,30 +3,32 @@ package subscriber
 import (
 	"context"
 	"fmt"
-	"ftl/builtin"
-	"ftl/publisher"
 	"strings"
 	"time"
 
-	"github.com/TBD54566975/ftl/go-runtime/ftl" // Import the FTL SDK.
+	"ftl/builtin"
+	"ftl/publisher"
+
 	"github.com/alecthomas/atomic"
+
+	"github.com/TBD54566975/ftl/go-runtime/ftl" // Import the FTL SDK.
 )
 
-var _ = ftl.Subscription(publisher.TestTopic, "test_subscription")
+var _ = ftl.Subscription(publisher.TestTopic, "testSubscription")
 
 var catchCount atomic.Value[int]
 
 //ftl:verb
-//ftl:subscribe test_subscription
+//ftl:subscribe testSubscription
 func Consume(ctx context.Context, req publisher.PubSubEvent) error {
 	ftl.LoggerFromContext(ctx).Infof("Subscriber is consuming %v", req.Time)
 	return nil
 }
 
-var _ = ftl.Subscription(publisher.Topic2, "doomed_subscription")
+var _ = ftl.Subscription(publisher.Topic2, "doomedSubscription")
 
 //ftl:verb
-//ftl:subscribe doomed_subscription
+//ftl:subscribe doomedSubscription
 //ftl:retry 2 1s 1s catch catch
 func ConsumeButFailAndRetry(ctx context.Context, req publisher.PubSubEvent) error {
 	return fmt.Errorf("always error: event %v", req.Time)
diff --git a/go-runtime/schema/subscription/analyzer.go b/go-runtime/schema/subscription/analyzer.go
index d4c4472be3..84dc481be6 100644
--- a/go-runtime/schema/subscription/analyzer.go
+++ b/go-runtime/schema/subscription/analyzer.go
@@ -73,9 +73,15 @@ func Extract(pass *analysis.Pass, obj types.Object, node *ast.GenDecl, callExpr
 		return optional.None[*schema.Subscription]()
 	}
 
+	subName := common.ExtractStringLiteralArg(pass, callExpr, 1)
+	expSubName := strcase.ToLowerCamel(subName)
+	if subName != expSubName {
+		common.Errorf(pass, node, "unsupported subscription name %q, did you mean to use %q?", subName, expSubName)
+		return optional.None[*schema.Subscription]()
+	}
 	subscription := &schema.Subscription{
 		Pos:   common.GoPosToSchemaPos(pass.Fset, callExpr.Pos()),
-		Name:  common.ExtractStringLiteralArg(pass, callExpr, 1),
+		Name:  subName,
 		Topic: topicRef,
 	}
 	common.ApplyMetadata[*schema.Subscription](pass, obj, func(md *common.ExtractedMetadata) {