From 5de99072a8c2397694edaf38faee3dba6e870111 Mon Sep 17 00:00:00 2001 From: Will Roden Date: Thu, 24 Aug 2023 14:28:25 -0500 Subject: [PATCH 1/6] Add WithOptions --- example/tokenauth/main.go | 3 +- github/github.go | 175 ++++++++++++++++++++++++++------------ 2 files changed, 121 insertions(+), 57 deletions(-) diff --git a/example/tokenauth/main.go b/example/tokenauth/main.go index a9e845bed30..564db2663e6 100644 --- a/example/tokenauth/main.go +++ b/example/tokenauth/main.go @@ -3,7 +3,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// The tokenauth command demonstrates using the oauth2.StaticTokenSource. +// The tokenauth command demonstrates using a Personal Access Token (PAT) to +// authenticate with GitHub. // You can test out a GitHub Personal Access Token using this simple example. // You can generate them here: https://github.com/settings/tokens package main diff --git a/github/github.go b/github/github.go index aad4a7335d6..d4f0a7d040e 100644 --- a/github/github.go +++ b/github/github.go @@ -306,16 +306,98 @@ func addOptions(s string, opts interface{}) (string, error) { // NewClient returns a new GitHub API client. If a nil httpClient is // provided, a new http.Client will be used. To use API methods which require -// authentication, provide an http.Client that will perform the authentication -// for you (such as that provided by the golang.org/x/oauth2 library). +// authentication, either use NewTokenClient instead or provide NewClient with +// an http.Client that will perform the authentication for you (such as that +// provided by the golang.org/x/oauth2 library). func NewClient(httpClient *http.Client) *Client { - if httpClient == nil { - httpClient = &http.Client{} + c := &Client{ + client: httpClient, } - baseURL, _ := url.Parse(defaultBaseURL) - uploadURL, _ := url.Parse(uploadBaseURL) + // empty WithOptions always returns a nil error. + _ = c.initialize() + return c +} + +type ClientOption func(*Client) error + +// WithAuthToken configures the client to use the provided token for the Authorization header. +func WithAuthToken(token string) ClientOption { + return func(c *Client) error { + c.client.Transport = &oauth2.Transport{ + Source: oauth2.StaticTokenSource( + &oauth2.Token{AccessToken: token}, + ), + Base: c.client.Transport, + } + return nil + } +} + +// WithEnterpriseURLs configures the client to use the provided base and upload URLs. +// If the base URL does not have the suffix "/api/v3/", it will be added automatically. +// If the upload URL does not have the suffix "/api/uploads", it will be added automatically. +// +// Note that WithEnterpriseURLs is a convenience helper only; +// its behavior is equivalent to setting the BaseURL and UploadURL fields. +// +// Another important thing is that by default, the GitHub Enterprise URL format +// should be http(s)://[hostname]/api/v3/ or you will always receive the 406 status code. +// The upload URL format should be http(s)://[hostname]/api/uploads/. +func WithEnterpriseURLs(baseURL, uploadURL string) ClientOption { + return func(c *Client) error { + var err error + c.BaseURL, err = url.Parse(baseURL) + if err != nil { + return err + } - c := &Client{client: httpClient, BaseURL: baseURL, UserAgent: defaultUserAgent, UploadURL: uploadURL} + if !strings.HasSuffix(c.BaseURL.Path, "/") { + c.BaseURL.Path += "/" + } + if !strings.HasSuffix(c.BaseURL.Path, "/api/v3/") && + !strings.HasPrefix(c.BaseURL.Host, "api.") && + !strings.Contains(c.BaseURL.Host, ".api.") { + c.BaseURL.Path += "api/v3/" + } + + c.UploadURL, err = url.Parse(uploadURL) + if err != nil { + return err + } + + if !strings.HasSuffix(c.UploadURL.Path, "/") { + c.UploadURL.Path += "/" + } + if !strings.HasSuffix(c.UploadURL.Path, "/api/uploads/") && + !strings.HasPrefix(c.UploadURL.Host, "api.") && + !strings.Contains(c.UploadURL.Host, ".api.") { + c.UploadURL.Path += "api/uploads/" + } + return nil + } +} + +// initialize applies the given options to the client, sets defaults and +// initializes services. +func (c *Client) initialize(opts ...ClientOption) error { + if c.client == nil { + c.client = &http.Client{} + } + for _, opt := range opts { + err := opt(c) + if err != nil { + return err + } + } + if c.BaseURL == nil { + c.BaseURL, _ = url.Parse(defaultBaseURL) + } + if c.UploadURL == nil { + c.UploadURL, _ = url.Parse(uploadBaseURL) + } + if c.UserAgent == "" { + c.UserAgent = defaultUserAgent + } c.common.client = c c.Actions = (*ActionsService)(&c.common) c.Activity = (*ActivityService)(&c.common) @@ -349,65 +431,46 @@ func NewClient(httpClient *http.Client) *Client { c.SecurityAdvisories = (*SecurityAdvisoriesService)(&c.common) c.Teams = (*TeamsService)(&c.common) c.Users = (*UsersService)(&c.common) - return c + return nil +} + +// WithOptions returns a clone of the client with the specified options. +func (c *Client) WithOptions(opts ...ClientOption) (*Client, error) { + c.clientMu.Lock() + clone := &Client{ + client: c.client, + UserAgent: c.UserAgent, + BaseURL: c.BaseURL, + UploadURL: c.UploadURL, + secondaryRateLimitReset: c.secondaryRateLimitReset, + } + c.clientMu.Unlock() + c.rateMu.Lock() + copy(clone.rateLimits[:], c.rateLimits[:]) + c.rateMu.Unlock() + err := clone.initialize(opts...) + if err != nil { + return nil, err + } + return clone, nil } // NewClientWithEnvProxy enhances NewClient with the HttpProxy env. +// Deprecated: Use NewClient(nil) instead. func NewClientWithEnvProxy() *Client { return NewClient(&http.Client{Transport: &http.Transport{Proxy: http.ProxyFromEnvironment}}) } -// NewTokenClient returns a new GitHub API client authenticated with the provided token. -func NewTokenClient(ctx context.Context, token string) *Client { - return NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}))) +// Deprecated: Use NewClient(nil).WithOptions(WithAuthToken(token)) instead. +func NewTokenClient(_ context.Context, token string) *Client { + // WithAuthToken always returns a nil error. + c, _ := NewClient(nil).WithOptions(WithAuthToken(token)) + return c } -// NewEnterpriseClient returns a new GitHub API client with provided -// base URL and upload URL (often is your GitHub Enterprise hostname). -// If the base URL does not have the suffix "/api/v3/", it will be added automatically. -// If the upload URL does not have the suffix "/api/uploads", it will be added automatically. -// If a nil httpClient is provided, a new http.Client will be used. -// -// Note that NewEnterpriseClient is a convenience helper only; -// its behavior is equivalent to using NewClient, followed by setting -// the BaseURL and UploadURL fields. -// -// Another important thing is that by default, the GitHub Enterprise URL format -// should be http(s)://[hostname]/api/v3/ or you will always receive the 406 status code. -// The upload URL format should be http(s)://[hostname]/api/uploads/. +// Deprecated: Use NewClient(httpClient).WithOptions(WithEnterpriseURLs(baseURL, uploadURL)) instead. func NewEnterpriseClient(baseURL, uploadURL string, httpClient *http.Client) (*Client, error) { - baseEndpoint, err := url.Parse(baseURL) - if err != nil { - return nil, err - } - - if !strings.HasSuffix(baseEndpoint.Path, "/") { - baseEndpoint.Path += "/" - } - if !strings.HasSuffix(baseEndpoint.Path, "/api/v3/") && - !strings.HasPrefix(baseEndpoint.Host, "api.") && - !strings.Contains(baseEndpoint.Host, ".api.") { - baseEndpoint.Path += "api/v3/" - } - - uploadEndpoint, err := url.Parse(uploadURL) - if err != nil { - return nil, err - } - - if !strings.HasSuffix(uploadEndpoint.Path, "/") { - uploadEndpoint.Path += "/" - } - if !strings.HasSuffix(uploadEndpoint.Path, "/api/uploads/") && - !strings.HasPrefix(uploadEndpoint.Host, "api.") && - !strings.Contains(uploadEndpoint.Host, ".api.") { - uploadEndpoint.Path += "api/uploads/" - } - - c := NewClient(httpClient) - c.BaseURL = baseEndpoint - c.UploadURL = uploadEndpoint - return c, nil + return NewClient(httpClient).WithOptions(WithEnterpriseURLs(baseURL, uploadURL)) } // RequestOption represents an option that can modify an http.Request. From 77c8b76e896aa53b730a783479bfa76337c9cc01 Mon Sep 17 00:00:00 2001 From: Will Roden Date: Fri, 25 Aug 2023 10:25:16 -0500 Subject: [PATCH 2/6] update tests and examples --- example/actionpermissions/main.go | 2 +- example/appengine/app.go | 11 +- .../newreposecretwithxcrypto/main.go | 2 +- .../newusersecretwithxcrypto/main.go | 2 +- example/commitpr/main.go | 2 +- example/listenvironments/main.go | 2 +- example/migrations/main.go | 2 +- example/newfilewithappauth/main.go | 11 +- example/newrepo/main.go | 2 +- example/newreposecretwithlibsodium/main.go | 2 +- example/newreposecretwithxcrypto/main.go | 2 +- example/tagprotection/main.go | 2 +- example/tokenauth/main.go | 2 +- github/github_test.go | 306 +++++++----------- test/fields/fields.go | 2 +- test/integration/github_test.go | 2 +- 16 files changed, 138 insertions(+), 216 deletions(-) diff --git a/example/actionpermissions/main.go b/example/actionpermissions/main.go index 40f4a86ece0..9a32d91eb01 100644 --- a/example/actionpermissions/main.go +++ b/example/actionpermissions/main.go @@ -35,7 +35,7 @@ func main() { log.Fatal("No owner: owner of repo must be given") } ctx := context.Background() - client := github.NewTokenClient(ctx, token) + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) actionsPermissionsRepository, _, err := client.Repositories.GetActionsPermissions(ctx, *owner, *name) if err != nil { diff --git a/example/appengine/app.go b/example/appengine/app.go index 1694faaacfd..9fc769c0439 100644 --- a/example/appengine/app.go +++ b/example/appengine/app.go @@ -17,8 +17,18 @@ import ( "google.golang.org/appengine/log" ) +var client = github.NewClient(nil) + func init() { http.HandleFunc("/", handler) + token := os.Getenv("GITHUB_AUTH_TOKEN") + if token != "" { + var err error + client, err = client.WithOptions(github.WithAuthToken(token)) + if err != nil { + panic(err) + } + } } func handler(w http.ResponseWriter, r *http.Request) { @@ -28,7 +38,6 @@ func handler(w http.ResponseWriter, r *http.Request) { } ctx := appengine.NewContext(r) - client := github.NewTokenClient(ctx, os.Getenv("GITHUB_AUTH_TOKEN")) commits, _, err := client.Repositories.ListCommits(ctx, "google", "go-github", nil) if err != nil { diff --git a/example/codespaces/newreposecretwithxcrypto/main.go b/example/codespaces/newreposecretwithxcrypto/main.go index 4ec3757af25..a7b16eedbce 100644 --- a/example/codespaces/newreposecretwithxcrypto/main.go +++ b/example/codespaces/newreposecretwithxcrypto/main.go @@ -72,7 +72,7 @@ func main() { } ctx := context.Background() - client := github.NewTokenClient(ctx, token) + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) if err := addRepoSecret(ctx, client, *owner, *repo, secretName, secretValue); err != nil { log.Fatal(err) diff --git a/example/codespaces/newusersecretwithxcrypto/main.go b/example/codespaces/newusersecretwithxcrypto/main.go index 964452da82b..2f6e4175fb0 100644 --- a/example/codespaces/newusersecretwithxcrypto/main.go +++ b/example/codespaces/newusersecretwithxcrypto/main.go @@ -65,7 +65,7 @@ func main() { } ctx := context.Background() - client := github.NewTokenClient(ctx, token) + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) if err := addUserSecret(ctx, client, secretName, secretValue, *owner, *repo); err != nil { log.Fatal(err) diff --git a/example/commitpr/main.go b/example/commitpr/main.go index 2e81387017c..55e5b303bd8 100644 --- a/example/commitpr/main.go +++ b/example/commitpr/main.go @@ -189,7 +189,7 @@ func main() { if *sourceOwner == "" || *sourceRepo == "" || *commitBranch == "" || *sourceFiles == "" || *authorName == "" || *authorEmail == "" { log.Fatal("You need to specify a non-empty value for the flags `-source-owner`, `-source-repo`, `-commit-branch`, `-files`, `-author-name` and `-author-email`") } - client = github.NewTokenClient(ctx, token) + client, _ = github.NewClient(nil).WithOptions(github.WithAuthToken(token)) ref, err := getRef() if err != nil { diff --git a/example/listenvironments/main.go b/example/listenvironments/main.go index 15ec8cbb583..fe23e4454b3 100644 --- a/example/listenvironments/main.go +++ b/example/listenvironments/main.go @@ -31,7 +31,7 @@ func main() { } ctx := context.Background() - client := github.NewTokenClient(ctx, token) + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) expectedPageSize := 2 diff --git a/example/migrations/main.go b/example/migrations/main.go index f5c36c63059..97cd9874736 100644 --- a/example/migrations/main.go +++ b/example/migrations/main.go @@ -17,7 +17,7 @@ import ( func fetchAllUserMigrations() ([]*github.UserMigration, error) { ctx := context.Background() - client := github.NewTokenClient(ctx, "") + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken("")) migrations, _, err := client.Migrations.ListUserMigrations(ctx, &github.ListOptions{Page: 1}) return migrations, err diff --git a/example/newfilewithappauth/main.go b/example/newfilewithappauth/main.go index 6526fadc35e..4888a12bde1 100644 --- a/example/newfilewithappauth/main.go +++ b/example/newfilewithappauth/main.go @@ -35,13 +35,13 @@ func main() { itr.BaseURL = gitHost //create git client with app transport - client, err := github.NewEnterpriseClient( - gitHost, - gitHost, + client, err := github.NewClient( &http.Client{ Transport: itr, Timeout: time.Second * 30, - }) + }, + ).WithOptions(github.WithEnterpriseURLs(gitHost, gitHost)) + if err != nil { log.Fatalf("faild to create git client for app: %v\n", err) } @@ -71,8 +71,7 @@ func main() { ) oAuthClient := oauth2.NewClient(context.Background(), ts) - //create new git hub client with accessToken - apiClient, err := github.NewEnterpriseClient(gitHost, gitHost, oAuthClient) + apiClient, err := github.NewClient(oAuthClient).WithOptions(github.WithEnterpriseURLs(gitHost, gitHost)) if err != nil { log.Fatalf("failed to create new git client with token: %v\n", err) } diff --git a/example/newrepo/main.go b/example/newrepo/main.go index 758b293db23..26d557abd3d 100644 --- a/example/newrepo/main.go +++ b/example/newrepo/main.go @@ -36,7 +36,7 @@ func main() { log.Fatal("No name: New repos must be given a name") } ctx := context.Background() - client := github.NewTokenClient(ctx, token) + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) r := &github.Repository{Name: name, Private: private, Description: description, AutoInit: autoInit} repo, _, err := client.Repositories.Create(ctx, "", r) diff --git a/example/newreposecretwithlibsodium/main.go b/example/newreposecretwithlibsodium/main.go index 039289054ee..1b1a12b2786 100644 --- a/example/newreposecretwithlibsodium/main.go +++ b/example/newreposecretwithlibsodium/main.go @@ -71,7 +71,7 @@ func main() { } ctx := context.Background() - client := github.NewTokenClient(ctx, token) + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) if err != nil { log.Fatalf("unable to authorize using env GITHUB_AUTH_TOKEN: %v", err) } diff --git a/example/newreposecretwithxcrypto/main.go b/example/newreposecretwithxcrypto/main.go index 76eb3dd8741..b9a3f85cb21 100644 --- a/example/newreposecretwithxcrypto/main.go +++ b/example/newreposecretwithxcrypto/main.go @@ -72,7 +72,7 @@ func main() { } ctx := context.Background() - client := github.NewTokenClient(ctx, token) + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) if err := addRepoSecret(ctx, client, *owner, *repo, secretName, secretValue); err != nil { log.Fatal(err) diff --git a/example/tagprotection/main.go b/example/tagprotection/main.go index 8f85c7c0cb8..d867191ba48 100644 --- a/example/tagprotection/main.go +++ b/example/tagprotection/main.go @@ -44,7 +44,7 @@ func main() { token := string(byteToken) ctx := context.Background() - client := github.NewTokenClient(ctx, token) + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) // create new tag protection if pattern != "" { diff --git a/example/tokenauth/main.go b/example/tokenauth/main.go index 564db2663e6..64cef754d0c 100644 --- a/example/tokenauth/main.go +++ b/example/tokenauth/main.go @@ -26,7 +26,7 @@ func main() { token := string(byteToken) ctx := context.Background() - client := github.NewTokenClient(ctx, token) + client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) user, resp, err := client.Users.Get(ctx, "") if err != nil { diff --git a/github/github_test.go b/github/github_test.go index 94606edd72b..b3a0d9225d0 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -320,202 +320,116 @@ func TestNewTokenClient(t *testing.T) { } } -func TestNewEnterpriseClient(t *testing.T) { - baseURL := "https://custom-url/api/v3/" - uploadURL := "https://custom-upload-url/api/uploads/" - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), baseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), uploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) - } -} - -func TestNewEnterpriseClient_addsTrailingSlashToURLs(t *testing.T) { - baseURL := "https://custom-url/api/v3" - uploadURL := "https://custom-upload-url/api/uploads" - formattedBaseURL := baseURL + "/" - formattedUploadURL := uploadURL + "/" - - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), formattedBaseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), formattedUploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) - } -} - -func TestNewEnterpriseClient_addsEnterpriseSuffixToURLs(t *testing.T) { - baseURL := "https://custom-url/" - uploadURL := "https://custom-upload-url/" - formattedBaseURL := baseURL + "api/v3/" - formattedUploadURL := uploadURL + "api/uploads/" - - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), formattedBaseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), formattedUploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) - } -} - -func TestNewEnterpriseClient_addsEnterpriseSuffixAndTrailingSlashToURLs(t *testing.T) { - baseURL := "https://custom-url" - uploadURL := "https://custom-upload-url" - formattedBaseURL := baseURL + "/api/v3/" - formattedUploadURL := uploadURL + "/api/uploads/" - - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), formattedBaseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), formattedUploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) - } -} - -func TestNewEnterpriseClient_badBaseURL(t *testing.T) { - baseURL := "bogus\nbase\nURL" - uploadURL := "https://custom-upload-url/api/uploads/" - if _, err := NewEnterpriseClient(baseURL, uploadURL, nil); err == nil { - t.Fatal("NewEnterpriseClient returned nil, expected error") - } -} - -func TestNewEnterpriseClient_badUploadURL(t *testing.T) { - baseURL := "https://custom-url/api/v3/" - uploadURL := "bogus\nupload\nURL" - if _, err := NewEnterpriseClient(baseURL, uploadURL, nil); err == nil { - t.Fatal("NewEnterpriseClient returned nil, expected error") - } -} - -func TestNewEnterpriseClient_URLHasExistingAPIPrefix_AddTrailingSlash(t *testing.T) { - baseURL := "https://api.custom-url" - uploadURL := "https://api.custom-upload-url" - formattedBaseURL := baseURL + "/" - formattedUploadURL := uploadURL + "/" - - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), formattedBaseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), formattedUploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) - } -} - -func TestNewEnterpriseClient_URLHasExistingAPIPrefixAndTrailingSlash(t *testing.T) { - baseURL := "https://api.custom-url/" - uploadURL := "https://api.custom-upload-url/" - - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), baseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), uploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) - } -} - -func TestNewEnterpriseClient_URLHasAPISubdomain_AddTrailingSlash(t *testing.T) { - baseURL := "https://catalog.api.custom-url" - uploadURL := "https://catalog.api.custom-upload-url" - formattedBaseURL := baseURL + "/" - formattedUploadURL := uploadURL + "/" - - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), formattedBaseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), formattedUploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) - } -} - -func TestNewEnterpriseClient_URLHasAPISubdomainAndTrailingSlash(t *testing.T) { - baseURL := "https://catalog.api.custom-url/" - uploadURL := "https://catalog.api.custom-upload-url/" - - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), baseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), uploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) - } -} - -func TestNewEnterpriseClient_URLIsNotAProperAPISubdomain_addsEnterpriseSuffixAndSlash(t *testing.T) { - baseURL := "https://cloud-api.custom-url" - uploadURL := "https://cloud-api.custom-upload-url" - formattedBaseURL := baseURL + "/api/v3/" - formattedUploadURL := uploadURL + "/api/uploads/" - - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), formattedBaseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), formattedUploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) - } -} - -func TestNewEnterpriseClient_URLIsNotAProperAPISubdomain_addsEnterpriseSuffix(t *testing.T) { - baseURL := "https://cloud-api.custom-url/" - uploadURL := "https://cloud-api.custom-upload-url/" - formattedBaseURL := baseURL + "api/v3/" - formattedUploadURL := uploadURL + "api/uploads/" - - c, err := NewEnterpriseClient(baseURL, uploadURL, nil) - if err != nil { - t.Fatalf("NewEnterpriseClient returned unexpected error: %v", err) - } - - if got, want := c.BaseURL.String(), formattedBaseURL; got != want { - t.Errorf("NewClient BaseURL is %v, want %v", got, want) - } - if got, want := c.UploadURL.String(), formattedUploadURL; got != want { - t.Errorf("NewClient UploadURL is %v, want %v", got, want) +func TestWithEnterpriseURLs(t *testing.T) { + for _, test := range []struct { + name string + baseURL string + wantBaseURL string + uploadURL string + wantUploadURL string + wantErr string + }{ + { + name: "does not modify properly formed URLs", + baseURL: "https://custom-url/api/v3/", + wantBaseURL: "https://custom-url/api/v3/", + uploadURL: "https://custom-upload-url/api/uploads/", + wantUploadURL: "https://custom-upload-url/api/uploads/", + }, + { + name: "adds trailing slash", + baseURL: "https://custom-url/api/v3", + wantBaseURL: "https://custom-url/api/v3/", + uploadURL: "https://custom-upload-url/api/uploads", + wantUploadURL: "https://custom-upload-url/api/uploads/", + }, + { + name: "adds enterprise suffix", + baseURL: "https://custom-url/", + wantBaseURL: "https://custom-url/api/v3/", + uploadURL: "https://custom-upload-url/", + wantUploadURL: "https://custom-upload-url/api/uploads/", + }, + { + name: "adds enterprise suffix and trailing slash", + baseURL: "https://custom-url", + wantBaseURL: "https://custom-url/api/v3/", + uploadURL: "https://custom-upload-url", + wantUploadURL: "https://custom-upload-url/api/uploads/", + }, + { + name: "bad base URL", + baseURL: "bogus\nbase\nURL", + uploadURL: "https://custom-upload-url/api/uploads/", + wantErr: `invalid control character in URL`, + }, + { + name: "bad upload URL", + baseURL: "https://custom-url/api/v3/", + uploadURL: "bogus\nupload\nURL", + wantErr: `invalid control character in URL`, + }, + { + name: "URL has existing API prefix, adds trailing slash", + baseURL: "https://api.custom-url", + wantBaseURL: "https://api.custom-url/", + uploadURL: "https://api.custom-upload-url", + wantUploadURL: "https://api.custom-upload-url/", + }, + { + name: "URL has existing API prefix and trailing slash", + baseURL: "https://api.custom-url/", + wantBaseURL: "https://api.custom-url/", + uploadURL: "https://api.custom-upload-url/", + wantUploadURL: "https://api.custom-upload-url/", + }, + { + name: "URL has API subdomain, adds trailing slash", + baseURL: "https://catalog.api.custom-url", + wantBaseURL: "https://catalog.api.custom-url/", + uploadURL: "https://catalog.api.custom-upload-url", + wantUploadURL: "https://catalog.api.custom-upload-url/", + }, + { + name: "URL has API subdomain and trailing slash", + baseURL: "https://catalog.api.custom-url/", + wantBaseURL: "https://catalog.api.custom-url/", + uploadURL: "https://catalog.api.custom-upload-url/", + wantUploadURL: "https://catalog.api.custom-upload-url/", + }, + { + name: "URL is not a proper API subdomain, adds enterprise suffix and slash", + baseURL: "https://cloud-api.custom-url", + wantBaseURL: "https://cloud-api.custom-url/api/v3/", + uploadURL: "https://cloud-api.custom-upload-url", + wantUploadURL: "https://cloud-api.custom-upload-url/api/uploads/", + }, + { + name: "URL is not a proper API subdomain, adds enterprise suffix", + baseURL: "https://cloud-api.custom-url/", + wantBaseURL: "https://cloud-api.custom-url/api/v3/", + uploadURL: "https://cloud-api.custom-upload-url/", + wantUploadURL: "https://cloud-api.custom-upload-url/api/uploads/", + }, + } { + t.Run(test.name, func(t *testing.T) { + c, err := NewClient(nil).WithOptions(WithEnterpriseURLs(test.baseURL, test.uploadURL)) + if test.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), test.wantErr) { + t.Fatalf("error does not contain expected string %q: %v", test.wantErr, err) + } + return + } + if err != nil { + t.Fatalf("got unexpected error: %v", err) + } + if c.BaseURL.String() != test.wantBaseURL { + t.Errorf("BaseURL is %v, want %v", c.BaseURL.String(), test.wantBaseURL) + } + if c.UploadURL.String() != test.wantUploadURL { + t.Errorf("UploadURL is %v, want %v", c.UploadURL.String(), test.wantUploadURL) + } + }) } } diff --git a/test/fields/fields.go b/test/fields/fields.go index 8f5d1ffc05e..136a9307c0d 100644 --- a/test/fields/fields.go +++ b/test/fields/fields.go @@ -46,7 +46,7 @@ func main() { print("!!! No OAuth token. Some tests won't run. !!!\n\n") client = github.NewClient(nil) } else { - client = github.NewTokenClient(context.Background(), token) + client, _ = github.NewClient(nil).WithOptions(github.WithAuthToken(token)) auth = true } diff --git a/test/integration/github_test.go b/test/integration/github_test.go index 9bcbf7b554a..ead4e1aca2d 100644 --- a/test/integration/github_test.go +++ b/test/integration/github_test.go @@ -32,7 +32,7 @@ func init() { print("!!! No OAuth token. Some tests won't run. !!!\n\n") client = github.NewClient(nil) } else { - client = github.NewTokenClient(context.Background(), token) + client, _ = github.NewClient(nil).WithOptions(github.WithAuthToken(token)) auth = true } } From b38f8930de59044c608f44567b3e04d558a7eb53 Mon Sep 17 00:00:00 2001 From: Will Roden Date: Sun, 27 Aug 2023 16:34:07 -0500 Subject: [PATCH 3/6] update docs --- README.md | 43 +----- example/actionpermissions/main.go | 2 +- example/appengine/app.go | 11 +- .../newreposecretwithxcrypto/main.go | 2 +- .../newusersecretwithxcrypto/main.go | 2 +- example/commitpr/main.go | 2 +- example/go.mod | 1 - example/go.sum | 39 ----- example/listenvironments/main.go | 2 +- example/migrations/main.go | 2 +- example/newfilewithappauth/main.go | 12 +- example/newrepo/main.go | 2 +- example/newreposecretwithlibsodium/main.go | 5 +- example/newreposecretwithxcrypto/main.go | 2 +- example/tagprotection/main.go | 2 +- example/tokenauth/main.go | 2 +- github/doc.go | 26 +--- github/github.go | 141 ++++++++---------- github/github_test.go | 2 +- test/fields/fields.go | 2 +- test/integration/github_test.go | 2 +- 21 files changed, 97 insertions(+), 207 deletions(-) diff --git a/README.md b/README.md index 3eb9947c158..fae03041ffb 100644 --- a/README.md +++ b/README.md @@ -84,36 +84,18 @@ For more sample code snippets, head over to the ### Authentication ### -The go-github library does not directly handle authentication. Instead, when -creating a new client, pass an `http.Client` that can handle authentication for -you. The easiest and recommended way to do this is using the [oauth2][] -library, but you can always use any other library that provides an -`http.Client`. If you have an OAuth2 access token (for example, a [personal -API token][]), you can use it with the oauth2 library using: +Use the `WithAuthToken` method to configure your client to authenticate using an +OAuth token (for example, a [personal access token][]). This is what is needed +for a majority of use cases aside from GitHub Apps. ```go -import "golang.org/x/oauth2" - -func main() { - ctx := context.Background() - ts := oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: "... your access token ..."}, - ) - tc := oauth2.NewClient(ctx, ts) - - client := github.NewClient(tc) - - // list all repositories for the authenticated user - repos, _, err := client.Repositories.List(ctx, "", nil) -} +client := github.NewClient(nil).WithAuthToken("... your access token ...") ``` Note that when using an authenticated Client, all calls made by the client will include the specified OAuth token. Therefore, authenticated clients should almost never be shared between different users. -See the [oauth2 docs][] for complete instructions on using that library. - For API methods that require HTTP Basic Authentication, use the [`BasicAuthTransport`](https://godoc.org/github.com/google/go-github/github#BasicAuthTransport). @@ -232,16 +214,9 @@ https://github.com/gregjones/httpcache for that. For example: ```go import "github.com/gregjones/httpcache" - ts := oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: os.Getenv("GITHUB_TOKEN")}, - ) - tc := &http.Client{ - Transport: &oauth2.Transport{ - Base: httpcache.NewMemoryCacheTransport(), - Source: ts, - }, - } - client := github.NewClient(tc) + client := github.NewClient( + httpcache.NewMemoryCacheTransport().Client() + ).WithAuthToken(os.Getenv("GITHUB_TOKEN")) ``` Learn more about GitHub conditional requests at @@ -320,9 +295,7 @@ Furthermore, there are libraries like [cbrgm/githubevents][] that build upon the For complete usage of go-github, see the full [package docs][]. [GitHub API v3]: https://docs.github.com/en/rest -[oauth2]: https://github.com/golang/oauth2 -[oauth2 docs]: https://godoc.org/golang.org/x/oauth2 -[personal API token]: https://github.com/blog/1509-personal-api-tokens +[personal access token]: https://github.com/blog/1509-personal-api-tokens [package docs]: https://pkg.go.dev/github.com/google/go-github/v54/github [GraphQL API v4]: https://developer.github.com/v4/ [shurcooL/githubv4]: https://github.com/shurcooL/githubv4 diff --git a/example/actionpermissions/main.go b/example/actionpermissions/main.go index 9a32d91eb01..0a6bedbb3f6 100644 --- a/example/actionpermissions/main.go +++ b/example/actionpermissions/main.go @@ -35,7 +35,7 @@ func main() { log.Fatal("No owner: owner of repo must be given") } ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client := github.NewClient(nil).WithAuthToken(token) actionsPermissionsRepository, _, err := client.Repositories.GetActionsPermissions(ctx, *owner, *name) if err != nil { diff --git a/example/appengine/app.go b/example/appengine/app.go index 9fc769c0439..1913d9a891f 100644 --- a/example/appengine/app.go +++ b/example/appengine/app.go @@ -17,18 +17,8 @@ import ( "google.golang.org/appengine/log" ) -var client = github.NewClient(nil) - func init() { http.HandleFunc("/", handler) - token := os.Getenv("GITHUB_AUTH_TOKEN") - if token != "" { - var err error - client, err = client.WithOptions(github.WithAuthToken(token)) - if err != nil { - panic(err) - } - } } func handler(w http.ResponseWriter, r *http.Request) { @@ -38,6 +28,7 @@ func handler(w http.ResponseWriter, r *http.Request) { } ctx := appengine.NewContext(r) + client := github.NewClient(nil).WithAuthToken(os.Getenv("GITHUB_AUTH_TOKEN")) commits, _, err := client.Repositories.ListCommits(ctx, "google", "go-github", nil) if err != nil { diff --git a/example/codespaces/newreposecretwithxcrypto/main.go b/example/codespaces/newreposecretwithxcrypto/main.go index a7b16eedbce..0faf737daf8 100644 --- a/example/codespaces/newreposecretwithxcrypto/main.go +++ b/example/codespaces/newreposecretwithxcrypto/main.go @@ -72,7 +72,7 @@ func main() { } ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client := github.NewClient(nil).WithAuthToken(token) if err := addRepoSecret(ctx, client, *owner, *repo, secretName, secretValue); err != nil { log.Fatal(err) diff --git a/example/codespaces/newusersecretwithxcrypto/main.go b/example/codespaces/newusersecretwithxcrypto/main.go index 2f6e4175fb0..d887b4e5f40 100644 --- a/example/codespaces/newusersecretwithxcrypto/main.go +++ b/example/codespaces/newusersecretwithxcrypto/main.go @@ -65,7 +65,7 @@ func main() { } ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client := github.NewClient(nil).WithAuthToken(token) if err := addUserSecret(ctx, client, secretName, secretValue, *owner, *repo); err != nil { log.Fatal(err) diff --git a/example/commitpr/main.go b/example/commitpr/main.go index 55e5b303bd8..6cdc0e8f046 100644 --- a/example/commitpr/main.go +++ b/example/commitpr/main.go @@ -189,7 +189,7 @@ func main() { if *sourceOwner == "" || *sourceRepo == "" || *commitBranch == "" || *sourceFiles == "" || *authorName == "" || *authorEmail == "" { log.Fatal("You need to specify a non-empty value for the flags `-source-owner`, `-source-repo`, `-commit-branch`, `-files`, `-author-name` and `-author-email`") } - client, _ = github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client = github.NewClient(nil).WithAuthToken(token) ref, err := getRef() if err != nil { diff --git a/example/go.mod b/example/go.mod index aa04906aa9c..496140a1b63 100644 --- a/example/go.mod +++ b/example/go.mod @@ -7,7 +7,6 @@ require ( github.com/gofri/go-github-ratelimit v1.0.3 github.com/google/go-github/v54 v54.0.0 golang.org/x/crypto v0.12.0 - golang.org/x/oauth2 v0.7.0 golang.org/x/term v0.11.0 google.golang.org/appengine v1.6.7 ) diff --git a/example/go.sum b/example/go.sum index 1151ab7548d..513022c3565 100644 --- a/example/go.sum +++ b/example/go.sum @@ -1,10 +1,8 @@ -cloud.google.com/go/compute/metadata v0.2.0/go.mod h1:zFmK7XCadkQkj6TtorcaGlCW1hT1fIilQDwofLpJ20k= github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 h1:wPbRQzjjwFc0ih8puEVAOFGELsn1zoIIYdxvML7mDxA= github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8/go.mod h1:I0gYDMZ6Z5GRU7l58bNFSkPTFN6Yl12dsUlAZ8xy98g= github.com/bradleyfalzon/ghinstallation/v2 v2.0.4 h1:tXKVfhE7FcSkhkv0UwkLvPDeZ4kz6OXd0PKPlFqf81M= github.com/bradleyfalzon/ghinstallation/v2 v2.0.4/go.mod h1:B40qPqJxWE0jDZgOR1JmaMy+4AY1eBP+IByOvqyAKp0= github.com/bwesterb/go-ristretto v1.2.0/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0= -github.com/bwesterb/go-ristretto v1.2.3/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0= github.com/cloudflare/circl v1.1.0/go.mod h1:prBCrKB9DV4poKZY1l9zBXg2QJY7mvgRvtMxxK7fi4I= github.com/cloudflare/circl v1.3.3 h1:fE/Qz0QdIGqeWfnwq0RE0R7MI51s0M2E4Ga9kq5AEMs= github.com/cloudflare/circl v1.3.3/go.mod h1:5XYMA4rFBvNIrhs50XuiBJ15vF2pZn4nnUKZrLbUZFA= @@ -20,71 +18,34 @@ github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiu github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= -github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-github/v41 v41.0.0 h1:HseJrM2JFf2vfiZJ8anY2hqBjdfY1Vlj/K27ueww4gg= github.com/google/go-github/v41 v41.0.0/go.mod h1:XgmCA5H323A9rtgExdTcnDkcqp6S30AVACCBDOonIxg= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= -github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210817164053-32db794688a5/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.3.1-0.20221117191849-2c476679df9a/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4= golang.org/x/crypto v0.12.0 h1:tFM/ta59kqch6LlvYnPa0yx5a83cL2nHflFhYKvv9Yk= golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw= -golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= -golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= -golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= -golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= -golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns= golang.org/x/net v0.10.0 h1:X2//UzNDwYmtCLn7To6G58Wr6f5ahEAQgKNzv9Y951M= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= -golang.org/x/oauth2 v0.7.0 h1:qe6s0zUXlPX80/dITx3440hWZ7GwMwgDDyrSGTPJG/g= -golang.org/x/oauth2 v0.7.0/go.mod h1:hPLQkd9LyjfXTiRohC/41GhcFqxisoUQ99sCUOHO9x4= -golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211007075335-d3039528d8ac/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= -golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= -golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY= -golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.11.0 h1:F9tnn/DA/Im8nCwm+fX+1/eBwi4qFjRT++MhtVC4ZX0= golang.org/x/term v0.11.0/go.mod h1:zC9APTIj3jG3FdV/Ons+XE1riIZXG4aZ4GTHiPZJPIU= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= -golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= -golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= -golang.org/x/text v0.12.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= -golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c= google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= diff --git a/example/listenvironments/main.go b/example/listenvironments/main.go index fe23e4454b3..e2ff0c4b456 100644 --- a/example/listenvironments/main.go +++ b/example/listenvironments/main.go @@ -31,7 +31,7 @@ func main() { } ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client := github.NewClient(nil).WithAuthToken(token) expectedPageSize := 2 diff --git a/example/migrations/main.go b/example/migrations/main.go index 97cd9874736..d87668c1479 100644 --- a/example/migrations/main.go +++ b/example/migrations/main.go @@ -17,7 +17,7 @@ import ( func fetchAllUserMigrations() ([]*github.UserMigration, error) { ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken("")) + client := github.NewClient(nil).WithAuthToken("") migrations, _, err := client.Migrations.ListUserMigrations(ctx, &github.ListOptions{Page: 1}) return migrations, err diff --git a/example/newfilewithappauth/main.go b/example/newfilewithappauth/main.go index 4888a12bde1..8b2a4987585 100644 --- a/example/newfilewithappauth/main.go +++ b/example/newfilewithappauth/main.go @@ -17,7 +17,6 @@ import ( "github.com/bradleyfalzon/ghinstallation/v2" "github.com/google/go-github/v54/github" - "golang.org/x/oauth2" ) func main() { @@ -40,7 +39,7 @@ func main() { Transport: itr, Timeout: time.Second * 30, }, - ).WithOptions(github.WithEnterpriseURLs(gitHost, gitHost)) + ).WithEnterpriseURLs(gitHost, gitHost) if err != nil { log.Fatalf("faild to create git client for app: %v\n", err) @@ -66,12 +65,9 @@ func main() { log.Fatalf("failed to create installation token: %v\n", err) } - ts := oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: token.GetToken()}, - ) - oAuthClient := oauth2.NewClient(context.Background(), ts) - - apiClient, err := github.NewClient(oAuthClient).WithOptions(github.WithEnterpriseURLs(gitHost, gitHost)) + apiClient, err := github.NewClient(nil).WithAuthToken( + token.GetToken(), + ).WithEnterpriseURLs(gitHost, gitHost) if err != nil { log.Fatalf("failed to create new git client with token: %v\n", err) } diff --git a/example/newrepo/main.go b/example/newrepo/main.go index 26d557abd3d..84d315821f7 100644 --- a/example/newrepo/main.go +++ b/example/newrepo/main.go @@ -36,7 +36,7 @@ func main() { log.Fatal("No name: New repos must be given a name") } ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client := github.NewClient(nil).WithAuthToken(token) r := &github.Repository{Name: name, Private: private, Description: description, AutoInit: autoInit} repo, _, err := client.Repositories.Create(ctx, "", r) diff --git a/example/newreposecretwithlibsodium/main.go b/example/newreposecretwithlibsodium/main.go index 1b1a12b2786..c0d412b51ac 100644 --- a/example/newreposecretwithlibsodium/main.go +++ b/example/newreposecretwithlibsodium/main.go @@ -71,10 +71,7 @@ func main() { } ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) - if err != nil { - log.Fatalf("unable to authorize using env GITHUB_AUTH_TOKEN: %v", err) - } + client := github.NewClient(nil).WithAuthToken(token) if err := addRepoSecret(ctx, client, *owner, *repo, secretName, secretValue); err != nil { log.Fatal(err) diff --git a/example/newreposecretwithxcrypto/main.go b/example/newreposecretwithxcrypto/main.go index b9a3f85cb21..0e9a11d9cd9 100644 --- a/example/newreposecretwithxcrypto/main.go +++ b/example/newreposecretwithxcrypto/main.go @@ -72,7 +72,7 @@ func main() { } ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client := github.NewClient(nil).WithAuthToken(token) if err := addRepoSecret(ctx, client, *owner, *repo, secretName, secretValue); err != nil { log.Fatal(err) diff --git a/example/tagprotection/main.go b/example/tagprotection/main.go index d867191ba48..2499b242603 100644 --- a/example/tagprotection/main.go +++ b/example/tagprotection/main.go @@ -44,7 +44,7 @@ func main() { token := string(byteToken) ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client := github.NewClient(nil).WithAuthToken(token) // create new tag protection if pattern != "" { diff --git a/example/tokenauth/main.go b/example/tokenauth/main.go index 64cef754d0c..645bb749aca 100644 --- a/example/tokenauth/main.go +++ b/example/tokenauth/main.go @@ -26,7 +26,7 @@ func main() { token := string(byteToken) ctx := context.Background() - client, _ := github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client := github.NewClient(nil).WithAuthToken(token) user, resp, err := client.Users.Get(ctx, "") if err != nil { diff --git a/github/doc.go b/github/doc.go index 42337fb878b..b4e6163d2b9 100644 --- a/github/doc.go +++ b/github/doc.go @@ -40,34 +40,16 @@ For more sample code snippets, head over to the https://github.com/google/go-git # Authentication -The go-github library does not directly handle authentication. Instead, when -creating a new client, pass an http.Client that can handle authentication for -you. The easiest and recommended way to do this is using the golang.org/x/oauth2 -library, but you can always use any other library that provides an http.Client. -If you have an OAuth2 access token (for example, a personal API token), you can -use it with the oauth2 library using: +Use Client.WithAuthToken to configure your client to authenticate using an Oauth token +(for example, a personal access token). This is what is needed for a majority of use cases +aside from GitHub Apps. - import "golang.org/x/oauth2" - - func main() { - ctx := context.Background() - ts := oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: "... your access token ..."}, - ) - tc := oauth2.NewClient(ctx, ts) - - client := github.NewClient(tc) - - // list all repositories for the authenticated user - repos, _, err := client.Repositories.List(ctx, "", nil) - } + client := github.NewClient(nil).WithAuthToken("... your access token ...") Note that when using an authenticated Client, all calls made by the client will include the specified OAuth token. Therefore, authenticated clients should almost never be shared between different users. -See the oauth2 docs for complete instructions on using that library. - For API methods that require HTTP Basic Authentication, use the BasicAuthTransport. diff --git a/github/github.go b/github/github.go index bf647458086..97ee8ff88eb 100644 --- a/github/github.go +++ b/github/github.go @@ -305,37 +305,37 @@ func addOptions(s string, opts interface{}) (string, error) { // NewClient returns a new GitHub API client. If a nil httpClient is // provided, a new http.Client will be used. To use API methods which require -// authentication, either use NewTokenClient instead or provide NewClient with +// authentication, either use Client.WithAuthToken or provide NewClient with // an http.Client that will perform the authentication for you (such as that // provided by the golang.org/x/oauth2 library). func NewClient(httpClient *http.Client) *Client { - c := &Client{ - client: httpClient, - } - // empty WithOptions always returns a nil error. - _ = c.initialize() + c := &Client{client: httpClient} + c.initialize() return c } -type ClientOption func(*Client) error - -// WithAuthToken configures the client to use the provided token for the Authorization header. -func WithAuthToken(token string) ClientOption { - return func(c *Client) error { - c.client.Transport = roundTripperFunc( - func(req *http.Request) (*http.Response, error) { - req = req.Clone(req.Context()) - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) - return http.DefaultTransport.RoundTrip(req) - }, - ) - return nil - } -} - -// WithEnterpriseURLs configures the client to use the provided base and upload URLs. -// If the base URL does not have the suffix "/api/v3/", it will be added automatically. -// If the upload URL does not have the suffix "/api/uploads", it will be added automatically. +// WithAuthToken returns a copy of the client configured to use the provided token for the Authorization header. +func (c *Client) WithAuthToken(token string) *Client { + c2 := c.copy() + defer c2.initialize() + transport := c2.client.Transport + if transport == nil { + transport = http.DefaultTransport + } + c2.client.Transport = roundTripperFunc( + func(req *http.Request) (*http.Response, error) { + req = req.Clone(req.Context()) + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + return transport.RoundTrip(req) + }, + ) + return c2 +} + +// WithEnterpriseURLs returns a copy of the client configured to use the provided base and +// upload URLs. If the base URL does not have the suffix "/api/v3/", it will be added +// automatically. If the upload URL does not have the suffix "/api/uploads", it will be +// added automatically. // // Note that WithEnterpriseURLs is a convenience helper only; // its behavior is equivalent to setting the BaseURL and UploadURL fields. @@ -343,52 +343,45 @@ func WithAuthToken(token string) ClientOption { // Another important thing is that by default, the GitHub Enterprise URL format // should be http(s)://[hostname]/api/v3/ or you will always receive the 406 status code. // The upload URL format should be http(s)://[hostname]/api/uploads/. -func WithEnterpriseURLs(baseURL, uploadURL string) ClientOption { - return func(c *Client) error { - var err error - c.BaseURL, err = url.Parse(baseURL) - if err != nil { - return err - } +func (c *Client) WithEnterpriseURLs(baseURL, uploadURL string) (*Client, error) { + c2 := c.copy() + defer c2.initialize() + var err error + c2.BaseURL, err = url.Parse(baseURL) + if err != nil { + return nil, err + } - if !strings.HasSuffix(c.BaseURL.Path, "/") { - c.BaseURL.Path += "/" - } - if !strings.HasSuffix(c.BaseURL.Path, "/api/v3/") && - !strings.HasPrefix(c.BaseURL.Host, "api.") && - !strings.Contains(c.BaseURL.Host, ".api.") { - c.BaseURL.Path += "api/v3/" - } + if !strings.HasSuffix(c2.BaseURL.Path, "/") { + c2.BaseURL.Path += "/" + } + if !strings.HasSuffix(c2.BaseURL.Path, "/api/v3/") && + !strings.HasPrefix(c2.BaseURL.Host, "api.") && + !strings.Contains(c2.BaseURL.Host, ".api.") { + c2.BaseURL.Path += "api/v3/" + } - c.UploadURL, err = url.Parse(uploadURL) - if err != nil { - return err - } + c2.UploadURL, err = url.Parse(uploadURL) + if err != nil { + return nil, err + } - if !strings.HasSuffix(c.UploadURL.Path, "/") { - c.UploadURL.Path += "/" - } - if !strings.HasSuffix(c.UploadURL.Path, "/api/uploads/") && - !strings.HasPrefix(c.UploadURL.Host, "api.") && - !strings.Contains(c.UploadURL.Host, ".api.") { - c.UploadURL.Path += "api/uploads/" - } - return nil + if !strings.HasSuffix(c2.UploadURL.Path, "/") { + c2.UploadURL.Path += "/" + } + if !strings.HasSuffix(c2.UploadURL.Path, "/api/uploads/") && + !strings.HasPrefix(c2.UploadURL.Host, "api.") && + !strings.Contains(c2.UploadURL.Host, ".api.") { + c2.UploadURL.Path += "api/uploads/" } + return c2, nil } -// initialize applies the given options to the client, sets defaults and -// initializes services. -func (c *Client) initialize(opts ...ClientOption) error { +// initialize sets default values and initializes services. +func (c *Client) initialize() { if c.client == nil { c.client = &http.Client{} } - for _, opt := range opts { - err := opt(c) - if err != nil { - return err - } - } if c.BaseURL == nil { c.BaseURL, _ = url.Parse(defaultBaseURL) } @@ -431,13 +424,13 @@ func (c *Client) initialize(opts ...ClientOption) error { c.SecurityAdvisories = (*SecurityAdvisoriesService)(&c.common) c.Teams = (*TeamsService)(&c.common) c.Users = (*UsersService)(&c.common) - return nil } -// WithOptions returns a clone of the client with the specified options. -func (c *Client) WithOptions(opts ...ClientOption) (*Client, error) { +// copy returns a copy of the current client. It must be initialized before use. +func (c *Client) copy() *Client { c.clientMu.Lock() - clone := &Client{ + // can't use *c here because that would copy mutexes by value. + clone := Client{ client: c.client, UserAgent: c.UserAgent, BaseURL: c.BaseURL, @@ -445,14 +438,13 @@ func (c *Client) WithOptions(opts ...ClientOption) (*Client, error) { secondaryRateLimitReset: c.secondaryRateLimitReset, } c.clientMu.Unlock() + if clone.client == nil { + clone.client = &http.Client{} + } c.rateMu.Lock() copy(clone.rateLimits[:], c.rateLimits[:]) c.rateMu.Unlock() - err := clone.initialize(opts...) - if err != nil { - return nil, err - } - return clone, nil + return &clone } // NewClientWithEnvProxy enhances NewClient with the HttpProxy env. @@ -462,16 +454,15 @@ func NewClientWithEnvProxy() *Client { } // NewTokenClient returns a new GitHub API client authenticated with the provided token. -// Deprecated: Use NewClient(nil).WithOptions(WithAuthToken(token)) instead. +// Deprecated: Use NewClient(nil).WithAuthToken(token) instead. func NewTokenClient(_ context.Context, token string) *Client { // This always returns a nil error. - c, _ := NewClient(nil).WithOptions(WithAuthToken(token)) - return c + return NewClient(nil).WithAuthToken(token) } // Deprecated: Use NewClient(httpClient).WithOptions(WithEnterpriseURLs(baseURL, uploadURL)) instead. func NewEnterpriseClient(baseURL, uploadURL string, httpClient *http.Client) (*Client, error) { - return NewClient(httpClient).WithOptions(WithEnterpriseURLs(baseURL, uploadURL)) + return NewClient(httpClient).WithEnterpriseURLs(baseURL, uploadURL) } // RequestOption represents an option that can modify an http.Request. diff --git a/github/github_test.go b/github/github_test.go index 38bbf35d0ce..906c6fe7d68 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -416,7 +416,7 @@ func TestWithEnterpriseURLs(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - c, err := NewClient(nil).WithOptions(WithEnterpriseURLs(test.baseURL, test.uploadURL)) + c, err := NewClient(nil).WithEnterpriseURLs(test.baseURL, test.uploadURL) if test.wantErr != "" { if err == nil || !strings.Contains(err.Error(), test.wantErr) { t.Fatalf("error does not contain expected string %q: %v", test.wantErr, err) diff --git a/test/fields/fields.go b/test/fields/fields.go index 136a9307c0d..6b44a04310d 100644 --- a/test/fields/fields.go +++ b/test/fields/fields.go @@ -46,7 +46,7 @@ func main() { print("!!! No OAuth token. Some tests won't run. !!!\n\n") client = github.NewClient(nil) } else { - client, _ = github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client = github.NewClient(nil).WithAuthToken(token) auth = true } diff --git a/test/integration/github_test.go b/test/integration/github_test.go index ead4e1aca2d..5bef6ca73aa 100644 --- a/test/integration/github_test.go +++ b/test/integration/github_test.go @@ -32,7 +32,7 @@ func init() { print("!!! No OAuth token. Some tests won't run. !!!\n\n") client = github.NewClient(nil) } else { - client, _ = github.NewClient(nil).WithOptions(github.WithAuthToken(token)) + client = github.NewClient(nil).WithAuthToken(token) auth = true } } From 8178d23d9304ace268ee877d968a8c6907e85e03 Mon Sep 17 00:00:00 2001 From: Will Roden Date: Sun, 27 Aug 2023 16:44:53 -0500 Subject: [PATCH 4/6] leave NewClientWithEnvProxy untouched --- github/github.go | 1 - 1 file changed, 1 deletion(-) diff --git a/github/github.go b/github/github.go index 97ee8ff88eb..35eadf5a648 100644 --- a/github/github.go +++ b/github/github.go @@ -448,7 +448,6 @@ func (c *Client) copy() *Client { } // NewClientWithEnvProxy enhances NewClient with the HttpProxy env. -// Deprecated: Use NewClient(nil) instead. func NewClientWithEnvProxy() *Client { return NewClient(&http.Client{Transport: &http.Transport{Proxy: http.ProxyFromEnvironment}}) } From ea7caf8868946a6cab14847ba3bd58e19217aa5e Mon Sep 17 00:00:00 2001 From: WillAbides <233500+WillAbides@users.noreply.github.com> Date: Tue, 29 Aug 2023 09:02:44 -0500 Subject: [PATCH 5/6] Update github/github.go Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> --- github/github.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/github/github.go b/github/github.go index 35eadf5a648..dbd856b0e2d 100644 --- a/github/github.go +++ b/github/github.go @@ -459,6 +459,9 @@ func NewTokenClient(_ context.Context, token string) *Client { return NewClient(nil).WithAuthToken(token) } +// NewEnterpriseClient returns a new GitHub API client with provided +// base URL and upload URL (often is your GitHub Enterprise hostname). +// // Deprecated: Use NewClient(httpClient).WithOptions(WithEnterpriseURLs(baseURL, uploadURL)) instead. func NewEnterpriseClient(baseURL, uploadURL string, httpClient *http.Client) (*Client, error) { return NewClient(httpClient).WithEnterpriseURLs(baseURL, uploadURL) From 8a74e055e38a5b3e2e16967faac6134ffe01d075 Mon Sep 17 00:00:00 2001 From: Will Roden Date: Tue, 29 Aug 2023 09:17:12 -0500 Subject: [PATCH 6/6] increase test coverage --- github/github_test.go | 56 ++++++++++++++++++++++++++----------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/github/github_test.go b/github/github_test.go index 906c6fe7d68..af506e65243 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -306,21 +306,28 @@ func TestClient(t *testing.T) { } } -func TestNewTokenClient(t *testing.T) { +func TestWithAuthToken(t *testing.T) { token := "gh_test_token" - ctx := context.Background() var gotAuthHeaderVals []string wantAuthHeaderVals := []string{"Bearer " + token} srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotAuthHeaderVals = r.Header["Authorization"] })) - _, err := NewTokenClient(ctx, token).Client().Get(srv.URL) - if err != nil { - t.Fatalf("Get returned unexpected error: %v", err) - } - if diff := cmp.Diff(wantAuthHeaderVals, gotAuthHeaderVals); diff != "" { - t.Errorf("Authorization header values mismatch (-want +got):\n%s", diff) + validate := func(c *Client) { + t.Helper() + gotAuthHeaderVals = nil + _, err := c.Client().Get(srv.URL) + if err != nil { + t.Fatalf("Get returned unexpected error: %v", err) + } + diff := cmp.Diff(wantAuthHeaderVals, gotAuthHeaderVals) + if diff != "" { + t.Errorf("Authorization header values mismatch (-want +got):\n%s", diff) + } } + validate(NewClient(nil).WithAuthToken(token)) + validate(new(Client).WithAuthToken(token)) + validate(NewTokenClient(context.Background(), token)) } func TestWithEnterpriseURLs(t *testing.T) { @@ -416,22 +423,27 @@ func TestWithEnterpriseURLs(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - c, err := NewClient(nil).WithEnterpriseURLs(test.baseURL, test.uploadURL) - if test.wantErr != "" { - if err == nil || !strings.Contains(err.Error(), test.wantErr) { - t.Fatalf("error does not contain expected string %q: %v", test.wantErr, err) + validate := func(c *Client, err error) { + t.Helper() + if test.wantErr != "" { + if err == nil || !strings.Contains(err.Error(), test.wantErr) { + t.Fatalf("error does not contain expected string %q: %v", test.wantErr, err) + } + return + } + if err != nil { + t.Fatalf("got unexpected error: %v", err) + } + if c.BaseURL.String() != test.wantBaseURL { + t.Errorf("BaseURL is %v, want %v", c.BaseURL.String(), test.wantBaseURL) + } + if c.UploadURL.String() != test.wantUploadURL { + t.Errorf("UploadURL is %v, want %v", c.UploadURL.String(), test.wantUploadURL) } - return - } - if err != nil { - t.Fatalf("got unexpected error: %v", err) - } - if c.BaseURL.String() != test.wantBaseURL { - t.Errorf("BaseURL is %v, want %v", c.BaseURL.String(), test.wantBaseURL) - } - if c.UploadURL.String() != test.wantUploadURL { - t.Errorf("UploadURL is %v, want %v", c.UploadURL.String(), test.wantUploadURL) } + validate(NewClient(nil).WithEnterpriseURLs(test.baseURL, test.uploadURL)) + validate(new(Client).WithEnterpriseURLs(test.baseURL, test.uploadURL)) + validate(NewEnterpriseClient(test.baseURL, test.uploadURL, nil)) }) } }