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.

Sign up for GitHub

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

Hotfix/fix negative or not parsable limit bug #24

Merged
merged 3 commits into from
Sep 29, 2016
Merged

Hotfix/fix negative or not parsable limit bug #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85eea5a
add default for non-parsable limit
Aug 16, 2016
7493923
move the offset calculation after the negative limit check
Aug 16, 2016
18a57b4
cover limit and page edge cases with tests
Aug 16, 2016
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
6 changes: 3 additions & 3 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,7 @@ exports.middleware = function middleware(limit, maxLimit) {

req.query.page = (typeof req.query.page === 'string') ? parseInt(req.query.page, 10) || 1 : 1;

req.query.limit = (typeof req.query.limit === 'string') ? parseInt(req.query.limit, 10) : _limit;

req.skip = req.offset = (req.query.page * req.query.limit) - req.query.limit;
req.query.limit = (typeof req.query.limit === 'string') ? parseInt(req.query.limit, 10) || 0 : _limit;

if (req.query.limit > _maxLimit)
req.query.limit = _maxLimit;
Expand All @@ -108,6 +106,8 @@ exports.middleware = function middleware(limit, maxLimit) {
if (req.query.limit < 0)
req.query.limit = 0;

req.skip = req.offset = (req.query.page * req.query.limit) - req.query.limit;

res.locals.paginate = {};
res.locals.paginate.page = req.query.page;
res.locals.paginate.limit = req.query.limit;
Expand Down
247 changes: 157 additions & 90 deletions test/index-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ var url = require('url');
var reqres = require('reqres');
var async = require('async');

describe('paginate', function() {
describe('paginate', function () {

describe('.href(req)', function() {
describe('.href(req)', function () {

beforeEach(function() {
beforeEach(function () {
this.req = {
originalUrl: 'http://niftylettuce.com/',
query: {
Expand All @@ -18,72 +18,72 @@ describe('paginate', function() {
};
});

it('should return function', function() {
it('should return function', function () {
paginate.href(this.req).should.be.a('function');
});

describe('the returned function', function() {
describe('the returned function', function () {

it('should return the next page when invoked with no arguments', function() {
it('should return the next page when invoked with no arguments', function () {
paginate.href(this.req)().should.equal(util.format('%s?page=%d', url.parse(this.req.originalUrl).pathname, this.req.query.page + 1));
});

it('should return the next page when invoked with `false` as the first argument', function() {
it('should return the next page when invoked with `false` as the first argument', function () {
paginate.href(this.req)().should.equal(util.format('%s?page=%d', url.parse(this.req.originalUrl).pathname, this.req.query.page + 1));
});

it('should return the previous page href when invoked with `true` as the first argument', function() {
it('should return the previous page href when invoked with `true` as the first argument', function () {
paginate.href(this.req)(true).should.equal(util.format('%s?page=%d', url.parse(this.req.originalUrl).pathname, this.req.query.page - 1));
});

it('should return the previous page href and sorted by title when invoked with `true` and a `params` object', function() {
it('should return the previous page href and sorted by title when invoked with `true` and a `params` object', function () {
paginate.href(this.req)(true, { sort: 'title' }).should.equal(util.format('%s?page=%d&sort=title', url.parse(this.req.originalUrl).pathname, this.req.query.page - 1));
});

it('should return the next page href and sorted by title when invoked with `false` and a `params` object', function() {
it('should return the next page href and sorted by title when invoked with `false` and a `params` object', function () {
paginate.href(this.req)(false, { sort: 'title' }).should.equal(util.format('%s?page=%d&sort=title', url.parse(this.req.originalUrl).pathname, this.req.query.page + 1));
});

it('should return the current page sorted by title when invoked with just a `params` object as the first argument', function() {
it('should return the current page sorted by title when invoked with just a `params` object as the first argument', function () {
paginate.href(this.req)({ sort: 'title' }).should.equal(util.format('%s?page=%d&sort=title', url.parse(this.req.originalUrl).pathname, this.req.query.page));
});

});

});

describe('.hasNextPages(req)', function() {
describe('.hasNextPages(req)', function () {

beforeEach(function() {
beforeEach(function () {
this.req = {
query: {
page: 3
}
};
});

it('should return function', function() {
it('should return function', function () {
paginate.hasNextPages(this.req).should.be.a('function');
});

describe('the returned function', function() {
describe('the returned function', function () {

it('should return true when there are more pages', function() {
it('should return true when there are more pages', function () {
paginate.hasNextPages(this.req)(4).should.be.true;
});

it('should return false when there are no more pages', function() {
it('should return false when there are no more pages', function () {
paginate.hasNextPages(this.req)(3).should.be.false;
});

it('should throw an error when pageCount is not a number', function(){
(function(){
it('should throw an error when pageCount is not a number', function () {
(function () {
paginate.hasNextPages(this.req)('');
}).should.throw(/not a number/);
});

it('should throw an error when pageCount is less than zero', function() {
(function(){
it('should throw an error when pageCount is less than zero', function () {
(function () {
paginate.hasNextPages(this.req)('');
}).should.throw(/\>= 0/);
});
Expand All @@ -92,123 +92,190 @@ describe('paginate', function() {

});

describe('.middleware(limit, maxLimit)', function() {

describe('should be a pure function with successive calls not mutating', function() {

var firstMiddleware, secondMiddleware;

beforeEach(function() {
firstMiddleware = paginate.middleware(10, 20);
secondMiddleware = paginate.middleware(30, 40);
});

it('limit in previous calls', function(done) {
async.parallel(
[
function(callback) {
var req = reqres.req();
firstMiddleware(req, reqres.res(), function(err) {
req.query.limit.should.equal(10);
callback(null, null);
})
},
function(callback) {
var req = reqres.req();
secondMiddleware(req, reqres.res(), function(err) {
req.query.limit.should.equal(30);
callback(null, null);
})
}
],
done
);
});

it('maxLimit in previous calls', function(done) {
async.parallel(
[
function(callback) {
var req = reqres.req({query: {limit: '100'}});
firstMiddleware(req, reqres.res(), function(err) {
req.query.limit.should.equal(20);
callback(null, null);
})
},
function(callback) {
var req = reqres.req({query: {limit: '100'}});
secondMiddleware(req, reqres.res(), function(err) {
req.query.limit.should.equal(40);
callback(null, null);
})
}
],
done
);
});
describe('.middleware(limit, maxLimit)', function () {

describe('should be a pure function with successive calls not mutating', function () {

var firstMiddleware, secondMiddleware;

beforeEach(function () {
firstMiddleware = paginate.middleware(10, 20);
secondMiddleware = paginate.middleware(30, 40);
});

it('limit in previous calls', function (done) {
async.parallel(
[
function (callback) {
var req = reqres.req();
firstMiddleware(req, reqres.res(), function (err) {
req.query.limit.should.equal(10);
callback(null, null);
})
},
function (callback) {
var req = reqres.req();
secondMiddleware(req, reqres.res(), function (err) {
req.query.limit.should.equal(30);
callback(null, null);
})
}
],
done
);
});

it('maxLimit in previous calls', function (done) {
async.parallel(
[
function (callback) {
var req = reqres.req({ query: { limit: '100' } });
firstMiddleware(req, reqres.res(), function (err) {
req.query.limit.should.equal(20);
callback(null, null);
})
},
function (callback) {
var req = reqres.req({ query: { limit: '100' } });
secondMiddleware(req, reqres.res(), function (err) {
req.query.limit.should.equal(40);
callback(null, null);
})
}
],
done
);
});

it('limit, skip and offset should be zero, page should be one', function (done) {
async.parallel(
[
function (callback) {
var req = reqres.req({ query: { page: '0', limit: '0' } });
firstMiddleware(req, reqres.res(), function (err) {
req.query.page.should.equal(1);
req.query.limit.should.equal(0);
req.skip.should.equal(0);
req.offset.should.equal(0);
callback(null, null);
})
},
function (callback) {
var req = reqres.req({ query: { page: '0', limit: '0' } });
secondMiddleware(req, reqres.res(), function (err) {
req.query.page.should.equal(1);
req.query.limit.should.equal(0);
req.skip.should.equal(0);
req.offset.should.equal(0);
callback(null, null);
})
},
function (callback) {
var req = reqres.req({ query: { page: '-1', limit: '-1' } });
firstMiddleware(req, reqres.res(), function (err) {
req.query.page.should.equal(1);
req.query.limit.should.equal(0);
req.skip.should.equal(0);
req.offset.should.equal(0);
callback(null, null);
})
},
function (callback) {
var req = reqres.req({ query: { page: '-10', limit: '-10' } });
secondMiddleware(req, reqres.res(), function (err) {
req.query.page.should.equal(1);
req.query.limit.should.equal(0);
req.skip.should.equal(0);
req.offset.should.equal(0);
callback(null, null);
})
},
function (callback) {
var req = reqres.req({ query: { page: 'notInt', limit: 'notInt' } });
firstMiddleware(req, reqres.res(), function (err) {
req.query.limit.should.equal(0);
req.query.page.should.equal(1);
req.skip.should.equal(0);
req.offset.should.equal(0);
callback(null, null);
})
},
function (callback) {
var req = reqres.req({ query: { page: 'notInt', limit: 'notInt' } });
secondMiddleware(req, reqres.res(), function (err) {
req.query.limit.should.equal(0);
req.query.page.should.equal(1);
req.skip.should.equal(0);
req.offset.should.equal(0);
callback(null, null);
})
}
],
done
);
})

});

});

describe('.getArrayPages(limit, pageCount, currentPaget)', function() {
describe('.getArrayPages(limit, pageCount, currentPaget)', function () {

var pages;
var limit = 5,
fakeMaxCount = 10;

beforeEach(function() {
beforeEach(function () {
this.req = {
originalUrl: 'http://niftylettuce.com/',
query: {
page: 3
}
};

pages = paginate.getArrayPages(this.req)(limit, fakeMaxCount, this.req.query.page);
pages = paginate.getArrayPages(this.req)(limit, fakeMaxCount, this.req.query.page);
});

it('should return arrays of pages', function() {
it('should return arrays of pages', function () {
pages.should.be.a('array');
});

it('it should contain correct page numbers and page urls', function() {
pages.forEach(function(p, i) {
it('it should contain correct page numbers and page urls', function () {
pages.forEach(function (p, i) {
// index from 1
var idx = i + 1;
p.number.should.equal(idx);
p.url.should.contain('page='+(idx));
p.url.should.contain('page=' + (idx));
})
})

it('it should contain correct page numbers and page urls', function() {
pages.forEach(function(p, i) {
it('it should contain correct page numbers and page urls', function () {
pages.forEach(function (p, i) {
// index from 1
var idx = i + 1;
p.number.should.equal(idx);
p.url.should.contain('page='+(idx));
p.url.should.contain('page=' + (idx));
})
})

it('check validations', function() {
it('check validations', function () {

var contextReq = this.req;

// negative limit
(function(){
(function () {
paginate.getArrayPages(contextReq)(-1, fakeMaxCount, contextReq.query.page);
}).should.throw(/\>= 0/);

// pageCount is string
(function(){
(function () {
paginate.getArrayPages(contextReq)(limit, '', contextReq.query.page);
}).should.throw(/\>= 0/);

// currentPage is object
(function(){
(function () {
paginate.getArrayPages(contextReq)(limit, fakeMaxCount, {});
}).should.throw(/\>= 0/);
})
})

});