refactor(gateway): make CancelController operate on entities (#360)

## Summary
Remove proto coupling from CancelController.Cancel by changing its
signature
from (*pb.CancelRequest) (*pb.CancelResponse, error) to
(entity.CancelRequest) error.
The empty CancelResponse is now constructed at the handler layer.

Add mapper.ProtoToCancelRequest in the mapper subpackage and wire it
through
GatewayServer.Cancel. Rename entity/cancel_request.go to
entity/cancel.go for
consistency with entity/land.go.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

## Test Plan


## Issues


## Stack
1. #359
1. @ #360
1. #363
1. #361

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: ubettigole

service/submitqueue/gateway/server/main.go

@@ -77,9 +77,13 @@ func (s *GatewayServer) Land(ctx context.Context, req *pb.LandRequest) (*pb.Land
 	return &pb.LandResponse{Sqid: result.ID}, nil
 }
 
-// Cancel delegates to the controller
+// Cancel maps the wire request to an entity, delegates to the controller, and
+// returns an empty response on success.
 func (s *GatewayServer) Cancel(ctx context.Context, req *pb.CancelRequest) (*pb.CancelResponse, error) {
-	return s.cancelController.Cancel(ctx, req)
+	if err := s.cancelController.Cancel(ctx, mapper.ProtoToCancelRequest(req)); err != nil {
+		return nil, err
+	}
+	return &pb.CancelResponse{}, nil
 }
 
 // Status delegates to the controller

service/submitqueue/gateway/server/mapper/BUILD.bazel

@@ -2,7 +2,10 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
-    srcs = ["land.go"],
+    srcs = [
+        "cancel.go",
+        "land.go",
+    ],
     importpath = "github.com/uber/submitqueue/service/submitqueue/gateway/server/mapper",
     visibility = ["//visibility:public"],
     deps = [
@@ -16,7 +19,10 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["land_test.go"],
+    srcs = [
+        "cancel_test.go",
+        "land_test.go",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//api/base/change/protopb:go_default_library",

service/submitqueue/gateway/server/mapper/cancel.go

@@ -0,0 +1,29 @@
+// Copyright (c) 2025 Uber Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package mapper
+
+import (
+	pb "github.com/uber/submitqueue/api/submitqueue/gateway/protopb"
+	"github.com/uber/submitqueue/submitqueue/entity"
+)
+
+// ProtoToCancelRequest maps the wire CancelRequest to the entity.CancelRequest
+// the controller operates on.
+func ProtoToCancelRequest(req *pb.CancelRequest) entity.CancelRequest {
+	return entity.CancelRequest{
+		ID:     req.GetSqid(),
+		Reason: req.GetReason(),
+	}
+}

service/submitqueue/gateway/server/mapper/cancel_test.go

@@ -0,0 +1,54 @@
+// Copyright (c) 2025 Uber Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package mapper
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	pb "github.com/uber/submitqueue/api/submitqueue/gateway/protopb"
+	"github.com/uber/submitqueue/submitqueue/entity"
+)
+
+func TestProtoToCancelRequest(t *testing.T) {
+	tests := []struct {
+		name     string
+		req      *pb.CancelRequest
+		expected entity.CancelRequest
+	}{
+		{
+			name:     "maps sqid and reason",
+			req:      &pb.CancelRequest{Sqid: "test-queue/42", Reason: "obsolete change"},
+			expected: entity.CancelRequest{ID: "test-queue/42", Reason: "obsolete change"},
+		},
+		{
+			name:     "maps sqid without reason",
+			req:      &pb.CancelRequest{Sqid: "test-queue/1"},
+			expected: entity.CancelRequest{ID: "test-queue/1"},
+		},
+		{
+			name:     "empty request yields zero value",
+			req:      &pb.CancelRequest{},
+			expected: entity.CancelRequest{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ProtoToCancelRequest(tt.req)
+			assert.Equal(t, tt.expected, got)
+		})
+	}
+}

submitqueue/entity/BUILD.bazel

@@ -7,7 +7,7 @@ go_library(
         "batch_changes.go",
         "batch_dependent.go",
         "build.go",
-        "cancel_request.go",
+        "cancel.go",
         "change_provider.go",
         "change_record.go",
         "conflict.go",
@@ -35,7 +35,7 @@ go_test(
     srcs = [
         "batch_test.go",
         "build_test.go",
-        "cancel_request_test.go",
+        "cancel_test.go",
         "land_test.go",
         "queue_test.go",
         "request_log_test.go",

submitqueue/entity/cancel_request.go submitqueue/entity/cancel.gosubmitqueue/entity/cancel_request.go renamed to submitqueue/entity/cancel.go

@@ -20,7 +20,6 @@ import (
 	"time"
 
 	"github.com/uber-go/tally"
-	pb "github.com/uber/submitqueue/api/submitqueue/gateway/protopb"
 	entityqueue "github.com/uber/submitqueue/platform/base/messagequeue"
 	"github.com/uber/submitqueue/platform/consumer"
 	"github.com/uber/submitqueue/platform/errs"
@@ -62,26 +61,21 @@ func NewCancelController(logger *zap.SugaredLogger, scope tally.Scope, requestLo
 // completion before the cancel propagates may still land. The RequestStatusCancelling
 // entry written here records the user's intent; the terminal outcome is reflected by a
 // later RequestStatusCancelled (orchestrator side) or RequestStatusLanded entry.
-func (c *CancelController) Cancel(ctx context.Context, req *pb.CancelRequest) (*pb.CancelResponse, error) {
+func (c *CancelController) Cancel(ctx context.Context, req entity.CancelRequest) error {
 	start := time.Now()
 	defer func() {
 		c.metricsScope.Timer("cancel_request_latency").Record(time.Since(start))
 	}()
 
 	c.metricsScope.Counter("cancel_request_count").Inc(1)
 
-	if req.Sqid == "" {
-		return nil, fmt.Errorf("CancelController requires the request to have a sqid specified: %w", ErrInvalidRequest)
-	}
-
-	cancelRequest := entity.CancelRequest{
-		ID:     req.Sqid,
-		Reason: req.Reason,
+	if req.ID == "" {
+		return fmt.Errorf("CancelController requires the request to have a sqid specified: %w", ErrInvalidRequest)
 	}
 
 	c.logger.Debugw("cancel request received",
-		"sqid", cancelRequest.ID,
-		"reason", cancelRequest.Reason,
+		"sqid", req.ID,
+		"reason", req.Reason,
 	)
 
 	// Verify the sqid exists before recording intent or publishing. Cancel is opt-in
@@ -90,37 +84,37 @@ func (c *CancelController) Cancel(ctx context.Context, req *pb.CancelRequest) (*
 	// controller writes its "accepted" log entry synchronously to the same store, so
 	// a NotFound here reliably means "this sqid was never accepted by the gateway"
 	// rather than "in flight" — there is no false-negative race window.
-	if _, err := c.requestLogStore.List(ctx, cancelRequest.ID); err != nil {
+	if _, err := c.requestLogStore.List(ctx, req.ID); err != nil {
 		if storage.IsNotFound(err) {
 			c.metricsScope.Counter("cancel_request_not_found").Inc(1)
-			return nil, errs.NewUserError(&RequestNotFoundError{Sqid: cancelRequest.ID})
+			return errs.NewUserError(&RequestNotFoundError{Sqid: req.ID})
 		}
-		return nil, fmt.Errorf("CancelController failed to look up request log for sqid=%s: %w", cancelRequest.ID, err)
+		return fmt.Errorf("CancelController failed to look up request log for sqid=%s: %w", req.ID, err)
 	}
 
 	// Record the user's intent in the request log before publishing. Writing direct to the
 	// store (rather than via the log topic) keeps the gateway-emitted entry consistent with
 	// the Land "accepted" entry and guarantees the entry is visible the moment Cancel returns.
 	metadata := map[string]string{}
-	if cancelRequest.Reason != "" {
-		metadata["reason"] = cancelRequest.Reason
+	if req.Reason != "" {
+		metadata["reason"] = req.Reason
 	}
-	logEntry := entity.NewRequestLog(cancelRequest.ID, entity.RequestStatusCancelling, 0, "", metadata)
+	logEntry := entity.NewRequestLog(req.ID, entity.RequestStatusCancelling, 0, "", metadata)
 	if err := c.requestLogStore.Insert(ctx, logEntry); err != nil {
-		return nil, fmt.Errorf("CancelController failed to insert cancelling log for sqid=%s: %w", cancelRequest.ID, err)
+		return fmt.Errorf("CancelController failed to insert cancelling log for sqid=%s: %w", req.ID, err)
 	}
 
-	if err := c.publishToQueue(ctx, cancelRequest); err != nil {
-		return nil, fmt.Errorf("CancelController failed to publish cancel request to queue: %w", err)
+	if err := c.publishToQueue(ctx, req); err != nil {
+		return fmt.Errorf("CancelController failed to publish cancel request to queue: %w", err)
 	}
 
 	c.logger.Infow("cancel request published to queue",
-		"sqid", cancelRequest.ID,
+		"sqid", req.ID,
 		"topic_key", topickey.TopicKeyCancel,
 	)
 	c.metricsScope.Counter("cancel_publish_success").Inc(1)
 
-	return &pb.CancelResponse{}, nil
+	return nil
 }
 
 // publishToQueue publishes a cancel request to the cancel queue for async processing.

submitqueue/entity/cancel_request_test.go submitqueue/entity/cancel_test.gosubmitqueue/entity/cancel_request_test.go renamed to submitqueue/entity/cancel_test.go

@@ -22,7 +22,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/uber-go/tally"
-	pb "github.com/uber/submitqueue/api/submitqueue/gateway/protopb"
 	entityqueue "github.com/uber/submitqueue/platform/base/messagequeue"
 	"github.com/uber/submitqueue/platform/consumer"
 	"github.com/uber/submitqueue/platform/errs"
@@ -68,6 +67,11 @@ func newRequestLogStoreNoop(t *testing.T, ctrl *gomock.Controller) *storagemock.
 	return store
 }
 
+// testCancelRequest returns a valid entity.CancelRequest for testing.
+func testCancelRequest(sqid string, reason string) entity.CancelRequest {
+	return entity.CancelRequest{ID: sqid, Reason: reason}
+}
+
 func TestNewCancelController(t *testing.T) {
 	ctrl := gomock.NewController(t)
 
@@ -81,11 +85,9 @@ func TestCancel_HappyPath(t *testing.T) {
 	controller := NewCancelController(zap.NewNop().Sugar(), tally.NoopScope, newRequestLogStoreNoop(t, ctrl), newCancelTestRegistryWithNoopPublisher(t, ctrl))
 	ctx := context.Background()
 
-	req := &pb.CancelRequest{Sqid: "test-queue/42", Reason: "user changed their mind"}
-	resp, err := controller.Cancel(ctx, req)
+	err := controller.Cancel(ctx, testCancelRequest("test-queue/42", "user changed their mind"))
 
 	require.NoError(t, err)
-	require.NotNil(t, resp)
 }
 
 func TestCancel_ReturnsErrorOnEmptySqid(t *testing.T) {
@@ -94,8 +96,7 @@ func TestCancel_ReturnsErrorOnEmptySqid(t *testing.T) {
 	controller := NewCancelController(zap.NewNop().Sugar(), tally.NoopScope, newRequestLogStoreNoop(t, ctrl), newCancelTestRegistryWithNoopPublisher(t, ctrl))
 	ctx := context.Background()
 
-	req := &pb.CancelRequest{Sqid: "", Reason: "anything"}
-	_, err := controller.Cancel(ctx, req)
+	err := controller.Cancel(ctx, testCancelRequest("", "anything"))
 
 	require.Error(t, err)
 	assert.True(t, IsInvalidRequest(err))
@@ -119,8 +120,7 @@ func TestCancel_PublishesToQueue(t *testing.T) {
 	controller := NewCancelController(zap.NewNop().Sugar(), tally.NoopScope, newRequestLogStoreNoop(t, ctrl), registry)
 	ctx := context.Background()
 
-	req := &pb.CancelRequest{Sqid: "my-queue/7", Reason: "obsolete change"}
-	_, err := controller.Cancel(ctx, req)
+	err := controller.Cancel(ctx, testCancelRequest("my-queue/7", "obsolete change"))
 	require.NoError(t, err)
 
 	assert.Equal(t, "cancel", publishedTopic)
@@ -160,8 +160,7 @@ func TestCancel_InsertsCancellingLog(t *testing.T) {
 
 	controller := NewCancelController(zap.NewNop().Sugar(), tally.NoopScope, logStore, registry)
 
-	req := &pb.CancelRequest{Sqid: "my-queue/42", Reason: "obsolete change"}
-	_, err := controller.Cancel(context.Background(), req)
+	err := controller.Cancel(context.Background(), testCancelRequest("my-queue/42", "obsolete change"))
 	require.NoError(t, err)
 
 	assert.Equal(t, "my-queue/42", insertedLog.RequestID)
@@ -180,11 +179,10 @@ func TestCancel_LogInsertFailure(t *testing.T) {
 	logStore.EXPECT().Insert(gomock.Any(), gomock.Any()).Return(fmt.Errorf("db unavailable"))
 
 	registry, publisher := newCancelTestRegistry(t, ctrl)
-	// No Publish expectation: log insert must fail before publish runs.
 	_ = publisher
 
 	controller := NewCancelController(zap.NewNop().Sugar(), tally.NoopScope, logStore, registry)
-	_, err := controller.Cancel(context.Background(), &pb.CancelRequest{Sqid: "q/1"})
+	err := controller.Cancel(context.Background(), testCancelRequest("q/1", ""))
 	require.Error(t, err)
 }
 
@@ -197,8 +195,7 @@ func TestCancel_ReturnsErrorOnPublishFailure(t *testing.T) {
 	controller := NewCancelController(zap.NewNop().Sugar(), tally.NoopScope, newRequestLogStoreNoop(t, ctrl), registry)
 	ctx := context.Background()
 
-	req := &pb.CancelRequest{Sqid: "test-queue/1"}
-	_, err := controller.Cancel(ctx, req)
+	err := controller.Cancel(ctx, testCancelRequest("test-queue/1", ""))
 
 	require.Error(t, err)
 }
@@ -211,14 +208,12 @@ func TestCancel_UnknownSqidIsUserError(t *testing.T) {
 
 	logStore := storagemock.NewMockRequestLogStore(ctrl)
 	logStore.EXPECT().List(gomock.Any(), "ghost/1").Return(nil, storage.ErrNotFound)
-	// No Insert expectation: existence check must short-circuit before Insert.
 
 	registry, publisher := newCancelTestRegistry(t, ctrl)
-	// No Publish expectation: existence check must short-circuit before Publish.
 	_ = publisher
 
 	controller := NewCancelController(zap.NewNop().Sugar(), tally.NoopScope, logStore, registry)
-	_, err := controller.Cancel(context.Background(), &pb.CancelRequest{Sqid: "ghost/1"})
+	err := controller.Cancel(context.Background(), testCancelRequest("ghost/1", ""))
 	require.Error(t, err)
 	assert.True(t, IsRequestNotFound(err))
 	assert.True(t, errs.IsUserError(err))
@@ -242,7 +237,7 @@ func TestCancel_RequestLogLookupFailure(t *testing.T) {
 	_ = publisher
 
 	controller := NewCancelController(zap.NewNop().Sugar(), tally.NoopScope, logStore, registry)
-	_, err := controller.Cancel(context.Background(), &pb.CancelRequest{Sqid: "q/1"})
+	err := controller.Cancel(context.Background(), testCancelRequest("q/1", ""))
 	require.Error(t, err)
 	assert.False(t, errs.IsUserError(err))
 	assert.False(t, IsRequestNotFound(err))