diff --git a/api/runway/messagequeue/merge_test.go b/api/runway/messagequeue/merge_test.go index 5be5c368..754489db 100644 --- a/api/runway/messagequeue/merge_test.go +++ b/api/runway/messagequeue/merge_test.go @@ -33,12 +33,12 @@ func TestMergeRequestRoundTrip(t *testing.T) { Steps: []*MergeStep{ { StepId: "queue-a/1", - Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/1/0123456789abcdef0123456789abcdef01234567"}}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/repo/pull/1/0123456789abcdef0123456789abcdef01234567"}}, Strategy: strategypb.Strategy_REBASE, }, { StepId: "queue-a/2", - Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}, Strategy: strategypb.Strategy_MERGE, }, }, diff --git a/api/runway/messagequeue/proto/merge.proto b/api/runway/messagequeue/proto/merge.proto index 72210667..acbd800b 100644 --- a/api/runway/messagequeue/proto/merge.proto +++ b/api/runway/messagequeue/proto/merge.proto @@ -25,19 +25,21 @@ option java_multiple_files = true; option java_outer_classname = "MergeProto"; option java_package = "com.uber.submitqueue.runway.messagequeue"; -// MergeStep is one step of an ordered merge: a single set of change(s) applied -// with a strategy. Runway applies the steps of a request in order on top of the -// merge target; the ordering encodes the base-layering (earlier steps are the -// in-flight base, the last step is the candidate). +// MergeStep is one step of an ordered merge: a single change applied with a +// strategy. Runway applies the steps of a request in order on top of the merge +// target; the ordering encodes the base-layering (earlier steps are the +// in-flight base, the last step is the candidate). A change may contain +// multiple URIs (e.g. stacked PRs that land together). message MergeStep { // step_id is an opaque, caller-assigned identifier for this step. Runway // treats it as an attribution token only -- it echoes it back per-step in // StepResult so a multi-step result is attributable -- and never interprets // its contents. string step_id = 1; - // changes are the code change(s) to apply for this step. - repeated uber.base.change.Change changes = 2; - // strategy is how this step's changes are integrated into the merge target. + // change is the code change to apply for this step. A change may carry + // multiple URIs when the step represents stacked or grouped changes. + uber.base.change.Change change = 2; + // strategy is how this step's change is integrated into the merge target. uber.base.mergestrategy.Strategy strategy = 3; } diff --git a/api/runway/messagequeue/protopb/merge.pb.go b/api/runway/messagequeue/protopb/merge.pb.go index 5409c180..07608874 100644 --- a/api/runway/messagequeue/protopb/merge.pb.go +++ b/api/runway/messagequeue/protopb/merge.pb.go @@ -95,10 +95,11 @@ func (Outcome) EnumDescriptor() ([]byte, []int) { return file_merge_proto_rawDescGZIP(), []int{0} } -// MergeStep is one step of an ordered merge: a single set of change(s) applied -// with a strategy. Runway applies the steps of a request in order on top of the -// merge target; the ordering encodes the base-layering (earlier steps are the -// in-flight base, the last step is the candidate). +// MergeStep is one step of an ordered merge: a single change applied with a +// strategy. Runway applies the steps of a request in order on top of the merge +// target; the ordering encodes the base-layering (earlier steps are the +// in-flight base, the last step is the candidate). A change may contain +// multiple URIs (e.g. stacked PRs that land together). type MergeStep struct { state protoimpl.MessageState `protogen:"open.v1"` // step_id is an opaque, caller-assigned identifier for this step. Runway @@ -106,9 +107,10 @@ type MergeStep struct { // StepResult so a multi-step result is attributable -- and never interprets // its contents. StepId string `protobuf:"bytes,1,opt,name=step_id,json=stepId,proto3" json:"step_id,omitempty"` - // changes are the code change(s) to apply for this step. - Changes []*protopb.Change `protobuf:"bytes,2,rep,name=changes,proto3" json:"changes,omitempty"` - // strategy is how this step's changes are integrated into the merge target. + // change is the code change to apply for this step. A change may carry + // multiple URIs when the step represents stacked or grouped changes. + Change *protopb.Change `protobuf:"bytes,2,opt,name=change,proto3" json:"change,omitempty"` + // strategy is how this step's change is integrated into the merge target. Strategy protopb1.Strategy `protobuf:"varint,3,opt,name=strategy,proto3,enum=uber.base.mergestrategy.Strategy" json:"strategy,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache @@ -151,9 +153,9 @@ func (x *MergeStep) GetStepId() string { return "" } -func (x *MergeStep) GetChanges() []*protopb.Change { +func (x *MergeStep) GetChange() *protopb.Change { if x != nil { - return x.Changes + return x.Change } return nil } @@ -439,10 +441,10 @@ var File_merge_proto protoreflect.FileDescriptor const file_merge_proto_rawDesc = "" + "\n" + - "\vmerge.proto\x12\x18uber.runway.messagequeue\x1a\"api/base/change/proto/change.proto\x1a0api/base/mergestrategy/proto/mergestrategy.proto\x1a.api/base/messagequeue/proto/messagequeue.proto\"\x97\x01\n" + + "\vmerge.proto\x12\x18uber.runway.messagequeue\x1a\"api/base/change/proto/change.proto\x1a0api/base/mergestrategy/proto/mergestrategy.proto\x1a.api/base/messagequeue/proto/messagequeue.proto\"\x95\x01\n" + "\tMergeStep\x12\x17\n" + - "\astep_id\x18\x01 \x01(\tR\x06stepId\x122\n" + - "\achanges\x18\x02 \x03(\v2\x18.uber.base.change.ChangeR\achanges\x12=\n" + + "\astep_id\x18\x01 \x01(\tR\x06stepId\x120\n" + + "\x06change\x18\x02 \x01(\v2\x18.uber.base.change.ChangeR\x06change\x12=\n" + "\bstrategy\x18\x03 \x01(\x0e2!.uber.base.mergestrategy.StrategyR\bstrategy\"\xa2\x01\n" + "\fMergeRequest\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\x12\x1d\n" + @@ -495,7 +497,7 @@ var file_merge_proto_goTypes = []any{ (protopb1.Strategy)(0), // 7: uber.base.mergestrategy.Strategy } var file_merge_proto_depIdxs = []int32{ - 6, // 0: uber.runway.messagequeue.MergeStep.changes:type_name -> uber.base.change.Change + 6, // 0: uber.runway.messagequeue.MergeStep.change:type_name -> uber.base.change.Change 7, // 1: uber.runway.messagequeue.MergeStep.strategy:type_name -> uber.base.mergestrategy.Strategy 1, // 2: uber.runway.messagequeue.MergeRequest.steps:type_name -> uber.runway.messagequeue.MergeStep 3, // 3: uber.runway.messagequeue.StepResult.outputs:type_name -> uber.runway.messagequeue.StepOutput diff --git a/runway/extension/merger/noop/noop_test.go b/runway/extension/merger/noop/noop_test.go index 740281aa..2d283c66 100644 --- a/runway/extension/merger/noop/noop_test.go +++ b/runway/extension/merger/noop/noop_test.go @@ -33,12 +33,12 @@ func testRequest() *runwaymq.MergeRequest { Steps: []*runwaymq.MergeStep{ { StepId: "queue-a/1", - Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/1/abcdef0123456789abcdef0123456789abcdef01"}}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/repo/pull/1/abcdef0123456789abcdef0123456789abcdef01"}}, Strategy: strategypb.Strategy_REBASE, }, { StepId: "queue-a/2", - Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}}, + Change: &changepb.Change{Uris: []string{"github://github.example.com/uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}, Strategy: strategypb.Strategy_MERGE, }, }, diff --git a/submitqueue/orchestrator/controller/merge/merge.go b/submitqueue/orchestrator/controller/merge/merge.go index 494030cb..d9c87930 100644 --- a/submitqueue/orchestrator/controller/merge/merge.go +++ b/submitqueue/orchestrator/controller/merge/merge.go @@ -165,7 +165,7 @@ func (c *Controller) buildMergeRequest(ctx context.Context, batch entity.Batch) } steps = append(steps, &runwaymq.MergeStep{ StepId: request.ID, - Changes: []*changepb.Change{{Uris: request.Change.URIs}}, + Change: &changepb.Change{Uris: request.Change.URIs}, Strategy: toProtoStrategy(request.LandStrategy), }) } diff --git a/submitqueue/orchestrator/controller/merge/merge_test.go b/submitqueue/orchestrator/controller/merge/merge_test.go index 8207cb44..39f6664e 100644 --- a/submitqueue/orchestrator/controller/merge/merge_test.go +++ b/submitqueue/orchestrator/controller/merge/merge_test.go @@ -146,12 +146,12 @@ func TestProcess_PublishesFullPayloadToRunway(t *testing.T) { require.Len(t, got.Steps, 2) // One step per member request, in Contains order, attributed by request id. assert.Equal(t, req1.ID, got.Steps[0].StepId) - require.Len(t, got.Steps[0].Changes, 1) - assert.Equal(t, req1.Change.URIs, got.Steps[0].Changes[0].Uris) + require.NotNil(t, got.Steps[0].Change) + assert.Equal(t, req1.Change.URIs, got.Steps[0].Change.Uris) assert.Equal(t, strategypb.Strategy_SQUASH_REBASE, got.Steps[0].Strategy) assert.Equal(t, req2.ID, got.Steps[1].StepId) - require.Len(t, got.Steps[1].Changes, 1) - assert.Equal(t, req2.Change.URIs, got.Steps[1].Changes[0].Uris) + require.NotNil(t, got.Steps[1].Change) + assert.Equal(t, req2.Change.URIs, got.Steps[1].Change.Uris) assert.Equal(t, strategypb.Strategy_REBASE, got.Steps[1].Strategy) } diff --git a/submitqueue/orchestrator/controller/validate/validate.go b/submitqueue/orchestrator/controller/validate/validate.go index e0176168..bb3ce96a 100644 --- a/submitqueue/orchestrator/controller/validate/validate.go +++ b/submitqueue/orchestrator/controller/validate/validate.go @@ -200,7 +200,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er Steps: []*runwaymq.MergeStep{ { StepId: request.ID, - Changes: []*changepb.Change{{Uris: request.Change.URIs}}, + Change: &changepb.Change{Uris: request.Change.URIs}, Strategy: toProtoStrategy(request.LandStrategy), }, }, diff --git a/submitqueue/orchestrator/controller/validate/validate_test.go b/submitqueue/orchestrator/controller/validate/validate_test.go index c4526c5d..78691dfc 100644 --- a/submitqueue/orchestrator/controller/validate/validate_test.go +++ b/submitqueue/orchestrator/controller/validate/validate_test.go @@ -220,8 +220,8 @@ func TestController_Process_PublishesCheckToRunway(t *testing.T) { assert.Equal(t, request.Queue, got.QueueName) require.Len(t, got.Steps, 1) assert.Equal(t, request.ID, got.Steps[0].StepId) - require.Len(t, got.Steps[0].Changes, 1) - assert.Equal(t, request.Change.URIs, got.Steps[0].Changes[0].Uris) + require.NotNil(t, got.Steps[0].Change) + assert.Equal(t, request.Change.URIs, got.Steps[0].Change.Uris) assert.Equal(t, strategypb.Strategy_REBASE, got.Steps[0].Strategy) }