Skip to content

Commit d8e3355

Browse files
Simon Grondingr2m
authored andcommitted
feat(#4): match routes with a regex
1 parent d17ac6d commit d8e3355

File tree

4 files changed

+64
-12
lines changed

4 files changed

+64
-12
lines changed

lib/index.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,16 @@ module.exports = throttlingPlugin
22

33
const Bottleneck = require('bottleneck/light')
44
const wrapRequest = require('./wrap-request')
5-
65
const triggersNotificationPaths = require('./triggers-notification-paths')
6+
const routeMatcher = require('./route-matcher')(triggersNotificationPaths)
7+
8+
// Workaround to allow tests to directly access the triggersNotification function.
9+
const triggersNotification = throttlingPlugin.triggersNotification =
10+
routeMatcher.test.bind(routeMatcher)
711

812
function throttlingPlugin (octokit, octokitOptions = {}) {
913
const state = Object.assign({
10-
triggersNotification: triggersNotificationPaths,
14+
triggersNotification,
1115
minimumAbuseRetryAfter: 5,
1216
retryAfterBaseValue: 1000,
1317
globalLimiter: new Bottleneck({

lib/route-matcher.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
module.exports = routeMatcher
2+
3+
function routeMatcher (paths) {
4+
// EXAMPLE. For the following paths:
5+
/* [
6+
"/orgs/:org/invitations",
7+
"/repos/:owner/:repo/collaborators/:username"
8+
] */
9+
10+
const regexes = paths.map(p =>
11+
p.split('/')
12+
.map(c => c.startsWith(':') ? '(?:.+?)' : c)
13+
.join('/')
14+
)
15+
// 'regexes' would contain:
16+
/* [
17+
'/orgs/(?:.+?)/invitations',
18+
'/repos/(?:.+?)/(?:.+?)/collaborators/(?:.+?)'
19+
] */
20+
21+
const regex = `^(?:${regexes.map(r => `(?:${r})`).join('|')})[^/]*$`
22+
// 'regex' would contain:
23+
/*
24+
^(?:(?:\/orgs\/(?:.+?)\/invitations)|(?:\/repos\/(?:.+?)\/(?:.+?)\/collaborators\/(?:.+?)))[^\/]*$
25+
26+
It may look scary, but paste it into https://www.debuggex.com/
27+
and it will make a lot more sense!
28+
*/
29+
30+
return new RegExp(regex, 'i')
31+
}

lib/wrap-request.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ async function doRequest (state, request, options) {
1717
}
1818

1919
// Guarantee at least 3000ms between requests that trigger notifications
20-
if (isWrite && state.triggersNotification.includes(options.url)) {
20+
if (isWrite && state.triggersNotification(options.url)) {
2121
await state.triggersNotificationLimiter.schedule(jobOptions, noop)
2222
}
2323

test/index.js

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,23 @@ describe('Github API best practices', function () {
121121
expect(octokit.__requestTimings[5] - octokit.__requestTimings[0]).to.be.closeTo(100, 20)
122122
})
123123

124+
it('Should match custom routes when checking notification triggers', function () {
125+
const plugin = require('../lib')
126+
127+
expect(plugin.triggersNotification('/abc/def')).to.equal(false)
128+
expect(plugin.triggersNotification('/orgs/abc/invitation')).to.equal(false)
129+
expect(plugin.triggersNotification('/repos/abc/releases')).to.equal(false)
130+
expect(plugin.triggersNotification('/repos/abc/def/pulls/5')).to.equal(false)
131+
132+
expect(plugin.triggersNotification('/repos/abc/def/pulls')).to.equal(true)
133+
expect(plugin.triggersNotification('/repos/abc/def/pulls/5/comments')).to.equal(true)
134+
expect(plugin.triggersNotification('/repos/foo/bar/issues')).to.equal(true)
135+
136+
expect(plugin.triggersNotification('/repos/:owner/:repo/pulls')).to.equal(true)
137+
expect(plugin.triggersNotification('/repos/:owner/:repo/pulls/5/comments')).to.equal(true)
138+
expect(plugin.triggersNotification('/repos/:foo/:bar/issues')).to.equal(true)
139+
})
140+
124141
it('Should optimize throughput rather than maintain ordering', async function () {
125142
const octokit = new Octokit({
126143
throttle: {
@@ -131,7 +148,7 @@ describe('Github API best practices', function () {
131148
}
132149
})
133150

134-
const req1 = octokit.request('POST /orgs/:org/invitations', {
151+
const req1 = octokit.request('POST /orgs/abc/invitations', {
135152
request: {
136153
responses: [{ status: 200, headers: {}, data: {} }]
137154
}
@@ -151,12 +168,12 @@ describe('Github API best practices', function () {
151168
responses: [{ status: 200, headers: {}, data: {} }]
152169
}
153170
})
154-
const req5 = octokit.request('POST /repos/:owner/:repo/commits/:sha/comments', {
171+
const req5 = octokit.request('POST /repos/abc/def/commits/12345/comments', {
155172
request: {
156173
responses: [{ status: 200, headers: {}, data: {} }]
157174
}
158175
})
159-
const req6 = octokit.request('PATCH /orgs/:org/invitations', {
176+
const req6 = octokit.request('PATCH /orgs/abc/invitations', {
160177
request: {
161178
responses: [{ status: 200, headers: {}, data: {} }]
162179
}
@@ -173,14 +190,14 @@ describe('Github API best practices', function () {
173190
'END GET /route2',
174191
'START GET /route4',
175192
'END GET /route4',
176-
'START POST /orgs/:org/invitations',
177-
'END POST /orgs/:org/invitations',
193+
'START POST /orgs/abc/invitations',
194+
'END POST /orgs/abc/invitations',
178195
'START POST /route3',
179196
'END POST /route3',
180-
'START POST /repos/:owner/:repo/commits/:sha/comments',
181-
'END POST /repos/:owner/:repo/commits/:sha/comments',
182-
'START PATCH /orgs/:org/invitations',
183-
'END PATCH /orgs/:org/invitations',
197+
'START POST /repos/abc/def/commits/12345/comments',
198+
'END POST /repos/abc/def/commits/12345/comments',
199+
'START PATCH /orgs/abc/invitations',
200+
'END PATCH /orgs/abc/invitations',
184201
'START GET /route6',
185202
'END GET /route6'
186203
])

0 commit comments

Comments
 (0)