diff --git a/.changes/unreleased/Fixed-20261027-205433.yaml b/.changes/unreleased/Fixed-20261027-205433.yaml new file mode 100644 index 000000000..8819cd456 --- /dev/null +++ b/.changes/unreleased/Fixed-20261027-205433.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: 'github: Fix GHE.com API hosts skipped during stack updates and merges.' +time: 2026-10-27T20:54:33.723586-07:00 diff --git a/internal/forge/github/change_match_test.go b/internal/forge/github/change_match_test.go index e9b8ef09b..e9a00c02a 100644 --- a/internal/forge/github/change_match_test.go +++ b/internal/forge/github/change_match_test.go @@ -171,7 +171,7 @@ func TestRepository_MatchChangesToBranches_filtersPushRepository(t *testing.T) { []string{"feature"}, &forge.MatchChangesToBranchesOptions{ PushRepository: &RepositoryID{ - url: DefaultURL, + url: "https://github.com", owner: "other", name: "fork", }, diff --git a/internal/forge/github/forge.go b/internal/forge/github/forge.go index 722a01249..ce9a87ed9 100644 --- a/internal/forge/github/forge.go +++ b/internal/forge/github/forge.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "net/http" - "net/url" "go.abhg.dev/gs/internal/forge" "go.abhg.dev/gs/internal/gateway/github" @@ -15,12 +14,6 @@ import ( "go.abhg.dev/gs/internal/silog" ) -// Default URLs for GitHub and its API. -const ( - DefaultURL = "https://github.com" - DefaultAPIURL = "https://api.github.com" -) - // Options defines command line options for the GitHub Forge. // These are all hidden in the CLI, // and are expected to be set only via environment variables. @@ -44,6 +37,10 @@ type Options struct { Stacks bool `name:"github-stacks" hidden:"" config:"forge.github.stacks" default:"true" help:"Whether to use GitHub native stack operations. One of 'true' and 'false'."` } +func (o Options) deployment() github.Deployment { + return github.Deployment{URL: o.URL, APIURL: o.APIURL} +} + // Definition configures GitHub forge instances. type Definition struct { changeMetadataCodec @@ -65,7 +62,7 @@ func (*Definition) ID() string { return "github" } // BaseURL reports the GitHub web URL used for host matching. func (d *Definition) BaseURL() string { - return cmp.Or(d.Options.URL, DefaultURL) + return d.Options.deployment().WebURL() } // CLIPlugin returns the CLI plugin for the GitHub Forge. @@ -116,7 +113,7 @@ func (f *Forge) logger() *silog.Logger { // URL returns the base URL configured for the GitHub Forge // or the default URL if none is set. func (f *Forge) URL() string { - return cmp.Or(f.Options.URL, DefaultURL) + return f.Options.deployment().WebURL() } // BaseURL reports the GitHub web URL used for host matching and links. @@ -124,25 +121,6 @@ func (f *Forge) BaseURL() string { return f.baseURL } -// APIURL returns the base API URL configured for the GitHub Forge -// or the default URL if none is set. -func (f *Forge) APIURL() string { - if f.Options.APIURL != "" { - return f.Options.APIURL - } - - // If the API URL is not set, and base URL is NOT github.com, - // assume API URL is $baseURL/api. - if f.Options.URL != "" && f.Options.URL != DefaultURL { - apiURL, err := url.JoinPath(f.Options.URL, "/api") - if err == nil { - return apiURL - } - } - - return DefaultAPIURL -} - // ID reports a unique key for this forge. func (*Forge) ID() string { return "github" } @@ -171,7 +149,11 @@ func (f *Forge) OpenRepository(ctx context.Context, tok forge.AuthenticationToke } gatewayTokens := newGatewayTokenSource(tokenSource) - gatewayClient, err := github.NewGateway(f.APIURL(), f.HTTPClient, gatewayTokens) + gatewayClient, err := github.NewGateway(github.Config{ + Deployment: f.Options.deployment(), + Tokens: gatewayTokens, + HTTPClient: f.HTTPClient, + }) if err != nil { return nil, fmt.Errorf("create GitHub gateway: %w", err) } diff --git a/internal/forge/github/forge_test.go b/internal/forge/github/forge_test.go index 060049e3f..323dd383e 100644 --- a/internal/forge/github/forge_test.go +++ b/internal/forge/github/forge_test.go @@ -1,6 +1,9 @@ package github import ( + "io" + "net/http" + "strings" "testing" "github.com/alecthomas/kong" @@ -51,25 +54,21 @@ func TestURLs(t *testing.T) { name string opts Options - wantURL string - wantAPIURL string + wantURL string }{ { - name: "Default", - wantURL: DefaultURL, - wantAPIURL: DefaultAPIURL, + name: "Default", + wantURL: "https://github.com", }, { - name: "ExplicitURL", - opts: Options{URL: DefaultURL}, - wantURL: DefaultURL, - wantAPIURL: DefaultAPIURL, + name: "ExplicitURL", + opts: Options{URL: "https://github.com"}, + wantURL: "https://github.com", }, { - name: "CustomURL", - opts: Options{URL: "https://example.com"}, - wantURL: "https://example.com", - wantAPIURL: "https://example.com/api", + name: "CustomURL", + opts: Options{URL: "https://example.com"}, + wantURL: "https://example.com", }, { name: "CustomBoth", @@ -77,27 +76,160 @@ func TestURLs(t *testing.T) { URL: "https://example.com", APIURL: "https://api.example.com", }, - wantURL: "https://example.com", - wantAPIURL: "https://api.example.com", + wantURL: "https://example.com", }, { - name: "InvalidURL", - opts: Options{URL: ":/"}, - wantURL: ":/", - wantAPIURL: DefaultAPIURL, + name: "InvalidURL", + opts: Options{URL: ":/"}, + wantURL: ":/", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { f := Forge{Options: tt.opts} + d := Definition{Options: tt.opts} assert.Equal(t, tt.wantURL, f.URL()) - assert.Equal(t, tt.wantAPIURL, f.APIURL()) + assert.Equal(t, tt.wantURL, d.BaseURL()) }) } } +func TestForge_OpenRepository_endpoints(t *testing.T) { + tests := []struct { + name string + opts Options + wantGQL string + wantREST string + }{ + { + name: "GitHub", + wantGQL: "https://api.github.com/graphql", + wantREST: "https://api.github.com", + }, + { + name: "ExplicitGitHub", + opts: Options{URL: "https://github.com"}, + wantGQL: "https://api.github.com/graphql", + wantREST: "https://api.github.com", + }, + { + name: "MalformedWebURL", + opts: Options{URL: ":/"}, + wantGQL: "https://api.github.com/graphql", + wantREST: "https://api.github.com", + }, + { + name: "EnterpriseCloud", + opts: Options{APIURL: "https://api.octocorp.ghe.com"}, + wantGQL: "https://api.octocorp.ghe.com/graphql", + wantREST: "https://api.octocorp.ghe.com", + }, + { + name: "EnterpriseCloudMixedCasePort", + opts: Options{APIURL: "https://API.OctoCorp.GHE.COM:443"}, + wantGQL: "https://API.OctoCorp.GHE.COM:443/graphql", + wantREST: "https://API.OctoCorp.GHE.COM:443", + }, + { + name: "Enterprise", + opts: Options{URL: "https://github.example.com"}, + wantGQL: "https://github.example.com/api/graphql", + wantREST: "https://github.example.com/api/v3", + }, + { + name: "WebPrefix", + opts: Options{URL: "https://github.example.com/service"}, + wantGQL: "https://github.example.com/service/api/graphql", + wantREST: "https://github.example.com/service/api/v3", + }, + { + name: "CustomAPI", + opts: Options{ + URL: "https://github.example.com", + APIURL: "https://proxy.example.com/github", + }, + wantGQL: "https://proxy.example.com/github/graphql", + wantREST: "https://proxy.example.com/github/v3", + }, + { + name: "EnterpriseCloudWebHost", + opts: Options{URL: "https://octocorp.ghe.com"}, + wantGQL: "https://octocorp.ghe.com/api/graphql", + wantREST: "https://octocorp.ghe.com/api/v3", + }, + { + name: "EnterpriseSimilarCloudHost", + opts: Options{APIURL: "https://api.octocorp.ghe.com.example.net/api"}, + wantGQL: "https://api.octocorp.ghe.com.example.net/api/graphql", + wantREST: "https://api.octocorp.ghe.com.example.net/api/v3", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stackURL := tt.wantREST + "/repos/octo/hello/stacks" + var requests []string + f := Forge{ + Options: tt.opts, + HTTPClient: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + requests = append(requests, r.URL.String()) + assert.Equal(t, "Bearer token", r.Header.Get("Authorization")) + status, body := http.StatusOK, "[]" + switch r.URL.String() { + case tt.wantGQL: + assert.Equal(t, http.MethodPost, r.Method) + body = `{"data":{"repository":{"id":"R_123"}}}` + case stackURL: + assert.Equal(t, http.MethodGet, r.Method) + default: + status, body = http.StatusNotFound, `{"message":"Not Found"}` + } + return &http.Response{ + StatusCode: status, + Body: io.NopCloser(strings.NewReader(body)), + }, nil + })}, + } + f.Options.Stacks = true + rid, err := f.ParseRepositoryPath("octo/hello") + require.NoError(t, err) + repo, err := f.OpenRepository(t.Context(), &AuthenticationToken{AccessToken: "token"}, rid) + require.NoError(t, err) + + _, err = repo.(forge.StackRepository).PlanMergeRanges(t.Context(), nil) + assert.NoError(t, err) + assert.Equal(t, []string{tt.wantGQL, stackURL}, requests) + }) + } +} + +func TestDefinition_New_invalidAPIURL(t *testing.T) { + d := Definition{Options: Options{ + URL: "https://github.example.com", + APIURL: ":/", + }} + assert.Equal(t, "https://github.example.com", d.BaseURL()) + remoteURL, err := giturl.Parse("https://github.example.com/octo/hello.git") + require.NoError(t, err) + f, err := d.New(remoteURL, &forge.NewOptions{ + HTTPClient: &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("unexpected HTTP request") + return nil, nil + })}, + }) + require.NoError(t, err) + + ep, err := f.(*Forge).oauth2Endpoint() + require.NoError(t, err) + assert.Equal(t, "https://github.example.com/login/oauth/access_token", ep.TokenURL) + + rid, err := f.ParseRepositoryPath("octo/hello") + require.NoError(t, err) + _, err = f.OpenRepository(t.Context(), &AuthenticationToken{AccessToken: "token"}, rid) + assert.ErrorContains(t, err, "parse API URL") +} + func TestExtractRepoInfo(t *testing.T) { tests := []struct { name string @@ -229,7 +361,7 @@ func TestForge_ParseRepositoryPath_knownForge(t *testing.T) { func TestChangeURL(t *testing.T) { repoID := RepositoryID{ - url: DefaultURL, + url: "https://github.com", owner: "example", name: "repo", } @@ -242,7 +374,7 @@ func TestRepository_ComparisonURL(t *testing.T) { r := &Repository{ owner: "example", repo: "repo", - forge: &Forge{Options: Options{URL: DefaultURL}}, + forge: &Forge{Options: Options{URL: "https://github.com"}}, } t.Run("SameRepository", func(t *testing.T) { @@ -258,7 +390,7 @@ func TestRepository_ComparisonURL(t *testing.T) { Base: "main", Head: "feat#review", HeadRepository: &RepositoryID{ - url: DefaultURL, + url: "https://github.com", owner: "fork", name: "repo", }, @@ -268,3 +400,7 @@ func TestRepository_ComparisonURL(t *testing.T) { got) }) } + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } diff --git a/internal/forge/github/integration_test.go b/internal/forge/github/integration_test.go index 031f16a05..9fe2c2d9a 100644 --- a/internal/forge/github/integration_test.go +++ b/internal/forge/github/integration_test.go @@ -60,11 +60,10 @@ func newRecorder( func newGateway(t *testing.T, httpClient *http.Client) *githubgateway.Gateway { t.Helper() - client, err := githubgateway.NewGateway( - github.DefaultAPIURL, - &http.Client{Transport: httpClient.Transport}, - testTokenSource("token"), - ) + client, err := githubgateway.NewGateway(githubgateway.Config{ + HTTPClient: &http.Client{Transport: httpClient.Transport}, + Tokens: testTokenSource("token"), + }) require.NoError(t, err) return client } diff --git a/internal/forge/github/main_test.go b/internal/forge/github/main_test.go index 1f7d920ae..eb0fa350c 100644 --- a/internal/forge/github/main_test.go +++ b/internal/forge/github/main_test.go @@ -13,18 +13,6 @@ import ( "go.abhg.dev/gs/internal/gateway/github" ) -type testGatewayTokenSource struct{} - -func (testGatewayTokenSource) Token(context.Context) (string, error) { - return "test-token", nil -} - -func newTestGateway(t *testing.T, apiURL string) *github.Gateway { - client, err := github.NewGateway(apiURL, nil, testGatewayTokenSource{}) - require.NoError(t, err) - return client -} - func TestMain(m *testing.M) { name := filepath.Base(os.Args[0]) if runtime.GOOS == "windows" { @@ -46,3 +34,19 @@ password=test-token os.Exit(m.Run()) } + +func newTestGateway(t *testing.T, apiURL string) *github.Gateway { + t.Helper() + client, err := github.NewGateway(github.Config{ + Deployment: github.Deployment{APIURL: apiURL}, + Tokens: testGatewayTokenSource{}, + }) + require.NoError(t, err) + return client +} + +type testGatewayTokenSource struct{} + +func (testGatewayTokenSource) Token(context.Context) (string, error) { + return "test-token", nil +} diff --git a/internal/forge/github/merge_test.go b/internal/forge/github/merge_test.go index 86a62d879..04edbe498 100644 --- a/internal/forge/github/merge_test.go +++ b/internal/forge/github/merge_test.go @@ -82,7 +82,10 @@ func TestRepository_MergeChange_method(t *testing.T) { })) defer srv.Close() - gateway, err := github.NewGateway(srv.URL, nil, gatewayTestTokenSource("token")) + gateway, err := github.NewGateway(github.Config{ + Deployment: github.Deployment{APIURL: srv.URL}, + Tokens: gatewayTestTokenSource("token"), + }) require.NoError(t, err) repo, err := newRepository( t.Context(), new(Forge), diff --git a/internal/forge/github/submit_test.go b/internal/forge/github/submit_test.go index 90c336262..08482b26f 100644 --- a/internal/forge/github/submit_test.go +++ b/internal/forge/github/submit_test.go @@ -67,11 +67,11 @@ func TestRepository_SubmitChange_fromPushRepository(t *testing.T) { })) })) defer srv.Close() - gatewayClient, err := github.NewGateway( - srv.URL, - srv.Client(), - gatewayTestTokenSource("token"), - ) + gatewayClient, err := github.NewGateway(github.Config{ + Deployment: github.Deployment{APIURL: srv.URL}, + HTTPClient: srv.Client(), + Tokens: gatewayTestTokenSource("token"), + }) require.NoError(t, err) repo, err := newRepository( @@ -176,11 +176,11 @@ func TestRepository_addPullRequestMetadata(t *testing.T) { } })) defer srv.Close() - gatewayClient, err := github.NewGateway( - srv.URL, - srv.Client(), - gatewayTestTokenSource("token"), - ) + gatewayClient, err := github.NewGateway(github.Config{ + Deployment: github.Deployment{APIURL: srv.URL}, + HTTPClient: srv.Client(), + Tokens: gatewayTestTokenSource("token"), + }) require.NoError(t, err) repo, err := newRepository( diff --git a/internal/gateway/github/deployment.go b/internal/gateway/github/deployment.go new file mode 100644 index 000000000..5fcca36aa --- /dev/null +++ b/internal/gateway/github/deployment.go @@ -0,0 +1,74 @@ +package github + +import ( + "cmp" + "fmt" + "net/url" + "strings" +) + +// Deployment configures the addresses of a GitHub installation. +// The zero value selects GitHub.com. +// API addresses are resolved when constructing a [Gateway]. +type Deployment struct { + // URL is the web URL. If empty, https://github.com is used. + URL string + + // APIURL overrides the common base URL for the GraphQL and REST APIs. + // For GitHub Enterprise Server, this normally ends in /api, + // before the REST API's /v3 suffix. + // If empty, GitHub.com uses its public API host, + // and custom web URLs use an /api subpath. + APIURL string +} + +// WebURL returns the configured web URL or the GitHub.com default. +// It does not parse or validate either address. +func (d Deployment) WebURL() string { + return cmp.Or(d.URL, defaultWebURL) +} + +type apiEndpoints struct { + GraphQLEndpoint *url.URL + RESTBaseURL *url.URL +} + +// apiEndpoints resolves the common API base into privately owned protocol URLs. +// It returns an error if an explicit APIURL cannot be parsed. +func (d Deployment) apiEndpoints() (apiEndpoints, error) { + apiURL := &url.URL{Scheme: "https", Host: defaultAPIHost} + switch { + case d.APIURL != "": + var err error + apiURL, err = url.Parse(d.APIURL) + if err != nil { + return apiEndpoints{}, fmt.Errorf("parse API URL: %w", err) + } + case d.URL != "" && d.URL != defaultWebURL: + // Retain the API default if the web URL cannot be parsed. + // Web operations validate their own URL. + if webURL, err := url.Parse(d.URL); err == nil { + apiURL = webURL.JoinPath("/api") + } + } + + // GitHub.com and GHE.com serve GraphQL below the REST API host, + // while GitHub Enterprise Server uses /api/graphql and /api/v3. + apiHost := strings.ToLower(apiURL.Hostname()) + isGitHubCloud := apiHost == defaultAPIHost || + (strings.HasPrefix(apiHost, "api.") && strings.HasSuffix(apiHost, ".ghe.com")) + restBaseURL := apiURL + if !isGitHubCloud { + restBaseURL = apiURL.JoinPath("v3") + } + + return apiEndpoints{ + GraphQLEndpoint: apiURL.JoinPath("graphql"), + RESTBaseURL: restBaseURL, + }, nil +} + +const ( + defaultWebURL = "https://github.com" + defaultAPIHost = "api.github.com" +) diff --git a/internal/gateway/github/gateway.go b/internal/gateway/github/gateway.go index 239cbc73d..3c243df2a 100644 --- a/internal/gateway/github/gateway.go +++ b/internal/gateway/github/gateway.go @@ -1,10 +1,11 @@ // Package github provides the typed GitHub API operations needed by git-spice. // -// The package owns GitHub's GraphQL and REST wire protocols, +// The package owns GitHub's deployment URL conventions, +// GraphQL and REST wire protocols, // authenticated request execution, // and response error models. -// Callers supply credentials through [TokenSource] and adapt the typed results -// to their own domain models. +// Callers supply a [Deployment] through [Config] and credentials through +// [TokenSource], and adapt the typed results to their own domain models. // The package does not own credential discovery, persistence, or login flows. package github @@ -15,7 +16,6 @@ import ( "io" "net/http" "net/url" - "strings" ) // maxErrorBody limits the diagnostic content retained from an HTTP error @@ -34,16 +34,16 @@ type TokenSource interface { // A Gateway is safe for concurrent use when its HTTP client and token source are // safe for concurrent use. // Each HTTP request retrieves a token with the operation context and uses the -// configured GitHub endpoint. +// configured GitHub endpoints. // A typed operation may issue multiple requests when GitHub exposes related // data through separate API nodes. // GitHub responses containing both data and errors return only the errors; // callers never observe a partial result. type Gateway struct { - // graphQLEndpoint is derived from the configured API base URL. - graphQLEndpoint string + // graphQLEndpoint is the complete URL for GraphQL requests. + graphQLEndpoint *url.URL - // restBaseURL is the REST API root derived from the configured API base URL. + // restBaseURL is the root to which REST resource paths are appended. restBaseURL *url.URL // httpClient performs requests after the gateway has supplied authentication. @@ -53,42 +53,40 @@ type Gateway struct { tokens TokenSource } -// NewGateway builds a client for a GitHub API base URL. -// -// apiURLStr is the API base URL reported by the forge configuration. -// For GitHub Enterprise Server, this is the common `/api` root from which -// NewGateway derives `/api/graphql` and `/api/v3`. -// A nil HTTP client uses [http.DefaultClient]. -// The tokens source is required and is consulted for every HTTP request. -func NewGateway(apiURLStr string, httpClient *http.Client, tokens TokenSource) (*Gateway, error) { - apiURL, err := url.Parse(apiURLStr) - if err != nil { - return nil, fmt.Errorf("parse API URL: %w", err) - } +// Config configures a Gateway's deployment and authenticated HTTP requests. +type Config struct { + // Deployment selects the GitHub installation. + // Its zero value selects GitHub.com. + Deployment Deployment - graphQLURL := apiURL.JoinPath("graphql") + // Tokens supplies credentials for every HTTP request. + Tokens TokenSource // required - // TODO: Accept complete GraphQL and REST endpoint URLs instead of deriving - // both endpoints from GitHub's common API base URL. - // GitHub.com serves GraphQL below the REST API host, - // while GitHub Enterprise Server uses /api/graphql and /api/v3. - restBaseURL := apiURL - if !strings.EqualFold(apiURL.Hostname(), "api.github.com") { - restBaseURL = apiURL.JoinPath("v3") + // HTTPClient sends requests to both endpoints. + // If nil, [http.DefaultClient] is used. + HTTPClient *http.Client +} + +// NewGateway resolves the deployment's API addresses and builds a client. +func NewGateway(cfg Config) (*Gateway, error) { + endpoints, err := cfg.Deployment.apiEndpoints() + if err != nil { + return nil, err } + httpClient := cfg.HTTPClient if httpClient == nil { httpClient = http.DefaultClient } - if tokens == nil { + if cfg.Tokens == nil { return nil, errors.New("token source is required") } return &Gateway{ - graphQLEndpoint: graphQLURL.String(), - restBaseURL: restBaseURL, + graphQLEndpoint: endpoints.GraphQLEndpoint, + restBaseURL: endpoints.RESTBaseURL, httpClient: httpClient, - tokens: tokens, + tokens: cfg.Tokens, }, nil } diff --git a/internal/gateway/github/gateway_test.go b/internal/gateway/github/gateway_test.go index 4fd836c26..9881e01ed 100644 --- a/internal/gateway/github/gateway_test.go +++ b/internal/gateway/github/gateway_test.go @@ -14,36 +14,38 @@ import ( "github.com/stretchr/testify/require" ) -func TestNewGateway_endpoint(t *testing.T) { - tests := []struct { - name string - give string - wantGQL string - wantREST string - }{ - { - name: "GitHub", - give: "https://api.github.com", - wantGQL: "https://api.github.com/graphql", - wantREST: "https://api.github.com", - }, - { - name: "Enterprise", - give: "https://github.example.com/api", - wantGQL: "https://github.example.com/api/graphql", - wantREST: "https://github.example.com/api/v3", +func TestNewGateway_endpoints(t *testing.T) { + var requests []string + gateway, err := NewGateway(Config{ + Deployment: Deployment{ + URL: "https://github.example.com", + APIURL: "https://proxy.example.com/service", }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gateway, err := NewGateway(tt.give, nil, tokenSourceFunc(func(context.Context) (string, error) { - return "token", nil - })) - require.NoError(t, err) - assert.Equal(t, tt.wantGQL, gateway.graphQLEndpoint) - assert.Equal(t, tt.wantREST, gateway.restBaseURL.String()) - }) - } + Tokens: tokenSourceFunc(func(context.Context) (string, error) { + return "token", nil + }), + HTTPClient: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + requests = append(requests, r.URL.String()) + body := "[]" + if r.Method == http.MethodPost { + body = `{"data":{"repository":{"id":"R_123"}}}` + } + return &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(body)), + }, nil + })}, + }) + require.NoError(t, err) + + id, err := gateway.RepositoryID(t.Context(), "octo", "hello") + require.NoError(t, err) + assert.Equal(t, ID("R_123"), id) + require.NoError(t, gateway.CheckPullRequestStacks(t.Context(), "octo", "hello")) + assert.Equal(t, []string{ + "https://proxy.example.com/service/graphql", + "https://proxy.example.com/service/v3/repos/octo/hello/stacks", + }, requests) } func TestGateway_RepositoryID_request(t *testing.T) { @@ -69,11 +71,15 @@ func TestGateway_RepositoryID_request(t *testing.T) { }) var tokenCalls int - gateway, err := NewGateway("https://github.example.com/api", &http.Client{Transport: transport}, tokenSourceFunc(func(ctx context.Context) (string, error) { - assert.Same(t, t.Context(), ctx) - tokenCalls++ - return "secret", nil - })) + gateway, err := NewGateway(Config{ + Deployment: Deployment{URL: "https://github.example.com"}, + HTTPClient: &http.Client{Transport: transport}, + Tokens: tokenSourceFunc(func(ctx context.Context) (string, error) { + assert.Same(t, t.Context(), ctx) + tokenCalls++ + return "secret", nil + }), + }) require.NoError(t, err) id, err := gateway.RepositoryID(t.Context(), "octo", "hello") @@ -84,9 +90,11 @@ func TestGateway_RepositoryID_request(t *testing.T) { func TestGateway_RepositoryID_tokenError(t *testing.T) { want := errors.New("token unavailable") - gateway, err := NewGateway("https://api.github.com", nil, tokenSourceFunc(func(context.Context) (string, error) { - return "", want - })) + gateway, err := NewGateway(Config{ + Tokens: tokenSourceFunc(func(context.Context) (string, error) { + return "", want + }), + }) require.NoError(t, err) _, err = gateway.RepositoryID(t.Context(), "octo", "hello") @@ -96,9 +104,11 @@ func TestGateway_RepositoryID_tokenError(t *testing.T) { func TestGateway_RepositoryID_cancellation(t *testing.T) { ctx, cancel := context.WithCancel(t.Context()) cancel() - gateway, err := NewGateway("https://api.github.com", nil, tokenSourceFunc(func(ctx context.Context) (string, error) { - return "", ctx.Err() - })) + gateway, err := NewGateway(Config{ + Tokens: tokenSourceFunc(func(ctx context.Context) (string, error) { + return "", ctx.Err() + }), + }) require.NoError(t, err) _, err = gateway.RepositoryID(ctx, "octo", "hello") @@ -258,9 +268,12 @@ func newResponseGateway(t *testing.T, body string) *Gateway { func newTestGateway(t *testing.T, transport http.RoundTripper) *Gateway { t.Helper() - gateway, err := NewGateway("https://api.github.com", &http.Client{Transport: transport}, tokenSourceFunc(func(context.Context) (string, error) { - return "token", nil - })) + gateway, err := NewGateway(Config{ + HTTPClient: &http.Client{Transport: transport}, + Tokens: tokenSourceFunc(func(context.Context) (string, error) { + return "token", nil + }), + }) require.NoError(t, err) return gateway } diff --git a/internal/gateway/github/graphql.go b/internal/gateway/github/graphql.go index 620850738..20dbd8455 100644 --- a/internal/gateway/github/graphql.go +++ b/internal/gateway/github/graphql.go @@ -63,7 +63,7 @@ func (c *Gateway) executeGQL(ctx context.Context, query string, variables, resul return fmt.Errorf("encode GraphQL request: %w", err) } - req, err := c.newRequest(ctx, http.MethodPost, c.graphQLEndpoint, &body) + req, err := c.newRequest(ctx, http.MethodPost, c.graphQLEndpoint.String(), &body) if err != nil { return err } diff --git a/internal/gateway/github/rest_test.go b/internal/gateway/github/rest_test.go index abd0cadd8..d42662fa4 100644 --- a/internal/gateway/github/rest_test.go +++ b/internal/gateway/github/rest_test.go @@ -15,18 +15,17 @@ import ( func TestGateway_RESTAuthenticationRefresh(t *testing.T) { var tokenCalls int - gateway, err := NewGateway( - "https://api.github.com", - &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + gateway, err := NewGateway(Config{ + HTTPClient: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { assert.Equal(t, fmt.Sprintf("Bearer token-%d", tokenCalls), r.Header.Get("Authorization")) return restJSONResponse(http.StatusCreated, `{}`), nil })}, - tokenSourceFunc(func(ctx context.Context) (string, error) { + Tokens: tokenSourceFunc(func(ctx context.Context) (string, error) { assert.Same(t, t.Context(), ctx) tokenCalls++ return fmt.Sprintf("token-%d", tokenCalls), nil }), - ) + }) require.NoError(t, err) for range 2 { @@ -42,16 +41,15 @@ func TestGateway_RESTAuthenticationRefresh(t *testing.T) { func TestGateway_RESTTokenError(t *testing.T) { want := errors.New("token unavailable") - gateway, err := NewGateway( - "https://api.github.com", - &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + gateway, err := NewGateway(Config{ + HTTPClient: &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { t.Fatal("HTTP request made after token failure") return nil, nil })}, - tokenSourceFunc(func(context.Context) (string, error) { + Tokens: tokenSourceFunc(func(context.Context) (string, error) { return "", want }), - ) + }) require.NoError(t, err) err = gateway.CreatePullRequestStack(t.Context(), &CreatePullRequestStackInput{