interface for change provider and update request to include uri

Author: rashmi-prithyani

entity/request.go

@@ -33,13 +33,15 @@ 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.
+// Change represents a set of related code changes identified by one or more URIs from a particular code change provider, like Github Pull Requests.
 // The object is immutable after creation.
 type Change struct {
-	// Source is the code change provider (e.g., "github", "gerrit", "phabricator").
+	// Source is the code change provider ID that maps to a registered provider (e.g., "github", "github-enterprise", "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 is a list of change URIs that should be landed together. The format is provider-specific:
+	//   - GitHub: "owner/repo/pr_number@commit_hash" (e.g., "uber/submitqueue/123@abc123def")
+	//   - Phabricator: "revision_id@commit_hash" (e.g., "D12345@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{Source: "github", URIs: []string{"uber/submitqueue/456@abc123def", "uber/submitqueue/789@def456abc"}},
 		LandStrategy: RequestLandStrategyRebase,
 		State:        RequestStateNew,
 		Version:      1,
@@ -25,7 +25,7 @@ func TestRequest_ToBytes(t *testing.T) {
 	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, "uber/submitqueue/456@abc123def")
 	assert.Contains(t, jsonStr, "rebase")
 	assert.Contains(t, jsonStr, "new")
 }
@@ -34,7 +34,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{Source: "phabricator", URIs: []string{"D111@fedcba987"}},
 		LandStrategy: RequestLandStrategyMerge,
 		State:        RequestStateProcessing,
 		Version:      3,
@@ -52,7 +52,7 @@ func TestRequestFromBytes(t *testing.T) {
 	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 +85,33 @@ func TestRequest_SerializationRoundTrip(t *testing.T) {
 		req  Request
 	}{
 		{
-			name: "full request",
+			name: "full request with multiple PRs",
 			req: Request{
 				ID:           "queue1/100",
 				Queue:        "queue1",
-				Change:       Change{Source: "github", IDs: []string{"PR-1", "PR-2", "PR-3"}},
+				Change:       Change{Source: "github", URIs: []string{"uber/repo-a/101@aaa111", "uber/repo-a/102@bbb222", "uber/repo-a/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{Source: "phabricator", URIs: []string{"D12345@abc123def456"}},
 				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{Source: "github-enterprise", URIs: []string{"internal/service/999@deadbeef12"}},
 				LandStrategy: RequestLandStrategyMerge,
 				State:        RequestStateError,
 				Version:      10,

extension/change_provider/change_provider.go

@@ -0,0 +1,34 @@
+package changeprovider
+
+import "context"
+
+// FileChangeStatus defines the type of modification made to a file.
+type FileChangeStatus string
+
+const (
+	// FileChangeStatusAdded indicates a new file was added.
+	FileChangeStatusAdded FileChangeStatus = "added"
+	// FileChangeStatusModified indicates an existing file was modified.
+	FileChangeStatusModified FileChangeStatus = "modified"
+	// FileChangeStatusDeleted indicates a file was deleted.
+	FileChangeStatusDeleted FileChangeStatus = "deleted"
+)
+
+// FileChange represents a single file modification in a change.
+type FileChange struct {
+	// Path is the file path relative to the repository root.
+	Path string
+	// Status is the type of change made to the file.
+	Status FileChangeStatus
+}
+
+// ChangeProvider fetches the list of changed files from external code review providers.
+// Each implementation is configured for a specific provider (GitHub, GitLab, Phabricator).
+type ChangeProvider interface {
+	// Get retrieves the list of files changed across all provided URIs.
+	// The URI format is provider-specific:
+	//   - GitHub: "owner/repo/pr_number@commit_hash" (e.g., "uber/submitqueue/123@abc123def")
+	//   - Phabricator: "revision_id@commit_hash" (e.g., "D12345@abc123def")
+	// Returns the aggregated list of changed files across all URIs.
+	Get(ctx context.Context, uris []string) ([]FileChange, 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_source, change_uris, 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, &req.Change.Source, &changeURIsJSON, &req.LandStrategy, &req.State, &req.Version)
 
 	if errors.Is(err, sql.ErrNoRows) {
 		return entity.Request{}, storage.WrapNotFound(err)
@@ -39,23 +39,23 @@ 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)
+	if err := json.Unmarshal(changeURIsJSON, &req.Change.URIs); err != nil {
+		return entity.Request{}, fmt.Errorf("failed to unmarshal change URIs for request entity id=%s from the database: %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)
+	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=%v id=%s for Create request entity: %w", request.Change.URIs, 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_source, change_uris, land_strategy, state, version) VALUES (?, ?, ?, ?, ?, ?, ?)",
+		request.ID, request.Queue, request.Change.Source, changeURIsJSON, request.LandStrategy, request.State, request.Version,
 	)
 	if err != nil {
 		var mysqlErr *mysql.MySQLError

extension/storage/mysql/schema/request.sql

@@ -2,7 +2,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_uris JSON NOT NULL,
     land_strategy VARCHAR(64) NOT NULL,
     state VARCHAR(64) NOT NULL,
     version INT NOT NULL,

gateway/controller/land.go

@@ -64,13 +64,13 @@ func (c *LandController) Land(ctx context.Context, req *pb.LandRequest) (*pb.Lan
 	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 len(req.Change.GetUris()) == 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.
@@ -105,7 +105,7 @@ func (c *LandController) Land(ctx context.Context, req *pb.LandRequest) (*pb.Lan
 		"queue", req.Queue,
 		"sqid", request.ID,
 		"change_source", change.Source,
-		"change_ids", change.IDs,
+		"change_uris", change.URIs,
 		"strategy", string(strategy),
 	)
 

gateway/controller/land_test.go

@@ -216,7 +216,7 @@ func TestLand_ReturnsSqid(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Source: "github", Uris: []string{"uber/test-repo/123@abc123def"}},
 	}
 	resp, err := controller.Land(ctx, req)
 
@@ -248,7 +248,7 @@ 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{Source: "github", Uris: []string{"uber/myservice/1@abc111", "uber/myservice/2@def222"}},
 		Strategy: pb.Strategy_REBASE,
 	}
 	resp, err := controller.Land(ctx, req)
@@ -257,7 +257,7 @@ func TestLand_PassesCorrectParametersToStore(t *testing.T) {
 	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{"uber/myservice/1@abc111", "uber/myservice/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)
@@ -285,7 +285,7 @@ func TestLand_ReturnsErrorOnStorageFailure(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Source: "github", Uris: []string{"uber/test-repo/123@abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -313,7 +313,7 @@ func TestLand_ReturnsErrorOnCounterFailure(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Source: "github", Uris: []string{"uber/test-repo/123@abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -344,7 +344,7 @@ func TestLand_CounterDomainIncludesQueue(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "my-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Source: "github", Uris: []string{"uber/test-repo/123@abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -366,7 +366,7 @@ func TestLand_ReturnsErrorOnEmptyQueue(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "",
-		Change: &pb.Change{Source: "github", Ids: []string{"123"}},
+		Change: &pb.Change{Source: "github", Uris: []string{"uber/test-repo/123@abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -388,7 +388,7 @@ func TestLand_ReturnsErrorOnEmptyChangeSource(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "", Ids: []string{"123"}},
+		Change: &pb.Change{Source: "", Uris: []string{"uber/test-repo/123@abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -432,7 +432,7 @@ func TestLand_ReturnsErrorOnEmptyChangeIDs(t *testing.T) {
 
 	req := &pb.LandRequest{
 		Queue:  "test-queue",
-		Change: &pb.Change{Source: "github", Ids: []string{}},
+		Change: &pb.Change{Source: "github", Uris: []string{}},
 	}
 	_, err := controller.Land(ctx, req)
 
@@ -463,7 +463,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{Source: "github", Uris: []string{"uber/backend/456@fed987cba"}},
 		Strategy: pb.Strategy_REBASE,
 	}
 	resp, err := controller.Land(ctx, req)
@@ -482,7 +482,7 @@ func TestLand_PublishesToQueue(t *testing.T) {
 	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{"uber/backend/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)
@@ -506,7 +506,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{Source: "github", Uris: []string{"uber/service/1@abc123def"}},
 	}
 	_, err := controller.Land(ctx, req)
 

gateway/proto/gateway.proto

@@ -41,14 +41,17 @@ enum Strategy {
     MERGE = 3;
 }
 
-// Change represents a set of related code changes identified by one or more IDs from a particular code change provider, like Github Pull Requests.
+// Change represents a set of related code changes identified by one or more URIs from a particular code change provider, like Github Pull Requests.
 message Change {
-    // The code change provider (e.g., "github", "gerrit", "phabricator").
+    // The code change provider ID that maps to a registered provider (e.g., "github", "github-enterprise", "phabricator").
     string source = 1;
-    // List of change IDs, in a format specific to the code change provider, that should be landed together. SubmitQueue guarantees that the changes are landed in the order of the list,
-    // and no other changes are landed in between. SubmitQueue does not guarantee that each change is individually valid, but produces a special validity
-    // marker on such changes. The user is responsible to include all changes in a change stack in the order of the list, starting from the earlist change.
-    repeated string ids = 2;
+    // List of change URIs that should be landed together. The format is provider-specific:
+    //   - GitHub: "owner/repo/pr_number@commit_hash" (e.g., "uber/submitqueue/123@abc123def")
+    //   - Phabricator: "revision_id@commit_hash" (e.g., "D12345@abc123def")
+    // SubmitQueue guarantees that the changes are landed in the order of the list, and no other changes are landed in between.
+    // SubmitQueue does not guarantee that each change is individually valid, but produces a special validity marker on such changes.
+    // The user is responsible to include all changes in a change stack in the order of the list, starting from the earliest change.
+    repeated string uris = 2;
 }
 
 // LandRequest defines a request to land (merge into target branch of the source control repository) a set of code changes.