Skip to content

Commit 50e8544

Browse files
feat(provider): implement change provider interface with GitHub integration
- Add ChangeProvider interface and GitHub implementation - Use Client wrapper pattern with configurable auth - Add tests for the change provider
1 parent 31d6859 commit 50e8544

16 files changed

Lines changed: 1186 additions & 30 deletions

File tree

‎example/server/orchestrator/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ go_library(
1313
deps = [
1414
"//core/consumer",
1515
"//entity",
16+
"//extension/changeprovider",
17+
"//extension/changeprovider/github",
1618
"//extension/counter",
1719
"//extension/counter/mysql",
1820
"//extension/mergechecker",

‎example/server/orchestrator/main.go‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import (
3131
"github.com/uber-go/tally/v4"
3232
"github.com/uber/submitqueue/core/consumer"
3333
"github.com/uber/submitqueue/entity"
34+
"github.com/uber/submitqueue/extension/changeprovider"
35+
githubprovider "github.com/uber/submitqueue/extension/changeprovider/github"
3436
"github.com/uber/submitqueue/extension/counter"
3537
mysqlcounter "github.com/uber/submitqueue/extension/counter/mysql"
3638
"github.com/uber/submitqueue/extension/mergechecker"
@@ -190,8 +192,11 @@ func run() error {
190192
// Create merge checker
191193
mc := newMergeChecker(logger, scope)
192194

195+
// Create change provider
196+
cp := newChangeProvider(logger, scope)
197+
193198
// Register controllers
194-
if err := registerControllers(c, logger.Sugar(), scope, registry, mc, cnt, store); err != nil {
199+
if err := registerControllers(c, logger.Sugar(), scope, registry, mc, cp, cnt, store); err != nil {
195200
return err
196201
}
197202

@@ -376,7 +381,7 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
376381
// │ │ │
377382
// └────────┴────────────────────────┘
378383

379-
func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope tally.Scope, registry consumer.TopicRegistry, mc mergechecker.MergeChecker, cnt counter.Counter, store storage.Storage) error {
384+
func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope tally.Scope, registry consumer.TopicRegistry, mc mergechecker.MergeChecker, cp changeprovider.ChangeProvider, cnt counter.Counter, store storage.Storage) error {
380385
requestController := start.NewController(
381386
logger,
382387
scope,
@@ -395,6 +400,7 @@ func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope t
395400
store,
396401
registry,
397402
mc,
403+
cp,
398404
consumer.TopicKeyValidate,
399405
"orchestrator-validate",
400406
)
@@ -514,6 +520,36 @@ func registerControllers(c consumer.Consumer, logger *zap.SugaredLogger, scope t
514520
return nil
515521
}
516522

523+
// getEnv returns environment variable value or default if not set.
524+
func getEnv(key, defaultVal string) string {
525+
if val := os.Getenv(key); val != "" {
526+
return val
527+
}
528+
return defaultVal
529+
}
530+
531+
// parseTimeout parses a duration from environment variable with fallback to default.
532+
// Returns defaultVal if envVal is empty or cannot be parsed.
533+
func parseTimeout(envVal string, defaultVal time.Duration) time.Duration {
534+
if envVal == "" {
535+
return defaultVal
536+
}
537+
if d, err := time.ParseDuration(envVal); err == nil {
538+
return d
539+
}
540+
return defaultVal
541+
}
542+
543+
// buildGitHubHTTPClient creates an http.Client configured for GitHub API calls.
544+
// Configures timeout and optional bearer token authentication.
545+
func buildGitHubHTTPClient(token string, timeout time.Duration) *http.Client {
546+
httpClient := &http.Client{Timeout: timeout}
547+
if token != "" {
548+
httpClient.Transport = &bearerTransport{token: token}
549+
}
550+
return httpClient
551+
}
552+
517553
// newMergeChecker creates a MergeChecker for GitHub (github.com).
518554
// Configured via GITHUB_TOKEN and GITHUB_GRAPHQL_URL environment variables.
519555
func newMergeChecker(logger *zap.Logger, scope tally.Scope) mergechecker.MergeChecker {
@@ -539,6 +575,26 @@ func newMergeChecker(logger *zap.Logger, scope tally.Scope) mergechecker.MergeCh
539575
})
540576
}
541577

578+
// newChangeProvider creates a ChangeProvider for GitHub (github.com).
579+
// Configured via GITHUB_BASE_URL, GITHUB_TOKEN, and GITHUB_TIMEOUT environment variables.
580+
// Uses pure dependency injection - creates http.Client with auth configured in Transport.
581+
func newChangeProvider(logger *zap.Logger, scope tally.Scope) changeprovider.ChangeProvider {
582+
// 1. Read configuration from environment
583+
baseURL := getEnv("GITHUB_BASE_URL", "https://api.github.com")
584+
token := os.Getenv("GITHUB_TOKEN")
585+
timeout := parseTimeout(os.Getenv("GITHUB_TIMEOUT"), 30*time.Second)
586+
587+
// 2. Build HTTP client with caller-controlled config (auth + timeout)
588+
httpClient := buildGitHubHTTPClient(token, timeout)
589+
590+
// 3. Inject into provider
591+
return githubprovider.NewProvider(githubprovider.Params{
592+
Client: githubprovider.NewClient(httpClient, baseURL),
593+
Logger: logger.Sugar(),
594+
MetricsScope: scope.SubScope("changeprovider"),
595+
})
596+
}
597+
542598
// bearerTransport is an http.RoundTripper that adds a Bearer token to requests.
543599
type bearerTransport struct {
544600
token string

‎extension/changeprovider/change_provider.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@ type ChangedFile struct {
4444

4545
// ChangeInfo contains metadata and file changes for a code change.
4646
type ChangeInfo struct {
47-
// ID is the change identifier (e.g., "PR: uber-code/go-code/1" or "diff: uber-code/go-code/D1").
48-
ID string
47+
// URI is the full change URI for correlation with the input request
48+
// (e.g., "github://uber/repo/98/abc123sha" or "phab://D123/xyz789").
49+
URI string
4950
// User is the author of the change.
5051
User User
5152
// ChangedFiles is the list of files modified in this change. Order is unspecified.
@@ -56,6 +57,7 @@ type ChangeInfo struct {
5657
// Each implementation is configured for a specific provider (GitHub, GitLab, Phabricator).
5758
type ChangeProvider interface {
5859
// Get retrieves change information for the provided Change.
59-
// Returns the change info containing metadata and file changes.
60-
Get(ctx context.Context, change entity.Change) (ChangeInfo, error)
60+
// For a Change with multiple URIs (e.g., stacked PRs), returns one ChangeInfo per URI.
61+
// Returns a slice of ChangeInfo, one for each change in the stack.
62+
Get(ctx context.Context, change entity.Change) ([]ChangeInfo, error)
6163
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "github",
5+
srcs = [
6+
"config.go",
7+
"convert.go",
8+
"graphql.go",
9+
"provider.go",
10+
"validate.go",
11+
],
12+
importpath = "github.com/uber/submitqueue/extension/changeprovider/github",
13+
visibility = ["//visibility:public"],
14+
deps = [
15+
"//core/metrics",
16+
"//entity",
17+
"//entity/github",
18+
"//extension/changeprovider",
19+
"@com_github_uber_go_tally_v4//:tally",
20+
"@org_uber_go_zap//:zap",
21+
],
22+
)
23+
24+
go_test(
25+
name = "github_test",
26+
srcs = [
27+
"config_test.go",
28+
"graphql_test.go",
29+
"provider_test.go",
30+
"validate_test.go",
31+
],
32+
embed = [":github"],
33+
deps = [
34+
"//entity",
35+
"//entity/github",
36+
"//extension/changeprovider",
37+
"@com_github_stretchr_testify//assert",
38+
"@com_github_stretchr_testify//require",
39+
"@com_github_uber_go_tally_v4//:tally",
40+
"@org_uber_go_zap//zaptest",
41+
],
42+
)
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package github
2+
3+
import (
4+
"net/http"
5+
)
6+
7+
// Client is a GitHub API client that encapsulates connection details and authentication.
8+
// The client is protocol-agnostic - it provides helpers for both GraphQL and REST endpoints.
9+
type Client struct {
10+
httpClient *http.Client
11+
baseURL string
12+
}
13+
14+
// NewClient creates a new GitHub API client with a pre-configured HTTP client.
15+
// The caller is responsible for configuring authentication in the HTTP client's Transport.
16+
//
17+
// Parameters:
18+
// - httpClient: Configured HTTP client (with auth, timeout, transport, etc.)
19+
// - baseURL: GitHub instance base URL (e.g., "https://api.github.com" or "https://ghe.company.com/api")
20+
func NewClient(httpClient *http.Client, baseURL string) *Client {
21+
return &Client{
22+
httpClient: httpClient,
23+
baseURL: baseURL,
24+
}
25+
}
26+
27+
// HTTPClient returns the configured HTTP client.
28+
func (c *Client) HTTPClient() *http.Client {
29+
return c.httpClient
30+
}
31+
32+
// BaseURL returns the configured GitHub base URL.
33+
func (c *Client) BaseURL() string {
34+
return c.baseURL
35+
}
36+
37+
// GraphQLURL returns the GitHub GraphQL endpoint URL.
38+
// Constructs the URL by appending "/graphql" to the base URL.
39+
func (c *Client) GraphQLURL() string {
40+
return c.baseURL + "/graphql"
41+
}
42+
43+
// RESTURL constructs a GitHub REST API endpoint URL.
44+
// The path should start with "/" (e.g., "/repos/uber/submitqueue/pulls/123").
45+
func (c *Client) RESTURL(path string) string {
46+
return c.baseURL + path
47+
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package github
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
"time"
7+
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestNewClient(t *testing.T) {
12+
httpClient := &http.Client{Timeout: 30 * time.Second}
13+
baseURL := "https://api.github.com"
14+
15+
client := NewClient(httpClient, baseURL)
16+
17+
assert.NotNil(t, client)
18+
assert.Equal(t, httpClient, client.HTTPClient())
19+
assert.Equal(t, baseURL, client.BaseURL())
20+
}
21+
22+
func TestClient_GraphQLURL(t *testing.T) {
23+
tests := []struct {
24+
name string
25+
baseURL string
26+
expected string
27+
}{
28+
{
29+
name: "standard github",
30+
baseURL: "https://api.github.com",
31+
expected: "https://api.github.com/graphql",
32+
},
33+
{
34+
name: "github enterprise",
35+
baseURL: "https://ghe.example.com/api",
36+
expected: "https://ghe.example.com/api/graphql",
37+
},
38+
{
39+
name: "localhost",
40+
baseURL: "http://localhost:8080",
41+
expected: "http://localhost:8080/graphql",
42+
},
43+
}
44+
45+
for _, tt := range tests {
46+
t.Run(tt.name, func(t *testing.T) {
47+
client := NewClient(&http.Client{}, tt.baseURL)
48+
assert.Equal(t, tt.expected, client.GraphQLURL())
49+
})
50+
}
51+
}
52+
53+
func TestClient_BaseURL(t *testing.T) {
54+
tests := []struct {
55+
name string
56+
baseURL string
57+
}{
58+
{name: "standard github", baseURL: "https://api.github.com"},
59+
{name: "github enterprise", baseURL: "https://ghe.example.com/api"},
60+
}
61+
62+
for _, tt := range tests {
63+
t.Run(tt.name, func(t *testing.T) {
64+
client := NewClient(&http.Client{}, tt.baseURL)
65+
assert.Equal(t, tt.baseURL, client.BaseURL())
66+
})
67+
}
68+
}
69+
70+
func TestClient_RESTURL(t *testing.T) {
71+
tests := []struct {
72+
name string
73+
baseURL string
74+
path string
75+
expected string
76+
}{
77+
{
78+
name: "repos endpoint",
79+
baseURL: "https://api.github.com",
80+
path: "/repos/uber/submitqueue/pulls/123",
81+
expected: "https://api.github.com/repos/uber/submitqueue/pulls/123",
82+
},
83+
{
84+
name: "enterprise repos endpoint",
85+
baseURL: "https://ghe.example.com/api",
86+
path: "/repos/myorg/myrepo/pulls/456",
87+
expected: "https://ghe.example.com/api/repos/myorg/myrepo/pulls/456",
88+
},
89+
}
90+
91+
for _, tt := range tests {
92+
t.Run(tt.name, func(t *testing.T) {
93+
client := NewClient(&http.Client{}, tt.baseURL)
94+
assert.Equal(t, tt.expected, client.RESTURL(tt.path))
95+
})
96+
}
97+
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package github
2+
3+
import (
4+
entitygithub "github.com/uber/submitqueue/entity/github"
5+
"github.com/uber/submitqueue/extension/changeprovider"
6+
)
7+
8+
// convertToChangeInfo converts GitHub PR data to ChangeInfo.
9+
func convertToChangeInfo(parsed entitygithub.ChangeID, prData *pullRequestData) changeprovider.ChangeInfo {
10+
changedFiles := convertFiles(prData.Files.Nodes)
11+
12+
return changeprovider.ChangeInfo{
13+
URI: parsed.String(),
14+
User: changeprovider.User{
15+
Name: prData.Author.Name,
16+
Email: prData.Author.Email,
17+
},
18+
ChangedFiles: changedFiles,
19+
}
20+
}
21+
22+
// convertFiles converts GitHub file nodes to ChangedFile structs.
23+
func convertFiles(nodes []fileNode) []changeprovider.ChangedFile {
24+
changedFiles := make([]changeprovider.ChangedFile, 0, len(nodes))
25+
26+
for _, file := range nodes {
27+
linesModified := 0
28+
if file.Additions > 0 && file.Deletions > 0 {
29+
linesModified = min(file.Additions, file.Deletions)
30+
}
31+
32+
changedFiles = append(changedFiles, changeprovider.ChangedFile{
33+
Path: file.Path,
34+
Patch: file.Patch,
35+
LinesAdded: file.Additions,
36+
LinesDeleted: file.Deletions,
37+
LinesModified: linesModified,
38+
})
39+
}
40+
41+
return changedFiles
42+
}

0 commit comments

Comments
 (0)