Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support new GitHub v3 API calendar-based versioning #2581

Merged
merged 3 commits into from Dec 9, 2022
Merged

Support new GitHub v3 API calendar-based versioning #2581

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3cd13ca
Support new GitHub v3 API versioning
gmlewis Nov 29, 2022
e76acfa
Improve code coverage
gmlewis Nov 29, 2022
eb1e95a
Address review feedback
gmlewis Nov 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 42 additions & 10 deletions github/github.go
View file
Open in desktop
Expand Up @@ -27,12 +27,14 @@ import (
)

const (
Version = "v48.0.0"
Version = "v48.2.0"

defaultBaseURL = "https://api.github.com/"
defaultUserAgent = "go-github" + "/" + Version
uploadBaseURL = "https://uploads.github.com/"
defaultAPIVersion = "2022-11-28"
defaultBaseURL = "https://api.github.com/"
defaultUserAgent = "go-github" + "/" + Version
uploadBaseURL = "https://uploads.github.com/"

headerAPIVersion = "X-GitHub-Api-Version"
headerRateLimit = "X-RateLimit-Limit"
headerRateRemaining = "X-RateLimit-Remaining"
headerRateReset = "X-RateLimit-Reset"
Expand Down Expand Up @@ -392,12 +394,24 @@ func NewEnterpriseClient(baseURL, uploadURL string, httpClient *http.Client) (*C
return c, nil
}

// RequestOption represents an option that can modify an http.Request.
type RequestOption func(req *http.Request)

// WithVersion modifies the GitHub v3 API version for this individual request.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe change "modifies" to "overrides" or "replaces", just to emphasize that this completely replaces the previous version?

I could imagine other RequestOptions that add a new header without replacing existing ones (for example, if we were to have done something like this for adding the preview Accept headers)

// For more information, see:
// https://github.blog/2022-11-28-to-infinity-and-beyond-enabling-the-future-of-githubs-rest-api-with-api-versioning/
func WithVersion(version string) RequestOption {
return func(req *http.Request) {
req.Header.Set(headerAPIVersion, version)
}
}

// NewRequest creates an API request. A relative URL can be provided in urlStr,
// in which case it is resolved relative to the BaseURL of the Client.
// Relative URLs should always be specified without a preceding slash. If
// specified, the value pointed to by body is JSON encoded and included as the
// request body.
func (c *Client) NewRequest(method, urlStr string, body interface{}) (*http.Request, error) {
func (c *Client) NewRequest(method, urlStr string, body interface{}, opts ...RequestOption) (*http.Request, error) {
if !strings.HasSuffix(c.BaseURL.Path, "/") {
return nil, fmt.Errorf("BaseURL must have a trailing slash, but %q does not", c.BaseURL)
}
Expand Down Expand Up @@ -430,14 +444,20 @@ func (c *Client) NewRequest(method, urlStr string, body interface{}) (*http.Requ
if c.UserAgent != "" {
req.Header.Set("User-Agent", c.UserAgent)
}
req.Header.Set(headerAPIVersion, defaultAPIVersion)

for _, opt := range opts {
opt(req)
}

return req, nil
}

// NewFormRequest creates an API request. A relative URL can be provided in urlStr,
// in which case it is resolved relative to the BaseURL of the Client.
// Relative URLs should always be specified without a preceding slash.
// Body is sent with Content-Type: application/x-www-form-urlencoded.
func (c *Client) NewFormRequest(urlStr string, body io.Reader) (*http.Request, error) {
func (c *Client) NewFormRequest(urlStr string, body io.Reader, opts ...RequestOption) (*http.Request, error) {
if !strings.HasSuffix(c.BaseURL.Path, "/") {
return nil, fmt.Errorf("BaseURL must have a trailing slash, but %q does not", c.BaseURL)
}
Expand All @@ -457,13 +477,19 @@ func (c *Client) NewFormRequest(urlStr string, body io.Reader) (*http.Request, e
if c.UserAgent != "" {
req.Header.Set("User-Agent", c.UserAgent)
}
req.Header.Set(headerAPIVersion, defaultAPIVersion)

for _, opt := range opts {
opt(req)
}

return req, nil
}

// NewUploadRequest creates an upload request. A relative URL can be provided in
// urlStr, in which case it is resolved relative to the UploadURL of the Client.
// Relative URLs should always be specified without a preceding slash.
func (c *Client) NewUploadRequest(urlStr string, reader io.Reader, size int64, mediaType string) (*http.Request, error) {
func (c *Client) NewUploadRequest(urlStr string, reader io.Reader, size int64, mediaType string, opts ...RequestOption) (*http.Request, error) {
if !strings.HasSuffix(c.UploadURL.Path, "/") {
return nil, fmt.Errorf("UploadURL must have a trailing slash, but %q does not", c.UploadURL)
}
Expand All @@ -485,6 +511,12 @@ func (c *Client) NewUploadRequest(urlStr string, reader io.Reader, size int64, m
req.Header.Set("Content-Type", mediaType)
req.Header.Set("Accept", mediaTypeV3)
req.Header.Set("User-Agent", c.UserAgent)
req.Header.Set(headerAPIVersion, defaultAPIVersion)

for _, opt := range opts {
opt(req)
}

return req, nil
}

Expand Down Expand Up @@ -1358,8 +1390,8 @@ func formatRateReset(d time.Duration) string {

// When using roundTripWithOptionalFollowRedirect, note that it
// is the responsibility of the caller to close the response body.
func (c *Client) roundTripWithOptionalFollowRedirect(ctx context.Context, u string, followRedirects bool) (*http.Response, error) {
req, err := c.NewRequest("GET", u, nil)
func (c *Client) roundTripWithOptionalFollowRedirect(ctx context.Context, u string, followRedirects bool, opts ...RequestOption) (*http.Response, error) {
req, err := c.NewRequest("GET", u, nil, opts...)
if err != nil {
return nil, err
}
Expand All @@ -1380,7 +1412,7 @@ func (c *Client) roundTripWithOptionalFollowRedirect(ctx context.Context, u stri
if followRedirects && resp.StatusCode == http.StatusMovedPermanently {
resp.Body.Close()
u = resp.Header.Get("Location")
resp, err = c.roundTripWithOptionalFollowRedirect(ctx, u, false)
resp, err = c.roundTripWithOptionalFollowRedirect(ctx, u, false, opts...)
}
return resp, err
}
Expand Down
38 changes: 38 additions & 0 deletions github/github_test.go
View file
Open in desktop
Expand Up @@ -517,6 +517,17 @@ func TestNewRequest(t *testing.T) {
if !strings.Contains(userAgent, Version) {
t.Errorf("NewRequest() User-Agent should contain %v, found %v", Version, userAgent)
}

apiVersion := req.Header.Get(headerAPIVersion)
if got, want := apiVersion, defaultAPIVersion; got != want {
t.Errorf("NewRequest() %v header is %v, want %v", headerAPIVersion, got, want)
}

req, _ = c.NewRequest("GET", inURL, inBody, WithVersion("2022-11-29"))
apiVersion = req.Header.Get(headerAPIVersion)
if got, want := apiVersion, "2022-11-29"; got != want {
t.Errorf("NewRequest() %v header is %v, want %v", headerAPIVersion, got, want)
}
}

func TestNewRequest_invalidJSON(t *testing.T) {
Expand Down Expand Up @@ -626,6 +637,17 @@ func TestNewFormRequest(t *testing.T) {
if got, want := req.Header.Get("User-Agent"), c.UserAgent; got != want {
t.Errorf("NewFormRequest() User-Agent is %v, want %v", got, want)
}

apiVersion := req.Header.Get(headerAPIVersion)
if got, want := apiVersion, defaultAPIVersion; got != want {
t.Errorf("NewRequest() %v header is %v, want %v", headerAPIVersion, got, want)
}

req, _ = c.NewFormRequest(inURL, inBody, WithVersion("2022-11-29"))
apiVersion = req.Header.Get(headerAPIVersion)
if got, want := apiVersion, "2022-11-29"; got != want {
t.Errorf("NewRequest() %v header is %v, want %v", headerAPIVersion, got, want)
}
}

func TestNewFormRequest_badURL(t *testing.T) {
Expand Down Expand Up @@ -680,6 +702,22 @@ func TestNewFormRequest_errorForNoTrailingSlash(t *testing.T) {
}
}

func TestNewUploadRequest_WithVersion(t *testing.T) {
c := NewClient(nil)
req, _ := c.NewUploadRequest("https://example.com/", nil, 0, "")

apiVersion := req.Header.Get(headerAPIVersion)
if got, want := apiVersion, defaultAPIVersion; got != want {
t.Errorf("NewRequest() %v header is %v, want %v", headerAPIVersion, got, want)
}

req, _ = c.NewUploadRequest("https://example.com/", nil, 0, "", WithVersion("2022-11-29"))
apiVersion = req.Header.Get(headerAPIVersion)
if got, want := apiVersion, "2022-11-29"; got != want {
t.Errorf("NewRequest() %v header is %v, want %v", headerAPIVersion, got, want)
}
}

func TestNewUploadRequest_badURL(t *testing.T) {
c := NewClient(nil)
_, err := c.NewUploadRequest(":", nil, 0, "")
Expand Down