From d9d3f543ef9a27147f363e391bdbd5d63be339d2 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Mon, 7 Mar 2016 16:01:47 -0800 Subject: [PATCH 1/8] parse start parameter as an int first --- history.md | 4 ++++ lib/page.js | 2 +- package.json | 2 +- test/lib/page.js | 40 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/history.md b/history.md index c226af2..f463c2b 100644 --- a/history.md +++ b/history.md @@ -1,3 +1,7 @@ +# v0.2.16 / 2016-03-07 + +* mondern versions of mongoose expect the skip parameter to be an int. + # v0.2.15 / 2016-03-04 * Fixed issue where there was an incompatibility with mquery module in mongoose diff --git a/lib/page.js b/lib/page.js index 14ad161..7632da8 100644 --- a/lib/page.js +++ b/lib/page.js @@ -31,7 +31,7 @@ module.exports = function(mongoose) { } query - .skip(options.start) + .skip(parseInt(options.start, 10) || 0) .limit(options.count) .exec(function (err, results) { if (err) { diff --git a/package.json b/package.json index 056e148..a829e11 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "mongoose-middleware", "description": "Middleware for mongoose that makes filtering, sorting, pagination and projection chainable and simple to apply", - "version": "0.2.15", + "version": "0.2.16", "scripts": { "test": "gulp test-all" }, diff --git a/test/lib/page.js b/test/lib/page.js index e465043..075c153 100644 --- a/test/lib/page.js +++ b/test/lib/page.js @@ -175,4 +175,44 @@ describe('page', function () { return done(); }); }); + + it('should return results when start is a string', function (done) { + var options = { + start : "0", + count : 100 + }; + + pageLib.initialize({ maxDocs: 50 }); + + Kitteh + .find() + .page(options, function (err, data) { + should.not.exist(err); + should.exist(data); + + data.options.count.should.equals(50); + + return done(); + }); + }); + + it('should return results when start is NaN', function (done) { + var options = { + start : "start", + count : 100 + }; + + pageLib.initialize({ maxDocs: 50 }); + + Kitteh + .find() + .page(options, function (err, data) { + should.not.exist(err); + should.exist(data); + + data.options.count.should.equals(50); + + return done(); + }); + }); }); From 300091807faf16fa5473b153a878b1acccae5946 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Mon, 7 Mar 2016 16:14:47 -0800 Subject: [PATCH 2/8] limit needs to be parsed first too --- history.md | 2 +- lib/page.js | 7 ++++--- test/lib/page.js | 38 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/history.md b/history.md index f463c2b..de93689 100644 --- a/history.md +++ b/history.md @@ -1,6 +1,6 @@ # v0.2.16 / 2016-03-07 -* mondern versions of mongoose expect the skip parameter to be an int. +* mondern versions of mongoose expect the skip and limit parameter to be an int. # v0.2.15 / 2016-03-04 diff --git a/lib/page.js b/lib/page.js index 7632da8..1f8f262 100644 --- a/lib/page.js +++ b/lib/page.js @@ -18,8 +18,9 @@ module.exports = function(mongoose) { wrap = {}; options = options || defaults; - options.start = (options && options.start ? options.start : defaults.start); - options.count = (options && options.count ? options.count : defaults.count); + // this might be getting a little long; + options.start = (options && options.start && parseInt(options.start, 10) ? parseInt(options.start, 10) : defaults.start); + options.count = (options && options.count && parseInt(options.count, 10) ? parseInt(options.count, 10) : defaults.count); if (maxDocs > 0 && (options.count > maxDocs || options.count === 0)) { options.count = maxDocs; @@ -31,7 +32,7 @@ module.exports = function(mongoose) { } query - .skip(parseInt(options.start, 10) || 0) + .skip(options.start) .limit(options.count) .exec(function (err, results) { if (err) { diff --git a/test/lib/page.js b/test/lib/page.js index 075c153..53c8d36 100644 --- a/test/lib/page.js +++ b/test/lib/page.js @@ -190,8 +190,6 @@ describe('page', function () { should.not.exist(err); should.exist(data); - data.options.count.should.equals(50); - return done(); }); }); @@ -204,6 +202,42 @@ describe('page', function () { pageLib.initialize({ maxDocs: 50 }); + Kitteh + .find() + .page(options, function (err, data) { + should.not.exist(err); + should.exist(data); + + return done(); + }); + }); + + it('should return results when count is a string', function (done) { + var options = { + start : 0, + count : "100" + }; + + pageLib.initialize({ maxDocs: 50 }); + + Kitteh + .find() + .page(options, function (err, data) { + should.not.exist(err); + should.exist(data); + + return done(); + }); + }); + + it('should return results when count is NaN', function (done) { + var options = { + start : 0, + count : "count" + }; + + pageLib.initialize({ maxDocs: 50 }); + Kitteh .find() .page(options, function (err, data) { From 82772590eb0e16bc5a51dbf7e0f219a6baab3560 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Tue, 8 Mar 2016 10:06:14 -0800 Subject: [PATCH 3/8] or clauses need to be handed in as arrays --- lib/filter.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/filter.js b/lib/filter.js index 7095ccf..f3e0b5e 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -112,7 +112,8 @@ module.exports = function(mongoose) { self.where(key, term); } }); - }; + }, + orOptionsNode = {}; for (var key in spec) { if (spec.hasOwnProperty(key)) { @@ -122,7 +123,9 @@ module.exports = function(mongoose) { bulkApply(key, val); } else { if (isOptional) { - self.or(key, val); + orOptionsNode = {}; + orOptionsNode[key] = val; + self.or([orOptionsNode]); } else { self.where(key, val); } From 9ce78e108a8d8f54e32a6ea6a189ce1049881dfc Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Tue, 8 Mar 2016 11:03:38 -0800 Subject: [PATCH 4/8] initial cut of making unit tests pass again --- lib/filter.js | 9 ++++++--- test/lib/filter.js | 29 +++++++++++++++++------------ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/lib/filter.js b/lib/filter.js index f3e0b5e..0890fa9 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -104,16 +104,19 @@ module.exports = function(mongoose) { isOptional = false; } + var orOptionsNode = {}; + var bulkApply = function (key, val) { val.forEach(function (term) { if (isOptional) { - self.or(key, term); + orOptionsNode = {}; + orOptionsNode[key] = term; + self.or([orOptionsNode]); } else { self.where(key, term); } }); - }, - orOptionsNode = {}; + }; for (var key in spec) { if (spec.hasOwnProperty(key)) { diff --git a/test/lib/filter.js b/test/lib/filter.js index b717a70..b7159d6 100644 --- a/test/lib/filter.js +++ b/test/lib/filter.js @@ -28,19 +28,24 @@ describe('filter', function () { before(function () { filterLib = require('../../lib/filter')(mongoose); - mongoose.Query.prototype.or = function (key, val) { - if (typeof val === 'undefined') { - val = { expr : '', val : null }; - } - - if (orClause[key]) { - var newVal = [orClause[key], val]; - orClause[key] = newVal; - } else { - orClause[key] = val; + mongoose.Query.prototype.or = function (orOptions) { + + if (Array.isArray(orOptions)) { + orOptions.forEach(function (elem) { + for (var x in elem) { + if (elem.hasOwnProperty(x)) { + if (orClause[x]) { + var newVal = [orClause[x], elem[x]]; + orClause[x] = newVal; + } else { + orClause[x] = elem[x]; + } + } + } + }); } - return { + /*return { gt : function (v) { orClause[key].expr = 'gt'; orClause[key].val = v; @@ -61,7 +66,7 @@ describe('filter', function () { orClause[key].expr = 'ne'; orClause[key].val = v; } - }; + };*/ }; mongoose.Query.prototype.where = function (key, val) { From 08e751b3f356247a8c6250bbc2572116a0607ab7 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Tue, 8 Mar 2016 11:40:24 -0800 Subject: [PATCH 5/8] reevaluating how the optionals are built --- lib/filter.js | 66 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/lib/filter.js b/lib/filter.js index 0890fa9..997914a 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -99,22 +99,10 @@ module.exports = function(mongoose) { } }; - var applyRegex = function (spec, buildRegex, isOptional) { - if (typeof isOptional === 'undefined') { - isOptional = false; - } - - var orOptionsNode = {}; - + var applyRegex = function (spec, buildRegex) { var bulkApply = function (key, val) { val.forEach(function (term) { - if (isOptional) { - orOptionsNode = {}; - orOptionsNode[key] = term; - self.or([orOptionsNode]); - } else { - self.where(key, term); - } + self.where(key, term); }); }; @@ -125,16 +113,44 @@ module.exports = function(mongoose) { if (Array.isArray(val)) { bulkApply(key, val); } else { - if (isOptional) { - orOptionsNode = {}; - orOptionsNode[key] = val; - self.or([orOptionsNode]); - } else { - self.where(key, val); - } + self.where(key, val); + } + } + } + }, + // mongoose.Query.prototype.or handles or clauses differently than + // before. time was you could pass in a key value pair, now it looks + // like it expects array of objects + applyRegexAsOptional = function (spec, buildRegex) { + function bulkApply(key, val) { + var node = {}, + nodeOptions = []; + + val.forEach(function (term) { + node = {}; + node[key] = term; + nodeOptions.push(node); + }); + + return nodeOptions; + } + var orOptions = [], + orOptionsNode = {}; + + for (var key in spec) { + if (spec.hasOwnProperty(key)) { + var val = buildRegex(spec[key]); + + if (Array.isArray(val)) { + orOptions = orOptions.concat(bulkApply(key, val)); + } else { + orOptionsNode = {}; + orOptionsNode[key] = val; + orOptions.push(orOptionsNode); } } } + self.or(orOptions); }; var regexContains = function (val) { @@ -205,10 +221,10 @@ module.exports = function(mongoose) { mandatory.notEqual || mandatory.notEqualTo || mandatory.ne || {}); // OPTIONAL - applyRegex(optional.contains, regexContains, true); - applyRegex(optional.endsWith, regexEndsWith, true); - applyRegex(optional.startsWith, regexStartsWith, true); - applyRegex(optional.exact, regexExact, true); + applyRegexAsOptional(optional.contains, regexContains); + applyRegexAsOptional(optional.endsWith, regexEndsWith); + applyRegexAsOptional(optional.startsWith, regexStartsWith); + applyRegexAsOptional(optional.exact, regexExact); applyGreaterThan( optional.greaterThan || optional.gt || {}, From 2017dc4da2fee40b43358ddaaaff102fdeaca72b Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Tue, 8 Mar 2016 14:03:18 -0800 Subject: [PATCH 6/8] gte,gt,lte,lt,ne does not work with or clauses --- lib/filter.js | 57 +++++++++------------------------------------------ 1 file changed, 10 insertions(+), 47 deletions(-) diff --git a/lib/filter.js b/lib/filter.js index 997914a..4e14332 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -38,63 +38,42 @@ module.exports = function(mongoose) { return val; }; - var applyGreaterThan = function (spec, isOptional) { - if (typeof isOptional === 'undefined') { - isOptional = false; - } - + var applyGreaterThan = function (spec) { for (var key in spec) { if (spec.hasOwnProperty(key)) { - (isOptional ? self.or(key) : self.where(key)).gt(spec[key]); + self.where(key).gt(spec[key]); } } }; - var applyGreaterThanEqual = function (spec, isOptional) { - if (typeof isOptional === 'undefined') { - isOptional = false; - } - + var applyGreaterThanEqual = function (spec) { for (var key in spec) { if (spec.hasOwnProperty(key)) { - (isOptional ? self.or(key) : self.where(key)).gte(spec[key]); + self.where(key).gte(spec[key]); } } }; - var applyLesserThan = function (spec, isOptional) { - if (typeof isOptional === 'undefined') { - isOptional = false; - } - + var applyLesserThan = function (spec) { for (var key in spec) { if (spec.hasOwnProperty(key)) { - (isOptional ? self.or(key) : self.where(key)).lt(spec[key]); + self.where(key).lt(spec[key]); } } }; - var applyLesserThanEqual = function (spec, isOptional) { - if (typeof isOptional === 'undefined') { - isOptional = false; - } - + var applyLesserThanEqual = function (spec) { for (var key in spec) { if (spec.hasOwnProperty(key)) { - (isOptional ? self.or(key) : self.where(key)).lte(spec[key]); + self.where(key).lte(spec[key]); } } }; - var applyNotEqual = function (spec, isOptional) { - if (typeof isOptional === 'undefined') { - isOptional = false; - } - + var applyNotEqual = function (spec) { for (var key in spec) { if (spec.hasOwnProperty(key)) { - (isOptional ? self.or(key) : self.where(key)) - .ne(analyzeWhereSpec(spec[key])); + self.where(key).ne(analyzeWhereSpec(spec[key])); } } }; @@ -226,22 +205,6 @@ module.exports = function(mongoose) { applyRegexAsOptional(optional.startsWith, regexStartsWith); applyRegexAsOptional(optional.exact, regexExact); - applyGreaterThan( - optional.greaterThan || optional.gt || {}, - true); - applyGreaterThanEqual( - optional.greaterThanEqual || optional.gte || {}, - true); - applyLesserThan( - optional.lessThan || optional.lt || {}, - true); - applyLesserThanEqual( - optional.lessThanEqual || optional.lte || {}, - true); - applyNotEqual( - optional.notEqual || optional.notEqualTo || optional.ne || {}, - true); - return self; }; }; From cd38d9ffd40a77688f682ba77101f17ee357b876 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Tue, 8 Mar 2016 14:14:12 -0800 Subject: [PATCH 7/8] update docs --- history.md | 1 + readme.md | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/history.md b/history.md index de93689..06d668b 100644 --- a/history.md +++ b/history.md @@ -1,6 +1,7 @@ # v0.2.16 / 2016-03-07 * mondern versions of mongoose expect the skip and limit parameter to be an int. +* remove ability to specify gt,gte,lt,lte and ne parameters with an optional filter # v0.2.15 / 2016-03-04 diff --git a/readme.md b/readme.md index 2c7a7f6..b84f866 100644 --- a/readme.md +++ b/readme.md @@ -193,10 +193,14 @@ KittehModel Filters can be used in three ways: mandatory, optional and keyword searches. Additionally, for mandatory and optional searches, exact, contains and startsWith string pattern matches may be used. +The following filters can be used for *mandatory*, *optional*, and *keyword* searches. + * `exact` - Matches the string letter for letter, but is not case sensitive * `contains` - Matches documents where the string exists as a substring of the field (similar to a where field like '%term%' query in a relational datastore) * `startsWith` - Matches documents where field begins with the string supplied (similar to a where field like 'term%' query in a relational datastore) * `endsWith` - Matches documents where field ends with the string supplied (similar to a where field like '%term' query in a relational datastore) + +The following filters can *ONLY* be used for *mandatory* and *keyword* searches. * `greaterThan` (or `gt`) - Matches documents where field value is greater than supplied number or Date value in query * `greaterThanEqual` (or `gte`) - Matches documents where field value is greater than or equal to supplied number or Date value in query * `lessThan` (or `lt`) - Matches documents where field value is less than supplied number or Date value in query From c9f149af27ba7cd35ad127fd26e13178a0795649 Mon Sep 17 00:00:00 2001 From: Miguel Ruiz Date: Tue, 8 Mar 2016 14:28:04 -0800 Subject: [PATCH 8/8] remove unsupported filtering options of or queries throw exceptions now --- test/lib/filter.js | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/test/lib/filter.js b/test/lib/filter.js index b7159d6..930c2b6 100644 --- a/test/lib/filter.js +++ b/test/lib/filter.js @@ -45,28 +45,33 @@ describe('filter', function () { }); } - /*return { - gt : function (v) { - orClause[key].expr = 'gt'; - orClause[key].val = v; + // it doesn't seem the mquery/mongoose supports subsequent gt,lt, + // gte,lte,ne filtering for or queries, however prior to v0.2.16 of + // mongoose-middleware some features were built as though it was + // supported. this will give us some indication if any code remains + // that tries to use these filtering options + return { + gt : function () { + throw new Error( + 'mongoose.Query.prototype.or does not support gt'); }, - gte : function (v) { - orClause[key].expr = 'gte'; - orClause[key].val = v; + gte : function () { + throw new Error( + 'mongoose.Query.prototype.or does not support gte'); }, - lt : function (v) { - orClause[key].expr = 'lt'; - orClause[key].val = v; + lt : function () { + throw new Error( + 'mongoose.Query.prototype.or does not support lt'); }, - lte : function (v) { - orClause[key].expr = 'lte'; - orClause[key].val = v; + lte : function () { + throw new Error( + 'mongoose.Query.prototype.or does not support lte'); }, - ne : function (v) { - orClause[key].expr = 'ne'; - orClause[key].val = v; + ne : function () { + throw new Error( + 'mongoose.Query.prototype.or does not support ne'); } - };*/ + }; }; mongoose.Query.prototype.where = function (key, val) {