8000 fix: fix determination of read/write endpoints · coderbyheart/github@a753ef8 · GitHub
[go: up one dir, main page]

Skip to content

Commit a753ef8

Browse files
committed
fix: fix determination of read/write endpoints
1 parent 5a3e287 commit a753ef8

File tree

2 files changed

+32
-7
lines changed

2 files changed

+32
-7
lines changed

lib/get-client.js

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,28 +31,30 @@ const getThrottler = memoize((rate, globalThrottler) =>
3131
/**
3232
* Determine if a call to a client function will trigger a read (`GET`) or a write (`POST`, `PATCH`, etc...) request.
3333
*
34+
* @param {String} limitKey The key to find the limit rate for the API endpoint and method.
3435
* @param {String} endpoint The client API enpoint (for example the endpoint for a call to `github.repos.get` is `repos`).
3536
* @param {String} command The client API command (for example the command for a call to `github.repos.get` is `get`).
3637
*
3738
* @return {String} `write` or `read` if there is rate limit configuration for this `endpoint` and `command`, `undefined` otherwise.
3839
*/
39-
const getAccess = (endpoint, command) => {
40+
const getAccess = (limitKey, endpoint, command) => {
4041
const method = GH_ROUTES[endpoint] && GH_ROUTES[endpoint][command] && GH_ROUTES[endpoint][command].method;
4142
const access = method && method === 'GET' ? 'read' : 'write';
42-
return RATE_LIMITS[endpoint][access] && access;
43+
return RATE_LIMITS[limitKey][access] && access;
4344
};
4445

4546
/**
4647
* Get the limiter identifier associated with a client API call.
4748
*
49+
* @param {String} limitKey The key to find the limit rate for the API endpoint and method.
4850
* @param {String} endpoint The client API enpoint (for example the endpoint for a call to `github.repos.get` is `repos`).
4951
* @param {String} command The client API command (for example the command for a call to `github.repos.get` is `get`).
5052
*
5153
* @return {String} A string identifying the limiter to use for this `endpoint` and `command` (e.g. `search` or `core.write`).
5254
*/
53-
const getLimitKey = (endpoint, command) => {
54-
return endpoint
55-
? [endpoint, RATE_LIMITS[endpoint] && getAccess(endpoint, command)].filter(Boolean).join('.')
55+
const getLimitKey = (limitKey, endpoint, command) => {
56+
return limitKey
57+
? [limitKey, RATE_LIMITS[limitKey] && getAccess(limitKey, endpoint, command)].filter(Boolean).join('.')
5658
: RATE_LIMITS[command]
5759
? command
5860
: 'core';
@@ -65,10 +67,11 @@ const getLimitKey = (endpoint, command) => {
6567
*
6668
* @param {Throttler} globalThrottler The throller function for the global rate limit.
6769
* @param {String} limitKey The key to find the limit rate for the API endpoint and method.
70+
* @param {String} endpoint The client API enpoint (for example the endpoint for a call to `github.repos.get` is `repos`).
6871
*
6972
* @return {Function} The `handler` for a `Proxy` wrapping an Octokit instance.
7073
*/
71-
const handler = (globalThrottler, limitKey) => ({
74+
const handler = (globalThrottler, limitKey, endpoint) => ({
7275
/**
7376
* If the target has the property as own, determine the rate limit based on the property name and recursively wrap the value in a `Proxy`. Otherwise returns the property value.
7477
*
@@ -80,7 +83,7 @@ const handler = (globalThrottler, limitKey) => ({
8083
*/
8184
get: (target, name, receiver) =>
8285
Reflect.apply(Object.prototype.hasOwnProperty, target, [name])
83-
? new Proxy(target[name], handler(globalThrottler, getLimitKey(limitKey, name)))
86+
? new Proxy(target[name], handler(globalThrottler, getLimitKey(limitKey, endpoint, name), endpoint || name))
8487
: Reflect.get(target, name, receiver),
8588

8689
/**

test/get-client.test.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,28 @@ test('Use the same throttler for endpoints in the same rate limit group', async
163163
t.true(inRange(f - e, searchRate - 50, searchRate + 50));
164164
});
165165

166+
test('Use different throttler for read and write endpoints', async t => {
167+
const createRelease = stub().callsFake(() => Date.now());
168+
const get = stub().callsFake(() => Date.now());
169+
const octokit = {repos: {createRelease, get}, authenticate: stub()};
170+
const writeRate = 300;
171+
const readRate = 150;
172+
const github = proxyquire('../lib/get-client', {
173+
'@octokit/rest': stub().returns(octokit),
174+
'./definitions/rate-limit': {RATE_LIMITS: {core: {write: writeRate, read: readRate}}, GLOBAL_RATE_LIMIT: 1},
175+
})();
176+
177+
const a = await github.repos.get();
178+
const b = await github.repos.get();
179+
const c = await github.repos.createRelease();
180+
const d = await github.repos.createRelease();
181+
182+
// `repos.get` should be called `readRate` ms after `repos.get`
183+
t.true(inRange(b - a, readRate - 50, readRate + 50));
184+
// `repos.createRelease` should be called `coreRate` ms after `repos.createRelease`
185+
t.true(inRange(d - c, writeRate - 50, writeRate + 50));
186+
});
187+
166188
test('Use the same throttler when retrying', async t => {
167189
// eslint-disable-next-line require-await
168190
const createRelease = stub().callsFake(async () => {

0 commit comments

Comments
 (0)
0