Skip to content

Commit dbf91ee

Browse files
authored
Add audit log rate limit category and make rate limit category getter public (#3088)
1 parent 169ad4d commit dbf91ee

File tree

9 files changed

+141
-81
lines changed

9 files changed

+141
-81
lines changed

github/code-scanning_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func TestCodeScanningService_UploadSarif(t *testing.T) {
9393
return err
9494
})
9595

96-
testNewRequestAndDoFailureCategory(t, methodName, client, codeScanningUploadCategory, func() (*Response, error) {
96+
testNewRequestAndDoFailureCategory(t, methodName, client, CodeScanningUploadCategory, func() (*Response, error) {
9797
_, resp, err := client.CodeScanning.UploadSarif(ctx, "o", "r", sarifAnalysis)
9898
return resp, err
9999
})

github/enterprise_audit_log_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestEnterpriseService_GetAuditLog(t *testing.T) {
8484
return err
8585
})
8686

87-
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
87+
testNewRequestAndDoFailureCategory(t, methodName, client, AuditLogCategory, func() (*Response, error) {
8888
got, resp, err := client.Enterprise.GetAuditLog(ctx, "o", &GetAuditLogOptions{})
8989
if got != nil {
9090
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)

github/github-accessors.go

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

github/github-accessors_test.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

github/github.go

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ type Client struct {
170170
UserAgent string
171171

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

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

822822
req = withContext(ctx, req)
823823

824-
rateLimitCategory := category(req.Method, req.URL.Path)
824+
rateLimitCategory := GetRateLimitCategory(req.Method, req.URL.Path)
825825

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

1306-
type rateLimitCategory uint8
1306+
type RateLimitCategory uint8
13071307

13081308
const (
1309-
coreCategory rateLimitCategory = iota
1310-
searchCategory
1311-
graphqlCategory
1312-
integrationManifestCategory
1313-
sourceImportCategory
1314-
codeScanningUploadCategory
1315-
actionsRunnerRegistrationCategory
1316-
scimCategory
1317-
dependencySnapshotsCategory
1318-
codeSearchCategory
1319-
1320-
categories // An array of this length will be able to contain all rate limit categories.
1309+
CoreCategory RateLimitCategory = iota
1310+
SearchCategory
1311+
GraphqlCategory
1312+
IntegrationManifestCategory
1313+
SourceImportCategory
1314+
CodeScanningUploadCategory
1315+
ActionsRunnerRegistrationCategory
1316+
ScimCategory
1317+
DependencySnapshotsCategory
1318+
CodeSearchCategory
1319+
AuditLogCategory
1320+
1321+
Categories // An array of this length will be able to contain all rate limit categories.
13211322
)
13221323

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

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

13371338
case strings.HasPrefix(path, "/search/"):
1338-
return searchCategory
1339+
return SearchCategory
13391340
case path == "/graphql":
1340-
return graphqlCategory
1341+
return GraphqlCategory
13411342
case strings.HasPrefix(path, "/app-manifests/") &&
13421343
strings.HasSuffix(path, "/conversions") &&
13431344
method == http.MethodPost:
1344-
return integrationManifestCategory
1345+
return IntegrationManifestCategory
13451346

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

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

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

13601361
// https://docs.github.com/en/rest/dependency-graph/dependency-submission#create-a-snapshot-of-dependencies-for-a-repository
13611362
case strings.HasPrefix(path, "/repos/") &&
13621363
strings.HasSuffix(path, "/dependency-graph/snapshots") &&
13631364
method == http.MethodPost:
1364-
return dependencySnapshotsCategory
1365+
return DependencySnapshotsCategory
1366+
1367+
// https://docs.github.com/en/enterprise-cloud@latest/rest/orgs/orgs?apiVersion=2022-11-28#get-the-audit-log-for-an-organization
1368+
case strings.HasSuffix(path, "/audit-log"):
1369+
return AuditLogCategory
13651370
}
13661371
}
13671372

github/github_test.go

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -212,11 +212,11 @@ func testBadOptions(t *testing.T, methodName string, f func() error) {
212212
// Method f should be a regular call that would normally succeed, but
213213
// should return an error when NewRequest or s.client.Do fails.
214214
func testNewRequestAndDoFailure(t *testing.T, methodName string, client *Client, f func() (*Response, error)) {
215-
testNewRequestAndDoFailureCategory(t, methodName, client, coreCategory, f)
215+
testNewRequestAndDoFailureCategory(t, methodName, client, CoreCategory, f)
216216
}
217217

218218
// testNewRequestAndDoFailureCategory works Like testNewRequestAndDoFailure, but allows setting the category
219-
func testNewRequestAndDoFailureCategory(t *testing.T, methodName string, client *Client, category rateLimitCategory, f func() (*Response, error)) {
219+
func testNewRequestAndDoFailureCategory(t *testing.T, methodName string, client *Client, category RateLimitCategory, f func() (*Response, error)) {
220220
t.Helper()
221221
if methodName == "" {
222222
t.Error("testNewRequestAndDoFailure: must supply method methodName")
@@ -1132,68 +1132,73 @@ func TestDo_rateLimitCategory(t *testing.T) {
11321132
tests := []struct {
11331133
method string
11341134
url string
1135-
category rateLimitCategory
1135+
category RateLimitCategory
11361136
}{
11371137
{
11381138
method: http.MethodGet,
11391139
url: "/",
1140-
category: coreCategory,
1140+
category: CoreCategory,
11411141
},
11421142
{
11431143
method: http.MethodGet,
11441144
url: "/search/issues?q=rate",
1145-
category: searchCategory,
1145+
category: SearchCategory,
11461146
},
11471147
{
11481148
method: http.MethodGet,
11491149
url: "/graphql",
1150-
category: graphqlCategory,
1150+
category: GraphqlCategory,
11511151
},
11521152
{
11531153
method: http.MethodPost,
11541154
url: "/app-manifests/code/conversions",
1155-
category: integrationManifestCategory,
1155+
category: IntegrationManifestCategory,
11561156
},
11571157
{
11581158
method: http.MethodGet,
11591159
url: "/app-manifests/code/conversions",
1160-
category: coreCategory, // only POST requests are in the integration manifest category
1160+
category: CoreCategory, // only POST requests are in the integration manifest category
11611161
},
11621162
{
11631163
method: http.MethodPut,
11641164
url: "/repos/google/go-github/import",
1165-
category: sourceImportCategory,
1165+
category: SourceImportCategory,
11661166
},
11671167
{
11681168
method: http.MethodGet,
11691169
url: "/repos/google/go-github/import",
1170-
category: coreCategory, // only PUT requests are in the source import category
1170+
category: CoreCategory, // only PUT requests are in the source import category
11711171
},
11721172
{
11731173
method: http.MethodPost,
11741174
url: "/repos/google/go-github/code-scanning/sarifs",
1175-
category: codeScanningUploadCategory,
1175+
category: CodeScanningUploadCategory,
11761176
},
11771177
{
11781178
method: http.MethodGet,
11791179
url: "/scim/v2/organizations/ORG/Users",
1180-
category: scimCategory,
1180+
category: ScimCategory,
11811181
},
11821182
{
11831183
method: http.MethodPost,
11841184
url: "/repos/google/go-github/dependency-graph/snapshots",
1185-
category: dependencySnapshotsCategory,
1185+
category: DependencySnapshotsCategory,
11861186
},
11871187
{
11881188
method: http.MethodGet,
11891189
url: "/search/code?q=rate",
1190-
category: codeSearchCategory,
1190+
category: CodeSearchCategory,
1191+
},
1192+
{
1193+
method: http.MethodGet,
1194+
url: "/orgs/google/audit-log",
1195+
category: AuditLogCategory,
11911196
},
11921197
// missing a check for actionsRunnerRegistrationCategory: API not found
11931198
}
11941199

11951200
for _, tt := range tests {
1196-
if got, want := category(tt.method, tt.url), tt.category; got != want {
1201+
if got, want := GetRateLimitCategory(tt.method, tt.url), tt.category; got != want {
11971202
t.Errorf("expecting category %v, found %v", got, want)
11981203
}
11991204
}

github/orgs_audit_log_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func TestOrganizationService_GetAuditLog(t *testing.T) {
164164
return err
165165
})
166166

167-
testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
167+
testNewRequestAndDoFailureCategory(t, methodName, client, AuditLogCategory, func() (*Response, error) {
168168
got, resp, err := client.Organizations.GetAuditLog(ctx, "o", &GetAuditLogOptions{})
169169
if got != nil {
170170
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)

github/rate_limit.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ type RateLimits struct {
5454
SCIM *Rate `json:"scim"`
5555
DependencySnapshots *Rate `json:"dependency_snapshots"`
5656
CodeSearch *Rate `json:"code_search"`
57+
AuditLog *Rate `json:"audit_log"`
5758
}
5859

5960
func (r RateLimits) String() string {
@@ -85,34 +86,37 @@ func (s *RateLimitService) Get(ctx context.Context) (*RateLimits, *Response, err
8586
if response.Resources != nil {
8687
s.client.rateMu.Lock()
8788
if response.Resources.Core != nil {
88-
s.client.rateLimits[coreCategory] = *response.Resources.Core
89+
s.client.rateLimits[CoreCategory] = *response.Resources.Core
8990
}
9091
if response.Resources.Search != nil {
91-
s.client.rateLimits[searchCategory] = *response.Resources.Search
92+
s.client.rateLimits[SearchCategory] = *response.Resources.Search
9293
}
9394
if response.Resources.GraphQL != nil {
94-
s.client.rateLimits[graphqlCategory] = *response.Resources.GraphQL
95+
s.client.rateLimits[GraphqlCategory] = *response.Resources.GraphQL
9596
}
9697
if response.Resources.IntegrationManifest != nil {
97-
s.client.rateLimits[integrationManifestCategory] = *response.Resources.IntegrationManifest
98+
s.client.rateLimits[IntegrationManifestCategory] = *response.Resources.IntegrationManifest
9899
}
99100
if response.Resources.SourceImport != nil {
100-
s.client.rateLimits[sourceImportCategory] = *response.Resources.SourceImport
101+
s.client.rateLimits[SourceImportCategory] = *response.Resources.SourceImport
101102
}
102103
if response.Resources.CodeScanningUpload != nil {
103-
s.client.rateLimits[codeScanningUploadCategory] = *response.Resources.CodeScanningUpload
104+
s.client.rateLimits[CodeScanningUploadCategory] = *response.Resources.CodeScanningUpload
104105
}
105106
if response.Resources.ActionsRunnerRegistration != nil {
106-
s.client.rateLimits[actionsRunnerRegistrationCategory] = *response.Resources.ActionsRunnerRegistration
107+
s.client.rateLimits[ActionsRunnerRegistrationCategory] = *response.Resources.ActionsRunnerRegistration
107108
}
108109
if response.Resources.SCIM != nil {
109-
s.client.rateLimits[scimCategory] = *response.Resources.SCIM
110+
s.client.rateLimits[ScimCategory] = *response.Resources.SCIM
110111
}
111112
if response.Resources.DependencySnapshots != nil {
112-
s.client.rateLimits[dependencySnapshotsCategory] = *response.Resources.DependencySnapshots
113+
s.client.rateLimits[DependencySnapshotsCategory] = *response.Resources.DependencySnapshots
113114
}
114115
if response.Resources.CodeSearch != nil {
115-
s.client.rateLimits[codeSearchCategory] = *response.Resources.CodeSearch
116+
s.client.rateLimits[CodeSearchCategory] = *response.Resources.CodeSearch
117+
}
118+
if response.Resources.AuditLog != nil {
119+
s.client.rateLimits[AuditLogCategory] = *response.Resources.AuditLog
116120
}
117121
s.client.rateMu.Unlock()
118122
}

0 commit comments

Comments
 (0)