interface for change provider and update request to include uri

entity/request.go

@@ -33,13 +33,17 @@ const (
 	RequestStateError RequestState = "error"
 )
 
-// Change represents a set of related code changes identified by one or more IDs from a particular code change provider, like Github Pull Requests.
-// The object is immutable after creation.
+// Change represents a code change identified by URIs from a code change provider (e.g., GitHub Pull Request, Phabricator Diff).
+// The provider is extracted from the URI scheme. The object is immutable after creation.
 type Change struct {
-	// Source is the code change provider (e.g., "github", "gerrit", "phabricator").
-	Source string `json:"source"`
-	// IDs is a list of change IDs, in a format specific to the code change provider, that should be landed together.
-	IDs []string `json:"ids"`
+	// URIs identifies the change(s) to land (RFC 3986 compliant).
+	// The scheme identifies the change provider, and the path contains provider-specific resource identifiers.
+	//
+	// GitHub is supported by default (though other providers can be added):
+	//   Template: "github://<org>/<repo>/pull/<pr>/<hash>"
+	//   Example: "github://uber/submitqueue/pull/123/abc123def"
+	//
+	URIs []string `json:"uris"`
 }
 
 // Request defines a request to land (merge into target branch of the source control repository) a set of code changes.

entity/request_test.go

@@ -11,7 +11,7 @@ func TestRequest_ToBytes(t *testing.T) {
 	req := Request{
 		ID:           "test-queue/123",
 		Queue:        "test-queue",
-		Change:       Change{Source: "github", IDs: []string{"PR-456", "PR-789"}},
+		Change:       Change{URIs: []string{"github://uber/submitqueue/pull/456/abc123def", "github://uber/submitqueue/pull/789/def456abc"}},
 		LandStrategy: RequestLandStrategyRebase,
 		State:        RequestStateNew,
 		Version:      1,
@@ -24,8 +24,7 @@ func TestRequest_ToBytes(t *testing.T) {
 	// Verify JSON contains expected fields
 	jsonStr := string(data)
 	assert.Contains(t, jsonStr, "test-queue/123")
-	assert.Contains(t, jsonStr, "github")
-	assert.Contains(t, jsonStr, "PR-456")
+	assert.Contains(t, jsonStr, "github://uber/submitqueue/pull/456/abc123def")
 	assert.Contains(t, jsonStr, "rebase")
 	assert.Contains(t, jsonStr, "new")
 }
@@ -34,7 +33,7 @@ func TestRequestFromBytes(t *testing.T) {
 	original := Request{
 		ID:           "my-queue/999",
 		Queue:        "my-queue",
-		Change:       Change{Source: "gerrit", IDs: []string{"CL-111"}},
+		Change:       Change{URIs: []string{"code.uber.internal.com/D111"}},
 		LandStrategy: RequestLandStrategyMerge,
 		State:        RequestStateProcessing,
 		Version:      3,
@@ -51,8 +50,7 @@ func TestRequestFromBytes(t *testing.T) {
 	// Verify all fields match
 	assert.Equal(t, original.ID, deserialized.ID)
 	assert.Equal(t, original.Queue, deserialized.Queue)
-	assert.Equal(t, original.Change.Source, deserialized.Change.Source)
-	assert.Equal(t, original.Change.IDs, deserialized.Change.IDs)
+	assert.Equal(t, original.Change.URIs, deserialized.Change.URIs)
 	assert.Equal(t, original.LandStrategy, deserialized.LandStrategy)
 	assert.Equal(t, original.State, deserialized.State)
 	assert.Equal(t, original.Version, deserialized.Version)
@@ -85,33 +83,33 @@ func TestRequest_SerializationRoundTrip(t *testing.T) {
 		req  Request
 	}{
 		{
-			name: "full request",
+			name: "github stacked diff",
 			req: Request{
 				ID:           "queue1/100",
 				Queue:        "queue1",
-				Change:       Change{Source: "github", IDs: []string{"PR-1", "PR-2", "PR-3"}},
+				Change:       Change{URIs: []string{"github://uber/repo-a/pull/101/aaa111", "github://uber/repo-a/pull/102/bbb222", "github://uber/repo-a/pull/103/ccc333"}},
 				LandStrategy: RequestLandStrategySquashRebase,
 				State:        RequestStateLanded,
 				Version:      5,
 			},
 		},
 		{
-			name: "minimal request",
+			name: "phabricator revision",
 			req: Request{
 				ID:           "queue2/200",
 				Queue:        "queue2",
-				Change:       Change{Source: "phabricator", IDs: []string{"D123"}},
+				Change:       Change{URIs: []string{"code.uber.internal.com/D12345"}},
 				LandStrategy: RequestLandStrategyRebase,
 				State:        RequestStateNew,
 				Version:      1,
 			},
 		},
 		{
-			name: "error state request",
+			name: "github enterprise request",
 			req: Request{
 				ID:           "queue3/300",
 				Queue:        "queue3",
-				Change:       Change{Source: "github", IDs: []string{"PR-999"}},
+				Change:       Change{URIs: []string{"github.uber.com/internal/service/999/deadbeef12"}},
 				LandStrategy: RequestLandStrategyMerge,
 				State:        RequestStateError,
 				Version:      10,

extension/changeprovider/BUILD.bazel

@@ -0,0 +1,9 @@
+load("@rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "changeprovider",
+    srcs = ["change_provider.go"],
+    importpath = "github.com/uber/submitqueue/extension/changeprovider",
+    visibility = ["//visibility:public"],
+    deps = ["//entity"],
+)

extension/changeprovider/change_provider.go

@@ -0,0 +1,47 @@
+package changeprovider
+
+import (
+	"context"
+
+	"github.com/uber/submitqueue/entity"
+)
+
+// User represents the author of a change.
+type User struct {
+	// Name is the display name of the user.
+	Name string
+	// Email is the email address of the user.
+	Email string
+}
+
+// ChangedFile represents a single file modification in a change.
+type ChangedFile struct {
+	// Path is the file path relative to the repository root.
+	Path string
+	// Patch is the diff patch content for this file.
+	Patch string
+	// LinesAdded is the number of lines added in this file.
+	LinesAdded int
+	// LinesDeleted is the number of lines deleted in this file.
+	LinesDeleted int
+	// LinesModified is the number of lines modified in this file.
+	LinesModified int
+}
+
+// ChangeInfo contains metadata and file changes for a code change.
+type ChangeInfo struct {
+	// ID is the change identifier (e.g., "PR: uber-code/go-code/1" or "diff: uber-code/go-code/D1").
+	ID string
+	// User is the author of the change.
+	User User
+	// ChangedFiles is the list of files modified in this change. Order is unspecified.
+	ChangedFiles []ChangedFile
+}
+
+// ChangeProvider fetches change metadata from external systems
+// Each implementation is configured for a specific provider (GitHub, GitLab, Phabricator).
+type ChangeProvider interface {
+	// Get retrieves change information for the provided Change.
+	// Returns the change info containing metadata and file changes.
+	Get(ctx context.Context, change entity.Change) (ChangeInfo, error)
+}

extension/storage/mysql/request_store.go

@@ -25,12 +25,12 @@ func NewRequestStore(db *sql.DB) storage.RequestStore {
 // Get retrieves a land request by ID. Returns ErrNotFound if the request is not found.
 func (r *requestStore) Get(ctx context.Context, id string) (entity.Request, error) {
 	var req entity.Request
-	var changeIDsJSON []byte
+	var changeURIsJSON []byte
 
 	err := r.db.QueryRowContext(ctx,
-		"SELECT id, queue, change_source, change_ids, land_strategy, state, version FROM request WHERE id = ?",
+		"SELECT id, queue, change_uri, land_strategy, state, version FROM request WHERE id = ?",
 		id,
-	).Scan(&req.ID, &req.Queue, &req.Change.Source, &changeIDsJSON, &req.LandStrategy, &req.State, &req.Version)
+	).Scan(&req.ID, &req.Queue, &changeURIsJSON, &req.LandStrategy, &req.State, &req.Version)
 
 	if errors.Is(err, sql.ErrNoRows) {
 		return entity.Request{}, storage.WrapNotFound(err)
@@ -39,23 +39,25 @@ func (r *requestStore) Get(ctx context.Context, id string) (entity.Request, erro
 		return entity.Request{}, fmt.Errorf("failed to get request entity id=%s from the database: %w", id, err)
 	}
 
-	if err := json.Unmarshal(changeIDsJSON, &req.Change.IDs); err != nil {
-		return entity.Request{}, fmt.Errorf("failed to unmarshal change IDs for request entity id=%s from the database: %w", id, err)
+	// Unmarshal the change URIs from JSON
+	if err := json.Unmarshal(changeURIsJSON, &req.Change.URIs); err != nil {
+		return entity.Request{}, fmt.Errorf("failed to unmarshal change URIs for request id=%s: %w", id, err)
 	}
 
 	return req, nil
 }
 
 // Create creates a new land request. The request must have a unique ID already assigned. Returns ErrAlreadyExists if the request ID already exists.
 func (r *requestStore) Create(ctx context.Context, request entity.Request) error {
-	changeIDsJSON, err := json.Marshal(request.Change.IDs)
+	// Marshal the change URIs to JSON
+	changeURIsJSON, err := json.Marshal(request.Change.URIs)
 	if err != nil {
-		return fmt.Errorf("failed to marshal change IDs=%v id=%s for Create request entity: %w", request.Change.IDs, request.ID, err)
+		return fmt.Errorf("failed to marshal change URIs for request id=%s: %w", request.ID, err)
 	}
 
 	_, err = r.db.ExecContext(ctx,
-		"INSERT INTO request (id, queue, change_source, change_ids, land_strategy, state, version) VALUES (?, ?, ?, ?, ?, ?, ?)",
-		request.ID, request.Queue, request.Change.Source, changeIDsJSON, request.LandStrategy, request.State, request.Version,
+		"INSERT INTO request (id, queue, change_uri, land_strategy, state, version) VALUES (?, ?, ?, ?, ?, ?)",
+		request.ID, request.Queue, changeURIsJSON, request.LandStrategy, request.State, request.Version,
 	)
 	if err != nil {
 		var mysqlErr *mysql.MySQLError

extension/storage/mysql/schema/request.sql

@@ -1,8 +1,7 @@
 CREATE TABLE IF NOT EXISTS request (
     id VARCHAR(255) NOT NULL,
     queue VARCHAR(255) NOT NULL,
-    change_source VARCHAR(255) NOT NULL,
-    change_ids JSON NOT NULL,
+    change_uri JSON NOT NULL,
     land_strategy VARCHAR(64) NOT NULL,
     state VARCHAR(64) NOT NULL,
     version INT NOT NULL,

gateway/controller/land.go

@@ -61,16 +61,12 @@ func (c *LandController) Land(ctx context.Context, req *pb.LandRequest) (*pb.Lan
 	if req.Queue == "" {
 		return nil, fmt.Errorf("LandController requires the request to have a queue name specified: %w", ErrInvalidRequest)
 	}
-	if req.Change == nil || req.Change.Source == "" {
-		return nil, fmt.Errorf("LandController requires the request to have a change source specified: %w", ErrInvalidRequest)
-	}
-	if len(req.Change.GetIds()) == 0 {
-		return nil, fmt.Errorf("LandController requires the request to have at least one change ID specified: %w", ErrInvalidRequest)
+	if req.Change == nil || len(req.Change.Uris) == 0 {
+		return nil, fmt.Errorf("LandController requires the request to have at least one change URI specified: %w", ErrInvalidRequest)
 	}
 
 	change := entity.Change{
-		Source: req.Change.GetSource(),
-		IDs:    req.Change.GetIds(),
+		URIs: req.Change.GetUris(),
 	}
 
 	// TODO: validate that queue is configured. Return error if not.
@@ -104,8 +100,8 @@ func (c *LandController) Land(ctx context.Context, req *pb.LandRequest) (*pb.Lan
 	c.logger.Debugw("land request created",
 		"queue", req.Queue,
 		"sqid", request.ID,
-		"change_source", change.Source,
-		"change_ids", change.IDs,
+		"change_uris", change.URIs,
+		"change_count", len(change.URIs),
 		"strategy", string(strategy),
 	)
 

gateway/controller/land_test.go

@@ -248,7 +248,7 @@ func TestLand_ReturnsSqid(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Uris: []string{"github://uber/test-repo/pull/123/abc123def"}},
 	}
 	resp, err := controller.Land(ctx, req)
 
@@ -280,16 +280,15 @@ func TestLand_PassesCorrectParametersToStore(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:    "my-queue",
-		Change:   &pb.Change{Source: "github", Ids: []string{"pr-1", "pr-2"}},
+		Change:   &pb.Change{Uris: []string{"github://uber/myservice/pull/1/abc111", "github://uber/myservice/pull/2/def222"}},
 		Strategy: pb.Strategy_REBASE,
 	}
 	resp, err := controller.Land(ctx, req)
 
 	require.NoError(t, err)
 	assert.Equal(t, "my-queue/42", capturedRequest.ID)
 	assert.Equal(t, "my-queue", capturedRequest.Queue)
-	assert.Equal(t, "github", capturedRequest.Change.Source)
-	assert.Equal(t, []string{"pr-1", "pr-2"}, capturedRequest.Change.IDs)
+	assert.Equal(t, []string{"github://uber/myservice/pull/1/abc111", "github://uber/myservice/pull/2/def222"}, capturedRequest.Change.URIs)
 	assert.Equal(t, entity.RequestLandStrategyRebase, capturedRequest.LandStrategy)
 	assert.Equal(t, entity.RequestStateNew, capturedRequest.State)
 	assert.Equal(t, int32(1), capturedRequest.Version)
@@ -317,7 +316,7 @@ func TestLand_ReturnsErrorOnStorageFailure(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Uris: []string{"github://uber/test-repo/pull/123/abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -345,7 +344,7 @@ func TestLand_ReturnsErrorOnCounterFailure(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Uris: []string{"github://uber/test-repo/pull/123/abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -376,7 +375,7 @@ func TestLand_CounterDomainIncludesQueue(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "my-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Uris: []string{"github://uber/test-repo/pull/123/abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -398,15 +397,15 @@ func TestLand_ReturnsErrorOnEmptyQueue(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Uris: []string{"github://uber/test-repo/pull/123/abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 
 	require.Error(t, err)
 	assert.True(t, IsInvalidRequest(err))
 }
 
-func TestLand_ReturnsErrorOnEmptyChangeSource(t *testing.T) {
+func TestLand_ReturnsErrorOnEmptyChangeUri(t *testing.T) {
 	store := &mockStorage{requestStore: &mockRequestStore{
 		createFunc: func(ctx context.Context, request entity.Request) error {
 			return nil
@@ -420,7 +419,7 @@ func TestLand_ReturnsErrorOnEmptyChangeSource(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "", Ids: []string{"123"}},
+		Change: &pb.Change{Uris: []string{}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -450,28 +449,6 @@ func TestLand_ReturnsErrorOnNilChange(t *testing.T) {
 	assert.True(t, IsInvalidRequest(err))
 }
 
-func TestLand_ReturnsErrorOnEmptyChangeIDs(t *testing.T) {
-	store := &mockStorage{requestStore: &mockRequestStore{
-		createFunc: func(ctx context.Context, request entity.Request) error {
-			return nil
-		},
-	}}
-	cnt := &mockCounter{nextFunc: func(ctx context.Context, domain string) (int64, error) {
-		return 1, nil
-	}}
-	controller := NewLandController(zap.NewNop().Sugar(), tally.NoopScope, store, cnt, noopPublisher(), "request")
-	ctx := context.Background()
-
-	req := &pb.LandRequest{
-		Queue:  "test-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{}},
-	}
-	_, err := controller.Land(ctx, req)
-
-	require.Error(t, err)
-	assert.True(t, IsInvalidRequest(err))
-}
-
 func TestLand_PublishesToQueue(t *testing.T) {
 	var publishedTopic string
 	var publishedMessage queue.Message
@@ -495,7 +472,7 @@ func TestLand_PublishesToQueue(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:    "test-queue",
-		Change:   &pb.Change{Source: "github", Ids: []string{"PR-456"}},
+		Change:   &pb.Change{Uris: []string{"github://uber/backend/pull/456/fed987cba"}},
 		Strategy: pb.Strategy_REBASE,
 	}
 	resp, err := controller.Land(ctx, req)
@@ -513,8 +490,7 @@ func TestLand_PublishesToQueue(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "test-queue/123", deserializedReq.ID)
 	assert.Equal(t, "test-queue", deserializedReq.Queue)
-	assert.Equal(t, "github", deserializedReq.Change.Source)
-	assert.Equal(t, []string{"PR-456"}, deserializedReq.Change.IDs)
+	assert.Equal(t, []string{"github://uber/backend/pull/456/fed987cba"}, deserializedReq.Change.URIs)
 	assert.Equal(t, entity.RequestLandStrategyRebase, deserializedReq.LandStrategy)
 	assert.Equal(t, entity.RequestStateNew, deserializedReq.State)
 	assert.Equal(t, int32(1), deserializedReq.Version)
@@ -538,7 +514,7 @@ func TestLand_ContinuesWhenPublishFails(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{"PR-1"}},
+		Change: &pb.Change{Uris: []string{"github://uber/service/pull/1/abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)