Skip to content

Commit

Permalink
Remove tidy option. fix #51
Browse files Browse the repository at this point in the history
  • Loading branch information
@noamshemesh
noamshemesh committed Aug 16, 2019
1 parent d8e1e50 commit d289aa6
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 290 deletions.
2 changes: 2 additions & 0 deletions Readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Rate limiter for Node.js backed by Redis.
[![Build Status](https://travis-ci.org/tj/node-ratelimiter.svg)](https://travis-ci.org/tj/node-ratelimiter)

## Release Notes
[v3.3.1](https://github.com/tj/node-ratelimiter/tree/v3.3.1) - [#51](/../../issues/47) - Remove tidy option as it's always true.

[v3.3.0](https://github.com/tj/node-ratelimiter/tree/v3.3.0) - [#47](/../../pull/47) by [@penghap](https://github.com/penghap) - Add tidy option to clean old records upon saving new records. Drop support in node 4.

[v3.2.0](https://github.com/tj/node-ratelimiter/tree/v3.2.0) - [#44](/../../pull/44) by [@xdmnl](https://github.com/xdmnl) - Return accurate reset time for each limited call.
Expand Down
Binary file added dump.rdb
View file
Binary file not shown.
7 changes: 2 additions & 5 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ module.exports = Limiter;
function Limiter(opts) {
this.id = opts.id;
this.db = opts.db;
this.tidy = opts.tidy || false;
assert(this.id, '.id required');
assert(this.db, '.db required');
this.max = opts.max || 2500;
Expand Down Expand Up @@ -59,7 +58,6 @@ Limiter.prototype.inspect = function() {

Limiter.prototype.get = function (fn) {
var db = this.db;
var tidy = this.key;
var duration = this.duration;
var key = this.key;
var max = this.max;
Expand All @@ -73,9 +71,8 @@ Limiter.prototype.get = function (fn) {
['zrange', key, -max, -max],
['pexpire', key, duration],
]
if (tidy) {
operations.splice(5, 0, ['zremrangebyrank', key, 0, -(max + 1)])
}

operations.splice(5, 0, ['zremrangebyrank', key, 0, -(max + 1)])
db.multi(operations)
.exec(function (err, res) {
if (err) return fn(err);
Expand Down
286 changes: 1 addition & 285 deletions test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,6 @@ var Limiter = require('..'),
done();
});
});

it('should represent the total limit per reset period, when the tidy is true', function (done) {
var limit = new Limiter({
max: 5,
id: 'something',
db: db,
tidy: true
});
limit.get(function (err, res) {
res.total.should.equal(5);
done();
});
});
});

describe('.remaining', function() {
Expand All @@ -65,26 +52,6 @@ var Limiter = require('..'),
});
});
});

it('should represent the number of requests remaining in the reset period, when the tidy is true', function (done) {
var limit = new Limiter({
max: 5,
duration: 100000,
id: 'something',
db: db,
tidy: true
});
limit.get(function (err, res) {
res.remaining.should.equal(5);
limit.get(function (err, res) {
res.remaining.should.equal(4);
limit.get(function (err, res) {
res.remaining.should.equal(3);
done();
});
});
});
});
});

describe('.reset', function() {
Expand All @@ -101,21 +68,6 @@ var Limiter = require('..'),
done();
});
});

it('should represent the next reset time in UTC epoch seconds, when the tidy is true', function (done) {
var limit = new Limiter({
max: 5,
duration: 60000,
id: 'something',
db: db,
tidy: true
});
limit.get(function (err, res) {
var left = res.reset - (Date.now() / 1000);
left.should.be.below(60).and.be.greaterThan(0);
done();
});
});
});

describe('.resetMs', function() {
Expand All @@ -133,22 +85,6 @@ var Limiter = require('..'),
done();
});
});

it('should represent the next reset time in UTC epoch milliseconds, when the tidy is true', function (done) {
var limit = new Limiter({
max: 5,
duration: 60000,
id: 'something',
db: db,
tidy: true
});
limit.get(function (err, res) {
var left = res.resetMs - Date.now();
Number.isInteger(left).should.be.true;
left.should.be.within(0, 60000);
done();
});
});
});

describe('when the limit is exceeded', function() {
Expand Down Expand Up @@ -192,48 +128,6 @@ var Limiter = require('..'),
});
});

describe('when the limit is exceeded and tidy is true', function () {
var limit;

beforeEach(function (done) {
limit = new Limiter({
max: 2,
id: 'something',
db: db,
tidy: true
});

limit.get(function () {
limit.get(function () {
done();
});
});
});

it('should retain .remaining at 0', function (done) {
limit.get(function (err, res) {
// function caller should reject this call
res.remaining.should.equal(0);
done();
});
});

it('should return an increasing reset time after each call', function (done) {
setTimeout(function () {
limit.get(function (err, res) {
var originalResetMs = res.resetMs;

setTimeout(function () {
limit.get(function (err, res) {
res.resetMs.should.be.greaterThan(originalResetMs);
done();
});
}, 10);
});
}, 10);
});
});

describe('when the duration is exceeded', function() {
it('should reset', function(done) {
this.timeout(5000);
Expand All @@ -258,31 +152,6 @@ var Limiter = require('..'),
});
});
});

it('should reset, when tidy is true', function (done) {
this.timeout(5000);
var limit = new Limiter({
duration: 2000,
max: 2,
id: 'something',
db: db,
tidy: true
});
limit.get(function (err, res) {
res.remaining.should.equal(2);
limit.get(function (err, res) {
res.remaining.should.equal(1);
setTimeout(function () {
limit.get(function (err, res) {
var left = res.reset - (Date.now() / 1000);
left.should.be.below(2);
res.remaining.should.equal(2);
done();
});
}, 3000);
});
});
});
});

describe('when multiple successive calls are made', function() {
Expand All @@ -303,23 +172,6 @@ var Limiter = require('..'),
});
});

it('the next calls should not create again the limiter in Redis, when tidy is true', function (done) {
var limit = new Limiter({
duration: 10000,
max: 2,
id: 'something',
db: db,
tidy: true
});
limit.get(function (err, res) {
res.remaining.should.equal(2);
});

limit.get(function (err, res) {
res.remaining.should.equal(1);
done();
});
});
it('updating the count should keep all TTLs in sync', function(done) {
var limit = new Limiter({
duration: 10000,
Expand All @@ -344,31 +196,6 @@ var Limiter = require('..'),
});
});
});
it('updating the count should keep all TTLs in sync, when tidy is true', function (done) {
var limit = new Limiter({
duration: 10000,
max: 2,
id: 'something',
db: db,
tidy: true
});
limit.get(function (err, res) {}); // All good here.
limit.get(function (err, res) {
db.multi()
.pttl(['limit:something:count'])
.pttl(['limit:something:limit'])
.pttl(['limit:something:reset'])
.exec(function (err, res) {
if (err) return done(err);
var ttlCount = (typeof res[0] === 'number') ? res[0] : res[0][1];
var ttlLimit = (typeof res[1] === 'number') ? res[1] : res[1][1];
var ttlReset = (typeof res[2] === 'number') ? res[2] : res[2][1];
ttlLimit.should.equal(ttlCount);
ttlReset.should.equal(ttlCount);
done();
});
});
});
});

describe('when trying to decrease before setting value', function() {
Expand All @@ -392,27 +219,6 @@ var Limiter = require('..'),
});
});
});
it('should create with ttl when trying to decrease, when tidy is true', function (done) {
var limit = new Limiter({
duration: 10000,
max: 2,
id: 'something',
db: db,
tidy: true
});
db.setex('limit:something:count', -1, 1, function () {
limit.get(function (err, res) {
res.remaining.should.equal(2);
limit.get(function (err, res) {
res.remaining.should.equal(1);
limit.get(function (err, res) {
res.remaining.should.equal(0);
done();
});
});
});
});
});
});

describe('when multiple concurrent clients modify the limit', function() {
Expand Down Expand Up @@ -477,70 +283,6 @@ var Limiter = require('..'),
});
});

describe('when multiple concurrent clients modify the limit and tidy is true', function () {
var clientsCount = 7,
max = 5,
left = max,
limits = [];

for (var i = 0; i < clientsCount; ++i) {
limits.push(new Limiter({
duration: 10000,
max: max,
id: 'something',
db: redisModule.createClient(),
tidy: true
}));
}

it('should prevent race condition and properly set the expected value', function (done) {
var responses = [];

function complete() {
responses.push(arguments);

if (responses.length == clientsCount) {
// If there were any errors, report.
var err = responses.some(function (res) {
return res[0];
});

if (err) {
done(err);
} else {
responses.sort(function (r1, r2) {
return r1[1].remaining < r2[1].remaining;
});
responses.forEach(function (res) {
res[1].remaining.should.equal(left < 0 ? 0 : left);
left--;
});

for (var i = max - 1; i < clientsCount; ++i) {
responses[i][1].remaining.should.equal(0);
}

done();
}
}
}

// Warm up and prepare the data.
limits[0].get(function (err, res) {
if (err) {
done(err);
} else {
res.remaining.should.equal(left--);

// Simulate multiple concurrent requests.
limits.forEach(function (limit) {
limit.get(complete);
});
}
});
});
});

describe('when limiter is called in parallel by multiple clients', function() {
var max = 6,
limiter;
Expand All @@ -565,32 +307,6 @@ var Limiter = require('..'),

});
});
})

describe('when limiter is called in parallel by multiple clients and tidy is true', function () {
var max = 6,
limiter;

limiter = new Limiter({
duration: 10000,
max: max,
id: 'asyncsomething',
db: redisModule.createClient()
});

it('should set the count properly without race conditions', function (done) {
async.times(max, function (n, next) {
limiter.get(next);
},
function (errs, limits) {

limits.forEach(function (limit) {
limit.remaining.should.equal(max--);
});
done();

});
});
})
});
});
});

0 comments on commit d289aa6

Please sign in to comment.