refactor(changeprovider): accept entity.Request, resolve change internally

Change ChangeProvider.Get to take the orchestrator's request identity (entity.Request) instead of a controller-pre-resolved entity.Change, per the extension contract. The GitHub implementation and the fake read request.Change themselves; the validate controller hands over the request it already loaded.

Output is unchanged: one entity.ChangeInfo per URI, each self-identifying by URI. The provider is the external resolver, so it needs no injected dependency — the factory and Config are unchanged.

Author: behinddwalls

submitqueue/extension/changeprovider/change_provider.go

@@ -29,10 +29,11 @@ import (
 // entity.Author, entity.ChangedFile — live in the entity package so the same typed
 // facts can be persisted (entity.ChangeRecord) and scored without re-declaration.
 type ChangeProvider interface {
-	// Get retrieves change information for the provided Change.
+	// Get retrieves change information for the provided request.
+	// It is handed the request identity and reads request.Change itself.
 	// For a Change with multiple URIs (e.g., stacked PRs), returns one ChangeInfo per URI.
 	// Returns a slice of ChangeInfo, one for each change in the stack.
-	Get(ctx context.Context, change entity.Change) ([]entity.ChangeInfo, error)
+	Get(ctx context.Context, request entity.Request) ([]entity.ChangeInfo, error)
 }
 
 // Config carries the per-queue identity handed to a Factory. The system knows

submitqueue/extension/changeprovider/fake/fake.go

@@ -47,9 +47,10 @@ func New() changeprovider.ChangeProvider {
 	return provider{}
 }
 
-// Get returns one ChangeInfo per URI in the change, unless a recognized marker
-// token requests a failure. The "one ChangeInfo per URI" contract is preserved.
-func (provider) Get(_ context.Context, change entity.Change) ([]entity.ChangeInfo, error) {
+// Get returns one ChangeInfo per URI in the request's change, unless a recognized
+// marker token requests a failure. The "one ChangeInfo per URI" contract is preserved.
+func (provider) Get(_ context.Context, request entity.Request) ([]entity.ChangeInfo, error) {
+	change := request.Change
 	if fakemarker.Token(change.URIs) == tokenError {
 		return nil, fmt.Errorf("fake: marked provider error")
 	}

submitqueue/extension/changeprovider/fake/fake_test.go

@@ -47,7 +47,7 @@ func TestProvider_Get_OnePerURI(t *testing.T) {
 	p := New()
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			infos, err := p.Get(context.Background(), entity.Change{URIs: tt.uris})
+			infos, err := p.Get(context.Background(), entity.Request{Change: entity.Change{URIs: tt.uris}})
 			require.NoError(t, err)
 			require.Len(t, infos, len(tt.uris))
 			for i, uri := range tt.uris {
@@ -59,8 +59,8 @@ func TestProvider_Get_OnePerURI(t *testing.T) {
 
 func TestProvider_Get_ErrorMarker(t *testing.T) {
 	p := New()
-	_, err := p.Get(context.Background(), entity.Change{
+	_, err := p.Get(context.Background(), entity.Request{Change: entity.Change{
 		URIs: []string{"github://owner/repo/pull/1/abc?sq-fake=provider-error"},
-	})
+	}})
 	require.Error(t, err)
 }

submitqueue/extension/changeprovider/github/provider.go

@@ -42,12 +42,14 @@ func NewProvider(params Params) changeprovider.ChangeProvider {
 	}
 }
 
-// Get retrieves change information from GitHub for the provided Change.
+// Get retrieves change information from GitHub for the request's change.
 // Returns one ChangeInfo per URI (one per PR in stacked changes).
-func (p *provider) Get(ctx context.Context, change entity.Change) (_ []entity.ChangeInfo, retErr error) {
+func (p *provider) Get(ctx context.Context, request entity.Request) (_ []entity.ChangeInfo, retErr error) {
 	op := coremetrics.Begin(p.metricsScope, "get")
 	defer func() { op.Complete(retErr) }()
 
+	change := request.Change
+
 	// Parse all change IDs
 	changeIDs := make([]entitygithub.ChangeID, 0, len(change.URIs))
 	for _, uri := range change.URIs {

submitqueue/extension/changeprovider/github/provider_test.go

@@ -106,7 +106,7 @@ func TestProvider_Get(t *testing.T) {
 			}
 
 			p := newTestProvider(t, serverURL)
-			infos, err := p.Get(context.Background(), entity.Change{URIs: tt.uris})
+			infos, err := p.Get(context.Background(), entity.Request{Change: entity.Change{URIs: tt.uris}})
 
 			if tt.wantErr {
 				require.Error(t, err)
@@ -147,9 +147,9 @@ func TestProvider_Get_Pagination(t *testing.T) {
 	defer server.Close()
 
 	p := newTestProvider(t, server.URL)
-	infos, err := p.Get(context.Background(), entity.Change{
+	infos, err := p.Get(context.Background(), entity.Request{Change: entity.Change{
 		URIs: []string{"github://uber/submitqueue/pull/456/" + shaXYZ},
-	})
+	}})
 
 	require.NoError(t, err)
 	assert.Equal(t, 2, callCount)
@@ -170,12 +170,12 @@ func TestProvider_Get_MultiplePRs(t *testing.T) {
 	defer server.Close()
 
 	p := newTestProvider(t, server.URL)
-	infos, err := p.Get(context.Background(), entity.Change{
+	infos, err := p.Get(context.Background(), entity.Request{Change: entity.Change{
 		URIs: []string{
 			"github://uber/submitqueue/pull/123/" + shaA,
 			"github://uber/submitqueue/pull/456/" + shaB,
 		},
-	})
+	}})
 
 	require.NoError(t, err)
 	assert.Equal(t, 2, callCount)
@@ -202,12 +202,12 @@ func TestProvider_Get_FetchError_StopsOnFirstFailure(t *testing.T) {
 	defer server.Close()
 
 	p := newTestProvider(t, server.URL)
-	_, err := p.Get(context.Background(), entity.Change{
+	_, err := p.Get(context.Background(), entity.Request{Change: entity.Change{
 		URIs: []string{
 			"github://uber/submitqueue/pull/123/" + shaA,
 			"github://uber/submitqueue/pull/456/" + shaB,
 		},
-	})
+	}})
 
 	require.Error(t, err)
 	assert.Equal(t, 2, callCount)

submitqueue/extension/changeprovider/mock/change_provider_mock.go

@@ -161,7 +161,7 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
 		coremetrics.NamedCounter(c.metricsScope, "process", "change_provider_errors", 1)
 		return fmt.Errorf("failed to build change provider for queue %s: %w", request.Queue, err)
 	}
-	changeInfos, err := changeProvider.Get(ctx, request.Change)
+	changeInfos, err := changeProvider.Get(ctx, request)
 	if err != nil {
 		coremetrics.NamedCounter(c.metricsScope, "process", "change_provider_errors", 1)
 		return fmt.Errorf("failed to fetch change information for request %s: %w", request.ID, err)

submitqueue/orchestrator/controller/validate/validate.go

@@ -46,7 +46,7 @@ func requestIDPayload(t *testing.T, id string) []byte {
 // mockChangeProvider is a simple mock that returns test data.
 type mockChangeProvider struct{}
 
-func (m *mockChangeProvider) Get(ctx context.Context, change entity.Change) ([]entity.ChangeInfo, error) {
+func (m *mockChangeProvider) Get(ctx context.Context, request entity.Request) ([]entity.ChangeInfo, error) {
 	return []entity.ChangeInfo{
 		{
 			URI: "github://org/repo/pull/123/abcdef0123456789abcdef0123456789abcdef01",