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.

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

update for v5 #370

Merged
merged 14 commits into from
Jul 6, 2024
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/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
permissions:
contents: write
pull-requests: write
uses: fastify/workflows/.github/workflows/plugins-ci-redis.yml@v4.0.0
uses: fastify/workflows/.github/workflows/plugins-ci-redis.yml@v5.0.0
with:
license-check: true
lint: true
7 changes: 1 addition & 6 deletions .taprc
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
jobs: 1

branches: 96
functions: 100
lines: 100
statements: 98

disable-coverage: true
Comment on lines -3 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it better to impose a fixed limit, so that we're aware when the code coverage drop?

Maybe we can open an issue to increase code coverage if possible.

Copy link
Member Author

@gurgunday gurgunday Jul 6, 2024

Choose a reason for hiding this comment

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

Maybe we can open an issue to increase code coverage if possible.

Yes

Isn't it better to impose a fixed limit, so that we're aware when the code coverage drop?

I'm not aware if that exists in tap@20, happy to add it back if so

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not aware if that exists in tap@20, happy to add it back if so

Yep, they broke a few things.
Maybe it's better to fix coverage in an independent PR, will take a look tomorrow

files:
- test/**/*.test.js
20 changes: 10 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,21 @@
},
"homepage": "https://github.com/fastify/fastify-rate-limit#readme",
"devDependencies": {
"@fastify/pre-commit": "^2.0.2",
"@fastify/pre-commit": "^2.1.0",
"@sinonjs/fake-timers": "^11.2.2",
"@types/node": "^20.8.10",
"fastify": "^4.24.3",
"ioredis": "^5.3.2",
"knex": "^3.0.1",
"sqlite3": "^5.1.6",
"@types/node": "^20.14.10",
"fastify": "^4.28.1",
"ioredis": "^5.4.1",
"knex": "^3.1.0",
"sqlite3": "^5.1.7",
"standard": "^17.1.0",
"tap": "16.3.9",
"tsd": "^0.31.0"
"tap": "20.0.3",
"tsd": "^0.31.1"
},
"dependencies": {
"@lukeed/ms": "^2.0.1",
"@lukeed/ms": "^2.0.2",
"fastify-plugin": "^4.5.1",
"toad-cache": "^3.3.1"
"toad-cache": "^3.7.0"
},
"publishConfig": {
"access": "public"
Expand Down
82 changes: 23 additions & 59 deletions test/global-rate-limit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ test('Basic', async t => {
t.equal(res.headers['x-ratelimit-limit'], '2')
t.equal(res.headers['x-ratelimit-remaining'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('With text timeWindow', async t => {
Expand Down Expand Up @@ -101,9 +99,7 @@ test('With text timeWindow', async t => {
t.equal(res.headers['x-ratelimit-limit'], '2')
t.equal(res.headers['x-ratelimit-remaining'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('With function timeWindow', async t => {
Expand Down Expand Up @@ -149,9 +145,7 @@ test('With function timeWindow', async t => {
t.equal(res.headers['x-ratelimit-limit'], '2')
t.equal(res.headers['x-ratelimit-remaining'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('When passing NaN to the timeWindow property then the timeWindow should be the default value - 60 seconds', async t => {
Expand Down Expand Up @@ -191,9 +185,7 @@ test('When passing NaN to the timeWindow property then the timeWindow should be

t.equal(res.statusCode, 200)

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('With ips allowList, allowed ips should not result in rate limiting', async t => {
Expand Down Expand Up @@ -483,10 +475,8 @@ test('With redis store', async t => {
t.equal(res.headers['x-ratelimit-remaining'], '1')
t.equal(res.headers['x-ratelimit-reset'], '1')

t.teardown(async () => {
await redis.flushall()
await redis.quit()
})
await redis.flushall()
await redis.quit()
})

test('With redis store (ban)', async t => {
Expand Down Expand Up @@ -539,10 +529,8 @@ test('With redis store (ban)', async t => {
t.equal(res.headers['x-ratelimit-remaining'], '0')
t.equal(res.headers['x-ratelimit-reset'], '1')

t.teardown(async () => {
await redis.flushall()
await redis.quit()
})
await redis.flushall()
await redis.quit()
})

test('Skip on redis error', async t => {
Expand Down Expand Up @@ -661,9 +649,7 @@ test('With keyGenerator', async t => {
t.equal(res.headers['x-ratelimit-limit'], '2')
t.equal(res.headers['x-ratelimit-remaining'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('With async/await keyGenerator', async t => {
Expand Down Expand Up @@ -932,9 +918,7 @@ test('hide rate limit headers', async t => {
t.equal(res.headers['x-ratelimit-remaining'], '0')
t.equal(res.headers['x-ratelimit-reset'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('hide rate limit headers on exceeding', async t => {
Expand Down Expand Up @@ -980,9 +964,7 @@ test('hide rate limit headers on exceeding', async t => {
t.notOk(res.headers['x-ratelimit-remaining'], 'the header must be missing')
t.notOk(res.headers['x-ratelimit-reset'], 'the header must be missing')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('hide rate limit headers at all times', async t => {
Expand Down Expand Up @@ -1034,9 +1016,7 @@ test('hide rate limit headers at all times', async t => {
t.notOk(res.headers['x-ratelimit-remaining'], 'the header must be missing')
t.notOk(res.headers['x-ratelimit-reset'], 'the header must be missing')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('With ban', async t => {
Expand Down Expand Up @@ -1142,9 +1122,7 @@ test('With enabled IETF Draft Spec', async t => {
t.equal(res.headers['ratelimit-limit'], '2')
t.equal(res.headers['ratelimit-remaining'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('hide IETF draft spec headers', async t => {
Expand Down Expand Up @@ -1192,9 +1170,7 @@ test('hide IETF draft spec headers', async t => {
t.equal(res.headers['ratelimit-remaining'], '0')
t.equal(res.headers['ratelimit-reset'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('afterReset and Rate Limit remain the same when enableDraftSpec is enabled', async t => {
Expand Down Expand Up @@ -1231,9 +1207,7 @@ test('afterReset and Rate Limit remain the same when enableDraftSpec is enabled'
t.equal(res.headers['ratelimit-reset'], res.headers['retry-after'])
}

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('Before async in "max"', async t => {
Expand Down Expand Up @@ -1335,10 +1309,8 @@ test('When continue exceeding is on (Redis)', async t => {
t.equal(second.headers['x-ratelimit-remaining'], '0')
t.equal(second.headers['x-ratelimit-reset'], '5')

t.teardown(async () => {
await redis.flushall()
await redis.quit()
})
await redis.flushall()
await redis.quit()
})

test('Redis with continueExceeding should not always return the timeWindow as ttl', async t => {
Expand Down Expand Up @@ -1394,10 +1366,8 @@ test('Redis with continueExceeding should not always return the timeWindow as tt
t.equal(res.headers['x-ratelimit-remaining'], '0')
t.equal(res.headers['x-ratelimit-reset'], '3')

t.teardown(async () => {
await redis.flushall()
await redis.quit()
})
await redis.flushall()
await redis.quit()
})

test('When use a custom nameSpace', async t => {
Expand Down Expand Up @@ -1459,10 +1429,8 @@ test('When use a custom nameSpace', async t => {
t.equal(res.headers['x-ratelimit-remaining'], '1')
t.equal(res.headers['x-ratelimit-reset'], '1')

t.teardown(async () => {
await redis.flushall()
await redis.quit()
})
await redis.flushall()
await redis.quit()
})

test('on preHandler hook', async t => {
Expand Down Expand Up @@ -1554,9 +1522,7 @@ test('ban directly', async t => {
t.equal(res.headers['x-ratelimit-limit'], '2')
t.equal(res.headers['x-ratelimit-remaining'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('wrong timewindow', async t => {
Expand Down Expand Up @@ -1608,7 +1574,5 @@ test('wrong timewindow', async t => {
t.equal(res.headers['x-ratelimit-limit'], '2')
t.equal(res.headers['x-ratelimit-remaining'], '0')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})
50 changes: 14 additions & 36 deletions test/route-rate-limit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@ test('Basic', async t => {
t.equal(res.headers['x-ratelimit-remaining'], '1')
t.equal(res.headers['x-ratelimit-reset'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('With text timeWindow', async t => {
Expand Down Expand Up @@ -134,9 +132,7 @@ test('With text timeWindow', async t => {
t.equal(res.headers['x-ratelimit-limit'], '2')
t.equal(res.headers['x-ratelimit-remaining'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('With function timeWindow', async t => {
Expand Down Expand Up @@ -188,9 +184,7 @@ test('With function timeWindow', async t => {
t.equal(res.headers['x-ratelimit-limit'], '2')
t.equal(res.headers['x-ratelimit-remaining'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('With ips allowList', async t => {
Expand Down Expand Up @@ -370,10 +364,8 @@ test('With redis store', async t => {
t.equal(res.headers['x-ratelimit-remaining'], '1')
t.equal(res.headers['x-ratelimit-reset'], '1')

t.teardown(async () => {
await redis.flushall()
await redis.quit()
})
await redis.flushall()
await redis.quit()
})

test('Throw on redis error', async t => {
Expand Down Expand Up @@ -505,9 +497,7 @@ test('With keyGenerator', async t => {
t.equal(res.headers['x-ratelimit-limit'], '2')
t.equal(res.headers['x-ratelimit-remaining'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('no rate limit without settings', async t => {
Expand Down Expand Up @@ -828,9 +818,7 @@ test('hide rate limit headers', async t => {
t.equal(res.headers['x-ratelimit-remaining'], '0')
t.equal(res.headers['x-ratelimit-reset'], '1')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('hide rate limit headers on exceeding', async t => {
Expand Down Expand Up @@ -884,9 +872,7 @@ test('hide rate limit headers on exceeding', async t => {
t.notOk(res.headers['x-ratelimit-remaining'], 'the header must be missing')
t.notOk(res.headers['x-ratelimit-reset'], 'the header must be missing')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('hide rate limit headers at all times', async t => {
Expand Down Expand Up @@ -952,9 +938,7 @@ test('hide rate limit headers at all times', async t => {
t.equal(res.headers['x-ratelimit-remaining'], '0')
t.notOk(res.headers['x-ratelimit-reset'], 'the header must be missing')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})

test('global timeWindow when not set in routes', async t => {
Expand Down Expand Up @@ -1395,10 +1379,8 @@ test('When continue exceeding is on (Redis)', async t => {
t.equal(second.headers['x-ratelimit-remaining'], '0')
t.equal(second.headers['x-ratelimit-reset'], '5')

t.teardown(async () => {
await redis.flushall()
await redis.quit()
})
await redis.flushall()
await redis.quit()
})

test('When continue exceeding is off under route (Redis)', async t => {
Expand Down Expand Up @@ -1449,10 +1431,8 @@ test('When continue exceeding is off under route (Redis)', async t => {
t.equal(third.headers['x-ratelimit-remaining'], '0')
t.equal(third.headers['x-ratelimit-reset'], '3')

t.teardown(async () => {
await redis.flushall()
await redis.quit()
})
await redis.flushall()
await redis.quit()
})

test('should consider routes allow list', async t => {
Expand Down Expand Up @@ -1721,7 +1701,5 @@ test('With NaN in subroute config', async t => {
t.equal(res.headers['x-ratelimit-limit'], '1000')
t.equal(res.headers['x-ratelimit-remaining'], '999')

t.teardown(() => {
t.context.clock.uninstall()
})
t.context.clock.uninstall()
})