Skip to content

Commit ae46190

Browse files
ubettigoleclaude
andauthored
refactor(gateway): make StatusController operate on entities (#363)
## Summary Remove proto coupling from StatusController.Status by changing its signature to accept entity.StatusRequest and return request.CurrentState directly. Add mapper.ProtoToStatusRequest and mapper.CurrentStateToProto in the mapper subpackage. The Status mapper is the first with a non-trivial entity-to-proto direction (translating the CurrentState read model into pb.StatusResponse). Add entity.StatusRequest{ID} for input consistency with the other controllers. 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>
1 parent 500bb73 commit ae46190

8 files changed

Lines changed: 171 additions & 24 deletions

File tree

service/submitqueue/gateway/server/main.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,14 @@ func (s *GatewayServer) Cancel(ctx context.Context, req *pb.CancelRequest) (*pb.
8686
return &pb.CancelResponse{}, nil
8787
}
8888

89-
// Status delegates to the controller
89+
// Status maps the wire request to an entity, delegates to the controller, and
90+
// maps the read-model result back to the wire response.
9091
func (s *GatewayServer) Status(ctx context.Context, req *pb.StatusRequest) (*pb.StatusResponse, error) {
91-
return s.statusController.Status(ctx, req)
92+
state, err := s.statusController.Status(ctx, mapper.ProtoToStatusRequest(req))
93+
if err != nil {
94+
return nil, err
95+
}
96+
return mapper.CurrentStateToProto(state), nil
9297
}
9398

9499
func main() {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go_library(
55
srcs = [
66
"cancel.go",
77
"land.go",
8+
"status.go",
89
],
910
importpath = "github.com/uber/submitqueue/service/submitqueue/gateway/server/mapper",
1011
visibility = ["//visibility:public"],
@@ -13,6 +14,7 @@ go_library(
1314
"//api/submitqueue/gateway/protopb:go_default_library",
1415
"//platform/base/change:go_default_library",
1516
"//platform/base/mergestrategy:go_default_library",
17+
"//submitqueue/core/request:go_default_library",
1618
"//submitqueue/entity:go_default_library",
1719
],
1820
)
@@ -22,6 +24,7 @@ go_test(
2224
srcs = [
2325
"cancel_test.go",
2426
"land_test.go",
27+
"status_test.go",
2528
],
2629
embed = [":go_default_library"],
2730
deps = [
@@ -30,6 +33,7 @@ go_test(
3033
"//api/submitqueue/gateway/protopb:go_default_library",
3134
"//platform/base/change:go_default_library",
3235
"//platform/base/mergestrategy:go_default_library",
36+
"//submitqueue/core/request:go_default_library",
3337
"//submitqueue/entity:go_default_library",
3438
"@com_github_stretchr_testify//assert:go_default_library",
3539
"@com_github_stretchr_testify//require:go_default_library",
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package mapper
16+
17+
import (
18+
pb "github.com/uber/submitqueue/api/submitqueue/gateway/protopb"
19+
"github.com/uber/submitqueue/submitqueue/core/request"
20+
"github.com/uber/submitqueue/submitqueue/entity"
21+
)
22+
23+
// ProtoToStatusRequest maps the wire StatusRequest to the entity.StatusRequest
24+
// the controller operates on.
25+
func ProtoToStatusRequest(req *pb.StatusRequest) entity.StatusRequest {
26+
return entity.StatusRequest{
27+
ID: req.GetSqid(),
28+
}
29+
}
30+
31+
// CurrentStateToProto maps the domain read model to the wire StatusResponse.
32+
func CurrentStateToProto(state request.CurrentState) *pb.StatusResponse {
33+
return &pb.StatusResponse{
34+
Status: string(state.Status),
35+
LastError: state.LastError,
36+
Metadata: state.Metadata,
37+
}
38+
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package mapper
16+
17+
import (
18+
"testing"
19+
20+
"github.com/stretchr/testify/assert"
21+
pb "github.com/uber/submitqueue/api/submitqueue/gateway/protopb"
22+
"github.com/uber/submitqueue/submitqueue/core/request"
23+
"github.com/uber/submitqueue/submitqueue/entity"
24+
)
25+
26+
func TestProtoToStatusRequest(t *testing.T) {
27+
tests := []struct {
28+
name string
29+
req *pb.StatusRequest
30+
expected entity.StatusRequest
31+
}{
32+
{
33+
name: "maps sqid to ID",
34+
req: &pb.StatusRequest{Sqid: "test-queue/42"},
35+
expected: entity.StatusRequest{ID: "test-queue/42"},
36+
},
37+
{
38+
name: "empty request yields zero value",
39+
req: &pb.StatusRequest{},
40+
expected: entity.StatusRequest{},
41+
},
42+
}
43+
44+
for _, tt := range tests {
45+
t.Run(tt.name, func(t *testing.T) {
46+
got := ProtoToStatusRequest(tt.req)
47+
assert.Equal(t, tt.expected, got)
48+
})
49+
}
50+
}
51+
52+
func TestCurrentStateToProto(t *testing.T) {
53+
tests := []struct {
54+
name string
55+
state request.CurrentState
56+
expected *pb.StatusResponse
57+
}{
58+
{
59+
name: "maps all fields",
60+
state: request.CurrentState{
61+
Status: entity.RequestStatusValidating,
62+
LastError: "validation failed",
63+
Metadata: map[string]string{"step": "lint"},
64+
},
65+
expected: &pb.StatusResponse{
66+
Status: string(entity.RequestStatusValidating),
67+
LastError: "validation failed",
68+
Metadata: map[string]string{"step": "lint"},
69+
},
70+
},
71+
{
72+
name: "zero value state maps to empty response",
73+
state: request.CurrentState{},
74+
expected: &pb.StatusResponse{
75+
Status: "",
76+
},
77+
},
78+
}
79+
80+
for _, tt := range tests {
81+
t.Run(tt.name, func(t *testing.T) {
82+
got := CurrentStateToProto(tt.state)
83+
assert.Equal(t, tt.expected, got)
84+
})
85+
}
86+
}

submitqueue/entity/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ go_library(
2121
"request_summary.go",
2222
"speculation_path_build.go",
2323
"speculation_tree.go",
24+
"status.go",
2425
],
2526
importpath = "github.com/uber/submitqueue/submitqueue/entity",
2627
visibility = ["//visibility:public"],

submitqueue/entity/status.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) 2025 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package entity
16+
17+
// StatusRequest identifies a request whose current status is being queried.
18+
type StatusRequest struct {
19+
// ID is the globally unique identifier of the request. Format: "<queue>/<counter_value>".
20+
ID string
21+
}

submitqueue/gateway/controller/status.go

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ import (
2121
"time"
2222

2323
"github.com/uber-go/tally"
24-
pb "github.com/uber/submitqueue/api/submitqueue/gateway/protopb"
2524
"github.com/uber/submitqueue/platform/errs"
2625
"github.com/uber/submitqueue/submitqueue/core/request"
26+
"github.com/uber/submitqueue/submitqueue/entity"
2727
"github.com/uber/submitqueue/submitqueue/extension/storage"
2828
"go.uber.org/zap"
2929
)
@@ -64,34 +64,30 @@ func NewStatusController(logger *zap.SugaredLogger, scope tally.Scope, requestLo
6464
}
6565

6666
// Status returns the current reconciled status of a request identified by its sqid.
67-
func (c *StatusController) Status(ctx context.Context, req *pb.StatusRequest) (*pb.StatusResponse, error) {
67+
func (c *StatusController) Status(ctx context.Context, req entity.StatusRequest) (request.CurrentState, error) {
6868
start := time.Now()
6969
defer func() {
7070
c.metricsScope.Timer("status_latency").Record(time.Since(start))
7171
}()
7272

7373
c.metricsScope.Counter("status_count").Inc(1)
7474

75-
if req.Sqid == "" {
76-
return nil, fmt.Errorf("StatusController requires the request to have a sqid specified: %w", ErrInvalidRequest)
75+
if req.ID == "" {
76+
return request.CurrentState{}, fmt.Errorf("StatusController requires the request to have a sqid specified: %w", ErrInvalidRequest)
7777
}
7878

79-
state, err := request.GetCurrentStateFromRequestLog(ctx, c.requestLogStore, req.Sqid)
79+
state, err := request.GetCurrentStateFromRequestLog(ctx, c.requestLogStore, req.ID)
8080
if err != nil {
8181
if storage.IsNotFound(err) {
82-
return nil, errs.NewUserError(&RequestNotFoundError{Sqid: req.Sqid})
82+
return request.CurrentState{}, errs.NewUserError(&RequestNotFoundError{Sqid: req.ID})
8383
}
84-
return nil, fmt.Errorf("StatusController failed to get current state for sqid=%s: %w", req.Sqid, err)
84+
return request.CurrentState{}, fmt.Errorf("StatusController failed to get current state for sqid=%s: %w", req.ID, err)
8585
}
8686

8787
c.logger.Debugw("request status retrieved",
88-
"sqid", req.Sqid,
88+
"sqid", req.ID,
8989
"status", string(state.Status),
9090
)
9191

92-
return &pb.StatusResponse{
93-
Status: string(state.Status),
94-
LastError: state.LastError,
95-
Metadata: state.Metadata,
96-
}, nil
92+
return state, nil
9793
}

submitqueue/gateway/controller/status_test.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"github.com/stretchr/testify/assert"
2323
"github.com/stretchr/testify/require"
2424
"github.com/uber-go/tally"
25-
pb "github.com/uber/submitqueue/api/submitqueue/gateway/protopb"
2625
"github.com/uber/submitqueue/platform/errs"
2726
"github.com/uber/submitqueue/submitqueue/entity"
2827
"github.com/uber/submitqueue/submitqueue/extension/storage"
@@ -42,12 +41,12 @@ func TestStatus_ReturnsCurrentState(t *testing.T) {
4241

4342
controller := NewStatusController(zap.NewNop().Sugar(), tally.NoopScope, store)
4443

45-
resp, err := controller.Status(context.Background(), &pb.StatusRequest{Sqid: "test-queue/1"})
44+
state, err := controller.Status(context.Background(), entity.StatusRequest{ID: "test-queue/1"})
4645

4746
require.NoError(t, err)
48-
assert.Equal(t, string(entity.RequestStatusValidating), resp.Status)
49-
assert.Equal(t, "boom", resp.LastError)
50-
assert.Equal(t, map[string]string{"k": "v"}, resp.Metadata)
47+
assert.Equal(t, entity.RequestStatusValidating, state.Status)
48+
assert.Equal(t, "boom", state.LastError)
49+
assert.Equal(t, map[string]string{"k": "v"}, state.Metadata)
5150
}
5251

5352
func TestStatus_Errors(t *testing.T) {
@@ -93,12 +92,9 @@ func TestStatus_Errors(t *testing.T) {
9392

9493
controller := NewStatusController(zap.NewNop().Sugar(), tally.NoopScope, store)
9594

96-
_, err := controller.Status(context.Background(), &pb.StatusRequest{Sqid: tt.sqid})
95+
_, err := controller.Status(context.Background(), entity.StatusRequest{ID: tt.sqid})
9796

9897
require.Error(t, err)
99-
// IsRequestNotFound is a precise type check; IsInvalidRequest matches any
100-
// user error (see errs.userError.Is), so only assert it where it is the
101-
// defining classification.
10298
assert.Equal(t, tt.wantNotFound, IsRequestNotFound(err))
10399
assert.Equal(t, tt.wantUserError, errs.IsUserError(err))
104100
assert.False(t, errs.IsRetryable(err))

0 commit comments

Comments
 (0)