Skip to content

Commit 6491c8a

Browse files
committed
refactor(entity): promote RequestLandStrategy to shared entity/mergestrategy
## Summary ### Why? The land-merge strategy enum lived in `submitqueue/entity` as `RequestLandStrategy`, but the upcoming runway merge-conflict contract needs the same type, and runway must not import `submitqueue/entity`. Mirroring the `Change` promotion in #240, the strategy belongs in the shared top-level `entity/` so both domains reference one type with no per-domain enum or mapping. ### What? Moves `RequestLandStrategy` and its constants out of `submitqueue/entity/request.go` into a new top-level `entity/mergestrategy` package as `MergeStrategy` (`MergeStrategyUnknown/Rebase/SquashRebase/Merge`). `Request.LandStrategy` now has type `mergestrategy.MergeStrategy`. All references across the gateway and orchestrator controllers, entities, and tests are updated to the shared type. Pure mechanical refactor — no behavior change. ## Test Plan ✅ `bazel build //...` ✅ `bazel test //... --test_tag_filters=-integration,-e2e` (51 tests pass) ✅ `make gazelle` clean
1 parent 3e75dd9 commit 6491c8a

18 files changed

Lines changed: 108 additions & 62 deletions

File tree

entity/mergestrategy/BUILD.bazel

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
load("@rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "mergestrategy",
5+
srcs = ["mergestrategy.go"],
6+
importpath = "github.com/uber/submitqueue/entity/mergestrategy",
7+
visibility = ["//visibility:public"],
8+
)
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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 mergestrategy holds the shared source-control integration strategy
16+
// used across SubmitQueue, runway, and other repo-local domains. It names how a
17+
// change is integrated into the target branch (rebase, squash-rebase, merge).
18+
package mergestrategy
19+
20+
// MergeStrategy defines the possible source control integration methods.
21+
type MergeStrategy string
22+
23+
const (
24+
// MergeStrategyUnknown is the unknown strategy. It is set by default when the structure is initialized. It should never be seen in the system and is used for error control.
25+
MergeStrategyUnknown MergeStrategy = ""
26+
// MergeStrategyRebase rebases commits onto the target branch before landing.
27+
MergeStrategyRebase MergeStrategy = "rebase"
28+
// MergeStrategySquashRebase squashes commits into a single commit before rebasing on top of the target branch.
29+
MergeStrategySquashRebase MergeStrategy = "squash_rebase"
30+
// MergeStrategyMerge merges commits into the target branch by creating a separate merge commit, preserving the commit history along with hashes.
31+
MergeStrategyMerge MergeStrategy = "merge"
32+
)

submitqueue/entity/BUILD.bazel

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ go_library(
2121
],
2222
importpath = "github.com/uber/submitqueue/submitqueue/entity",
2323
visibility = ["//visibility:public"],
24-
deps = ["//entity/change"],
24+
deps = [
25+
"//entity/change",
26+
"//entity/mergestrategy",
27+
],
2528
)
2629

2730
go_test(
@@ -37,6 +40,7 @@ go_test(
3740
embed = [":entity"],
3841
deps = [
3942
"//entity/change",
43+
"//entity/mergestrategy",
4044
"@com_github_stretchr_testify//assert",
4145
"@com_github_stretchr_testify//require",
4246
],

submitqueue/entity/land_request.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"encoding/json"
1919

2020
"github.com/uber/submitqueue/entity/change"
21+
"github.com/uber/submitqueue/entity/mergestrategy"
2122
)
2223

2324
// LandRequest represents the gateway-owned fields of a land request sent over the queue
@@ -31,7 +32,7 @@ type LandRequest struct {
3132
// Change is the set of code changes to land.
3233
Change change.Change `json:"change"`
3334
// LandStrategy is the source control integration strategy to use for this land operation.
34-
LandStrategy RequestLandStrategy `json:"land_strategy"`
35+
LandStrategy mergestrategy.MergeStrategy `json:"land_strategy"`
3536
}
3637

3738
// ToBytes serializes the LandRequest to JSON bytes for queue message payload.

submitqueue/entity/land_request_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/stretchr/testify/assert"
2121
"github.com/stretchr/testify/require"
2222
"github.com/uber/submitqueue/entity/change"
23+
"github.com/uber/submitqueue/entity/mergestrategy"
2324
)
2425

2526
func TestLandRequest_ToBytes(t *testing.T) {
@@ -30,7 +31,7 @@ func TestLandRequest_ToBytes(t *testing.T) {
3031
"github://uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01",
3132
"github://uber/submitqueue/pull/789/0123456789abcdef0123456789abcdef01234567",
3233
}},
33-
LandStrategy: RequestLandStrategyRebase,
34+
LandStrategy: mergestrategy.MergeStrategyRebase,
3435
}
3536

3637
data, err := req.ToBytes()
@@ -49,7 +50,7 @@ func TestLandRequestFromBytes(t *testing.T) {
4950
ID: "my-queue/999",
5051
Queue: "my-queue",
5152
Change: change.Change{URIs: []string{"code.uber.internal.com/D111"}},
52-
LandStrategy: RequestLandStrategyMerge,
53+
LandStrategy: mergestrategy.MergeStrategyMerge,
5354
}
5455

5556
// Serialize
@@ -83,7 +84,7 @@ func TestLandRequestFromBytes_EmptyData(t *testing.T) {
8384
// Empty JSON should deserialize with zero values
8485
assert.Empty(t, req.ID)
8586
assert.Empty(t, req.Queue)
86-
assert.Equal(t, RequestLandStrategyUnknown, req.LandStrategy)
87+
assert.Equal(t, mergestrategy.MergeStrategyUnknown, req.LandStrategy)
8788
}
8889

8990
func TestLandRequest_SerializationRoundTrip(t *testing.T) {
@@ -101,7 +102,7 @@ func TestLandRequest_SerializationRoundTrip(t *testing.T) {
101102
"github://uber/repo-a/pull/102/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
102103
"github://uber/repo-a/pull/103/cccccccccccccccccccccccccccccccccccccccc",
103104
}},
104-
LandStrategy: RequestLandStrategySquashRebase,
105+
LandStrategy: mergestrategy.MergeStrategySquashRebase,
105106
},
106107
},
107108
{
@@ -110,7 +111,7 @@ func TestLandRequest_SerializationRoundTrip(t *testing.T) {
110111
ID: "queue2/200",
111112
Queue: "queue2",
112113
Change: change.Change{URIs: []string{"code.uber.internal.com/D12345"}},
113-
LandStrategy: RequestLandStrategyRebase,
114+
LandStrategy: mergestrategy.MergeStrategyRebase,
114115
},
115116
},
116117
{
@@ -119,7 +120,7 @@ func TestLandRequest_SerializationRoundTrip(t *testing.T) {
119120
ID: "queue3/300",
120121
Queue: "queue3",
121122
Change: change.Change{URIs: []string{"github.uber.com/internal/service/999/deadbeef12"}},
122-
LandStrategy: RequestLandStrategyMerge,
123+
LandStrategy: mergestrategy.MergeStrategyMerge,
123124
},
124125
},
125126
}

submitqueue/entity/request.go

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,7 @@ import (
1818
"encoding/json"
1919

2020
"github.com/uber/submitqueue/entity/change"
21-
)
22-
23-
// RequestLandStrategy defines the possible source control integration methods.
24-
type RequestLandStrategy string
25-
26-
const (
27-
// RequestLandStrategyUnknown is the unknown strategy. It is set by default when the structure is initialized. It should never be seen in the system and used for error control.
28-
RequestLandStrategyUnknown RequestLandStrategy = ""
29-
// RequestLandStrategyRebase rebases commits onto the target branch before landing.
30-
RequestLandStrategyRebase RequestLandStrategy = "rebase"
31-
// RequestLandStrategySquashRebase squashes commits into a single commit before rebasing on top of the target branch.
32-
RequestLandStrategySquashRebase RequestLandStrategy = "squash_rebase"
33-
// RequestLandStrategyMerge merges commits into the target branch by creating a separate merge commit, preserving the commit history along with hashes.
34-
RequestLandStrategyMerge RequestLandStrategy = "merge"
21+
"github.com/uber/submitqueue/entity/mergestrategy"
3522
)
3623

3724
// RequestState defines the possible states of a land request. They are internal and used to implement a state machine. A separate RequestStatus type is used to track the customer-friendly status of a request.
@@ -96,7 +83,7 @@ type Request struct {
9683
// Change is a number of code changes (such as pull requests) to land into the target branch. Target branch is defined by the queue configuration.
9784
Change change.Change `json:"change"`
9885
// LandStrategy is the source control integration strategy to use for this land operation.
99-
LandStrategy RequestLandStrategy `json:"land_strategy"`
86+
LandStrategy mergestrategy.MergeStrategy `json:"land_strategy"`
10087

10188
// ****************
10289
// Following fields could be changed throughout the lifecycle of the request

submitqueue/entity/request_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/stretchr/testify/assert"
2121
"github.com/stretchr/testify/require"
2222
"github.com/uber/submitqueue/entity/change"
23+
"github.com/uber/submitqueue/entity/mergestrategy"
2324
)
2425

2526
func TestRequest_ToBytes(t *testing.T) {
@@ -30,7 +31,7 @@ func TestRequest_ToBytes(t *testing.T) {
3031
"github://uber/submitqueue/pull/456/abcdef0123456789abcdef0123456789abcdef01",
3132
"github://uber/submitqueue/pull/789/0123456789abcdef0123456789abcdef01234567",
3233
}},
33-
LandStrategy: RequestLandStrategyRebase,
34+
LandStrategy: mergestrategy.MergeStrategyRebase,
3435
State: RequestStateStarted,
3536
Version: 1,
3637
}
@@ -52,7 +53,7 @@ func TestRequestFromBytes(t *testing.T) {
5253
ID: "my-queue/999",
5354
Queue: "my-queue",
5455
Change: change.Change{URIs: []string{"code.uber.internal.com/D111"}},
55-
LandStrategy: RequestLandStrategyMerge,
56+
LandStrategy: mergestrategy.MergeStrategyMerge,
5657
State: RequestStateProcessing,
5758
Version: 3,
5859
}
@@ -91,7 +92,7 @@ func TestRequestFromBytes_EmptyData(t *testing.T) {
9192
assert.Empty(t, req.ID)
9293
assert.Empty(t, req.Queue)
9394
assert.Equal(t, RequestStateUnknown, req.State)
94-
assert.Equal(t, RequestLandStrategyUnknown, req.LandStrategy)
95+
assert.Equal(t, mergestrategy.MergeStrategyUnknown, req.LandStrategy)
9596
assert.Equal(t, int32(0), req.Version)
9697
}
9798

@@ -152,7 +153,7 @@ func TestRequest_SerializationRoundTrip(t *testing.T) {
152153
"github://uber/repo-a/pull/102/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
153154
"github://uber/repo-a/pull/103/cccccccccccccccccccccccccccccccccccccccc",
154155
}},
155-
LandStrategy: RequestLandStrategySquashRebase,
156+
LandStrategy: mergestrategy.MergeStrategySquashRebase,
156157
State: RequestStateLanded,
157158
Version: 5,
158159
},
@@ -163,7 +164,7 @@ func TestRequest_SerializationRoundTrip(t *testing.T) {
163164
ID: "queue2/200",
164165
Queue: "queue2",
165166
Change: change.Change{URIs: []string{"code.uber.internal.com/D12345"}},
166-
LandStrategy: RequestLandStrategyRebase,
167+
LandStrategy: mergestrategy.MergeStrategyRebase,
167168
State: RequestStateStarted,
168169
Version: 1,
169170
},
@@ -174,7 +175,7 @@ func TestRequest_SerializationRoundTrip(t *testing.T) {
174175
ID: "queue3/300",
175176
Queue: "queue3",
176177
Change: change.Change{URIs: []string{"github.uber.com/internal/service/999/deadbeef12"}},
177-
LandStrategy: RequestLandStrategyMerge,
178+
LandStrategy: mergestrategy.MergeStrategyMerge,
178179
State: RequestStateError,
179180
Version: 10,
180181
},

submitqueue/gateway/controller/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ go_library(
1515
"//core/errs",
1616
"//core/metrics",
1717
"//entity/change",
18+
"//entity/mergestrategy",
1819
"//entity/messagequeue",
1920
"//extension/counter",
2021
"//submitqueue/core/request",
@@ -40,6 +41,7 @@ go_test(
4041
deps = [
4142
"//core/consumer",
4243
"//core/errs",
44+
"//entity/mergestrategy",
4345
"//entity/messagequeue",
4446
"//extension/counter/mock",
4547
"//extension/messagequeue/mock",

submitqueue/gateway/controller/land.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/uber/submitqueue/core/errs"
2525
"github.com/uber/submitqueue/core/metrics"
2626
"github.com/uber/submitqueue/entity/change"
27+
"github.com/uber/submitqueue/entity/mergestrategy"
2728
entityqueue "github.com/uber/submitqueue/entity/messagequeue"
2829
"github.com/uber/submitqueue/extension/counter"
2930
"github.com/uber/submitqueue/submitqueue/core/topickey"
@@ -113,7 +114,7 @@ func (c *LandController) Land(ctx context.Context, req *pb.LandRequest) (resp *p
113114
}
114115

115116
// TODO: pass default queue land strategy to resolver function to process a default.
116-
strategy, err := resolveRequestLandStrategy(req.Strategy)
117+
strategy, err := resolveMergeStrategy(req.Strategy)
117118
if err != nil {
118119
return nil, fmt.Errorf("LandController failed to map strategy for queue=%s: %w", req.Queue, err)
119120
}
@@ -195,19 +196,19 @@ func (c *LandController) publishToQueue(ctx context.Context, landRequest entity.
195196
return nil
196197
}
197198

198-
// protoStrategyToEntity maps a proto Strategy enum to the entity RequestLandStrategy.
199-
func resolveRequestLandStrategy(s pb.Strategy) (entity.RequestLandStrategy, error) {
199+
// resolveMergeStrategy maps a proto Strategy enum to the shared mergestrategy.MergeStrategy.
200+
func resolveMergeStrategy(s pb.Strategy) (mergestrategy.MergeStrategy, error) {
200201
switch s {
201202
case pb.Strategy_DEFAULT:
202203
// TODO: resolve default strategy based on queue configuration
203-
return entity.RequestLandStrategyRebase, nil
204+
return mergestrategy.MergeStrategyRebase, nil
204205
case pb.Strategy_REBASE:
205-
return entity.RequestLandStrategyRebase, nil
206+
return mergestrategy.MergeStrategyRebase, nil
206207
case pb.Strategy_SQUASH_REBASE:
207-
return entity.RequestLandStrategySquashRebase, nil
208+
return mergestrategy.MergeStrategySquashRebase, nil
208209
case pb.Strategy_MERGE:
209-
return entity.RequestLandStrategyMerge, nil
210+
return mergestrategy.MergeStrategyMerge, nil
210211
default:
211-
return entity.RequestLandStrategyUnknown, fmt.Errorf("unknown land strategy in proto message: %v", s)
212+
return mergestrategy.MergeStrategyUnknown, fmt.Errorf("unknown land strategy in proto message: %v", s)
212213
}
213214
}

submitqueue/gateway/controller/land_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/uber-go/tally"
2525
"github.com/uber/submitqueue/core/consumer"
2626
"github.com/uber/submitqueue/core/errs"
27+
"github.com/uber/submitqueue/entity/mergestrategy"
2728
entityqueue "github.com/uber/submitqueue/entity/messagequeue"
2829
countermock "github.com/uber/submitqueue/extension/counter/mock"
2930
queuemock "github.com/uber/submitqueue/extension/messagequeue/mock"
@@ -289,7 +290,7 @@ func TestLand_PublishesToQueue(t *testing.T) {
289290
assert.Equal(t, "test-queue/123", deserializedReq.ID)
290291
assert.Equal(t, "test-queue", deserializedReq.Queue)
291292
assert.Equal(t, []string{"github://uber/backend/pull/456/fedcba9876543210fedcba9876543210fedcba98"}, deserializedReq.Change.URIs)
292-
assert.Equal(t, entity.RequestLandStrategyRebase, deserializedReq.LandStrategy)
293+
assert.Equal(t, mergestrategy.MergeStrategyRebase, deserializedReq.LandStrategy)
293294
}
294295

295296
func TestLand_ContinuesWhenPublishFails(t *testing.T) {

0 commit comments

Comments
 (0)