Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion github/code-scanning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func TestCodeScanningService_UploadSarif(t *testing.T) {
return err
})

testNewRequestAndDoFailureCategory(t, methodName, client, codeScanningUploadCategory, func() (*Response, error) {
testNewRequestAndDoFailureCategory(t, methodName, client, CodeScanningUploadCategory, func() (*Response, error) {
_, resp, err := client.CodeScanning.UploadSarif(ctx, "o", "r", sarifAnalysis)
return resp, err
})
Expand Down
2 changes: 1 addition & 1 deletion github/enterprise_audit_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestEnterpriseService_GetAuditLog(t *testing.T) {
return err
})

testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
testNewRequestAndDoFailureCategory(t, methodName, client, AuditLogCategory, func() (*Response, error) {
got, resp, err := client.Enterprise.GetAuditLog(ctx, "o", &GetAuditLogOptions{})
if got != nil {
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
Expand Down
8 changes: 8 additions & 0 deletions github/github-accessors.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions github/github-accessors_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 32 additions & 27 deletions github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ type Client struct {
UserAgent string

rateMu sync.Mutex
rateLimits [categories]Rate // Rate limits for the client as determined by the most recent API calls.
rateLimits [Categories]Rate // Rate limits for the client as determined by the most recent API calls.
secondaryRateLimitReset time.Time // Secondary rate limit reset for the client as determined by the most recent API calls.

common service // Reuse a single struct instead of allocating one for each service on the heap.
Expand Down Expand Up @@ -821,7 +821,7 @@ func (c *Client) BareDo(ctx context.Context, req *http.Request) (*Response, erro

req = withContext(ctx, req)

rateLimitCategory := category(req.Method, req.URL.Path)
rateLimitCategory := GetRateLimitCategory(req.Method, req.URL.Path)

if bypass := ctx.Value(bypassRateLimitCheck); bypass == nil {
// If we've hit rate limit, don't make further requests before Reset time.
Expand Down Expand Up @@ -937,7 +937,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request, v interface{}) (*Res
// current client state in order to quickly check if *RateLimitError can be immediately returned
// from Client.Do, and if so, returns it so that Client.Do can skip making a network API call unnecessarily.
// Otherwise it returns nil, and Client.Do should proceed normally.
func (c *Client) checkRateLimitBeforeDo(req *http.Request, rateLimitCategory rateLimitCategory) *RateLimitError {
func (c *Client) checkRateLimitBeforeDo(req *http.Request, rateLimitCategory RateLimitCategory) *RateLimitError {
c.rateMu.Lock()
rate := c.rateLimits[rateLimitCategory]
c.rateMu.Unlock()
Expand Down Expand Up @@ -1303,65 +1303,70 @@ func parseBoolResponse(err error) (bool, error) {
return false, err
}

type rateLimitCategory uint8
type RateLimitCategory uint8

const (
coreCategory rateLimitCategory = iota
searchCategory
graphqlCategory
integrationManifestCategory
sourceImportCategory
codeScanningUploadCategory
actionsRunnerRegistrationCategory
scimCategory
dependencySnapshotsCategory
codeSearchCategory

categories // An array of this length will be able to contain all rate limit categories.
CoreCategory RateLimitCategory = iota
SearchCategory
GraphqlCategory
IntegrationManifestCategory
SourceImportCategory
CodeScanningUploadCategory
ActionsRunnerRegistrationCategory
ScimCategory
DependencySnapshotsCategory
CodeSearchCategory
AuditLogCategory

Categories // An array of this length will be able to contain all rate limit categories.
)

// category returns the rate limit category of the endpoint, determined by HTTP method and Request.URL.Path.
func category(method, path string) rateLimitCategory {
// GetRateLimitCategory returns the rate limit RateLimitCategory of the endpoint, determined by HTTP method and Request.URL.Path.
func GetRateLimitCategory(method, path string) RateLimitCategory {
switch {
// https://docs.github.com/rest/rate-limit#about-rate-limits
default:
// NOTE: coreCategory is returned for actionsRunnerRegistrationCategory too,
// because no API found for this category.
return coreCategory
return CoreCategory

// https://docs.github.com/en/rest/search/search#search-code
case strings.HasPrefix(path, "/search/code") &&
method == http.MethodGet:
return codeSearchCategory
return CodeSearchCategory

case strings.HasPrefix(path, "/search/"):
return searchCategory
return SearchCategory
case path == "/graphql":
return graphqlCategory
return GraphqlCategory
case strings.HasPrefix(path, "/app-manifests/") &&
strings.HasSuffix(path, "/conversions") &&
method == http.MethodPost:
return integrationManifestCategory
return IntegrationManifestCategory

// https://docs.github.com/rest/migrations/source-imports#start-an-import
case strings.HasPrefix(path, "/repos/") &&
strings.HasSuffix(path, "/import") &&
method == http.MethodPut:
return sourceImportCategory
return SourceImportCategory

// https://docs.github.com/rest/code-scanning#upload-an-analysis-as-sarif-data
case strings.HasSuffix(path, "/code-scanning/sarifs"):
return codeScanningUploadCategory
return CodeScanningUploadCategory

// https://docs.github.com/enterprise-cloud@latest/rest/scim
case strings.HasPrefix(path, "/scim/"):
return scimCategory
return ScimCategory

// https://docs.github.com/en/rest/dependency-graph/dependency-submission#create-a-snapshot-of-dependencies-for-a-repository
case strings.HasPrefix(path, "/repos/") &&
strings.HasSuffix(path, "/dependency-graph/snapshots") &&
method == http.MethodPost:
return dependencySnapshotsCategory
return DependencySnapshotsCategory

// https://docs.github.com/en/enterprise-cloud@latest/rest/orgs/orgs?apiVersion=2022-11-28#get-the-audit-log-for-an-organization
case strings.HasSuffix(path, "/audit-log"):
return AuditLogCategory
}
}

Expand Down
35 changes: 20 additions & 15 deletions github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,11 @@ func testBadOptions(t *testing.T, methodName string, f func() error) {
// Method f should be a regular call that would normally succeed, but
// should return an error when NewRequest or s.client.Do fails.
func testNewRequestAndDoFailure(t *testing.T, methodName string, client *Client, f func() (*Response, error)) {
testNewRequestAndDoFailureCategory(t, methodName, client, coreCategory, f)
testNewRequestAndDoFailureCategory(t, methodName, client, CoreCategory, f)
}

// testNewRequestAndDoFailureCategory works Like testNewRequestAndDoFailure, but allows setting the category
func testNewRequestAndDoFailureCategory(t *testing.T, methodName string, client *Client, category rateLimitCategory, f func() (*Response, error)) {
func testNewRequestAndDoFailureCategory(t *testing.T, methodName string, client *Client, category RateLimitCategory, f func() (*Response, error)) {
t.Helper()
if methodName == "" {
t.Error("testNewRequestAndDoFailure: must supply method methodName")
Expand Down Expand Up @@ -1132,68 +1132,73 @@ func TestDo_rateLimitCategory(t *testing.T) {
tests := []struct {
method string
url string
category rateLimitCategory
category RateLimitCategory
}{
{
method: http.MethodGet,
url: "/",
category: coreCategory,
category: CoreCategory,
},
{
method: http.MethodGet,
url: "/search/issues?q=rate",
category: searchCategory,
category: SearchCategory,
},
{
method: http.MethodGet,
url: "/graphql",
category: graphqlCategory,
category: GraphqlCategory,
},
{
method: http.MethodPost,
url: "/app-manifests/code/conversions",
category: integrationManifestCategory,
category: IntegrationManifestCategory,
},
{
method: http.MethodGet,
url: "/app-manifests/code/conversions",
category: coreCategory, // only POST requests are in the integration manifest category
category: CoreCategory, // only POST requests are in the integration manifest category
},
{
method: http.MethodPut,
url: "/repos/google/go-github/import",
category: sourceImportCategory,
category: SourceImportCategory,
},
{
method: http.MethodGet,
url: "/repos/google/go-github/import",
category: coreCategory, // only PUT requests are in the source import category
category: CoreCategory, // only PUT requests are in the source import category
},
{
method: http.MethodPost,
url: "/repos/google/go-github/code-scanning/sarifs",
category: codeScanningUploadCategory,
category: CodeScanningUploadCategory,
},
{
method: http.MethodGet,
url: "/scim/v2/organizations/ORG/Users",
category: scimCategory,
category: ScimCategory,
},
{
method: http.MethodPost,
url: "/repos/google/go-github/dependency-graph/snapshots",
category: dependencySnapshotsCategory,
category: DependencySnapshotsCategory,
},
{
method: http.MethodGet,
url: "/search/code?q=rate",
category: codeSearchCategory,
category: CodeSearchCategory,
},
{
method: http.MethodGet,
url: "/orgs/google/audit-log",
category: AuditLogCategory,
},
// missing a check for actionsRunnerRegistrationCategory: API not found
}

for _, tt := range tests {
if got, want := category(tt.method, tt.url), tt.category; got != want {
if got, want := GetRateLimitCategory(tt.method, tt.url), tt.category; got != want {
t.Errorf("expecting category %v, found %v", got, want)
}
}
Expand Down
2 changes: 1 addition & 1 deletion github/orgs_audit_log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func TestOrganizationService_GetAuditLog(t *testing.T) {
return err
})

testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
testNewRequestAndDoFailureCategory(t, methodName, client, AuditLogCategory, func() (*Response, error) {
got, resp, err := client.Organizations.GetAuditLog(ctx, "o", &GetAuditLogOptions{})
if got != nil {
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
Expand Down
24 changes: 14 additions & 10 deletions github/rate_limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ type RateLimits struct {
SCIM *Rate `json:"scim"`
DependencySnapshots *Rate `json:"dependency_snapshots"`
CodeSearch *Rate `json:"code_search"`
AuditLog *Rate `json:"audit_log"`
}

func (r RateLimits) String() string {
Expand Down Expand Up @@ -85,34 +86,37 @@ func (s *RateLimitService) Get(ctx context.Context) (*RateLimits, *Response, err
if response.Resources != nil {
s.client.rateMu.Lock()
if response.Resources.Core != nil {
s.client.rateLimits[coreCategory] = *response.Resources.Core
s.client.rateLimits[CoreCategory] = *response.Resources.Core
}
if response.Resources.Search != nil {
s.client.rateLimits[searchCategory] = *response.Resources.Search
s.client.rateLimits[SearchCategory] = *response.Resources.Search
}
if response.Resources.GraphQL != nil {
s.client.rateLimits[graphqlCategory] = *response.Resources.GraphQL
s.client.rateLimits[GraphqlCategory] = *response.Resources.GraphQL
}
if response.Resources.IntegrationManifest != nil {
s.client.rateLimits[integrationManifestCategory] = *response.Resources.IntegrationManifest
s.client.rateLimits[IntegrationManifestCategory] = *response.Resources.IntegrationManifest
}
if response.Resources.SourceImport != nil {
s.client.rateLimits[sourceImportCategory] = *response.Resources.SourceImport
s.client.rateLimits[SourceImportCategory] = *response.Resources.SourceImport
}
if response.Resources.CodeScanningUpload != nil {
s.client.rateLimits[codeScanningUploadCategory] = *response.Resources.CodeScanningUpload
s.client.rateLimits[CodeScanningUploadCategory] = *response.Resources.CodeScanningUpload
}
if response.Resources.ActionsRunnerRegistration != nil {
s.client.rateLimits[actionsRunnerRegistrationCategory] = *response.Resources.ActionsRunnerRegistration
s.client.rateLimits[ActionsRunnerRegistrationCategory] = *response.Resources.ActionsRunnerRegistration
}
if response.Resources.SCIM != nil {
s.client.rateLimits[scimCategory] = *response.Resources.SCIM
s.client.rateLimits[ScimCategory] = *response.Resources.SCIM
}
if response.Resources.DependencySnapshots != nil {
s.client.rateLimits[dependencySnapshotsCategory] = *response.Resources.DependencySnapshots
s.client.rateLimits[DependencySnapshotsCategory] = *response.Resources.DependencySnapshots
}
if response.Resources.CodeSearch != nil {
s.client.rateLimits[codeSearchCategory] = *response.Resources.CodeSearch
s.client.rateLimits[CodeSearchCategory] = *response.Resources.CodeSearch
}
if response.Resources.AuditLog != nil {
s.client.rateLimits[AuditLogCategory] = *response.Resources.AuditLog
}
s.client.rateMu.Unlock()
}
Expand Down
Loading