From bfb2c69f3f8062ab557fc81dcc2bc02b8dbe27c7 Mon Sep 17 00:00:00 2001 From: groyoh Date: Tue, 1 Sep 2015 19:52:16 +0200 Subject: [PATCH] Changed query params handling to accept query params matching --- .jshintrc | 1 + lib/middleware/route-handlers.js | 5 +- lib/parse/url.js | 16 ++-- lib/query-comparator.js | 30 +++++-- package.json | 3 +- test/api/query-parameter-test.js | 29 +++++++ test/example/md/query-parameters.md | 46 ++++++++++ test/unit/query-comparator-test.js | 129 +++++++++++++++++++++------- test/unit/url-parser-test.js | 38 ++++++-- 9 files changed, 238 insertions(+), 59 deletions(-) diff --git a/.jshintrc b/.jshintrc index 69054f4..84f1624 100644 --- a/.jshintrc +++ b/.jshintrc @@ -11,6 +11,7 @@ "it": false, "console": false, "describe": false, + "context": false, "before": false, "after": false, "expect": false, diff --git a/lib/middleware/route-handlers.js b/lib/middleware/route-handlers.js index be68d79..335da6a 100644 --- a/lib/middleware/route-handlers.js +++ b/lib/middleware/route-handlers.js @@ -27,14 +27,13 @@ module.exports = function(options, cb) { }); if (handlers) { - var queryParams = Object.keys(req.query); - if (queryParams.length === 0){ + var queryParams = req.query; + if (Object.keys(queryParams).length === 0){ handlers.sort(queryComparator.noParamComparator); } else { queryComparator.countMatchingQueryParms(handlers, queryParams); handlers.sort(queryComparator.queryParameterComparator); } - var requestHandled = false; handlers.forEach(function (handler) { if (requestHandled) { diff --git a/lib/parse/url.js b/lib/parse/url.js index 8fdc02f..74aa4eb 100644 --- a/lib/parse/url.js +++ b/lib/parse/url.js @@ -1,5 +1,7 @@ +var qs = require('qs'); + //https://github.com/apiaryio/api-blueprint/blob/master/API%20Blueprint%20Specification.md#operators -var PARAMETER_OPERATORS_REGEX = /\#|\+|\?|\&/g; +var PARAMETER_OPERATORS_REGEX = /\#|\+|\?/g; var PATHNAME_REGEX = /\{(.*?)\}/g; var URL_SPLIT_REGEX = /\{?\?/; @@ -9,22 +11,16 @@ exports.parse = function (url) { var processPathname = function(path){ return path.replace(PATHNAME_REGEX, ':$1'); }; - var processQueryParameters = function(queryParams) { if (!queryParams) { - return []; + return {}; } - - return queryParams.split(/\,|\{|\}/g).map(function(i){ - return i.replace(PARAMETER_OPERATORS_REGEX, ''); - }).filter(function(i){ - return i.length > 0; - }); + return qs.parse(queryParams.replace(/\{|\}/g,'').replace(/\,/g,'&').replace(PARAMETER_OPERATORS_REGEX, '')); }; return { uriTemplate: url, - queryParams: urlArr.length > 1 ? processQueryParameters(urlArr[1]) : [], + queryParams: urlArr.length > 1 ? processQueryParameters(urlArr[1]) : {}, url: processPathname(urlArr[0]) }; }; diff --git a/lib/query-comparator.js b/lib/query-comparator.js index ce437bb..f6d2003 100644 --- a/lib/query-comparator.js +++ b/lib/query-comparator.js @@ -1,10 +1,15 @@ +var _ = require('lodash'); + exports.noParamComparator = function(a, b){ - return (a.parsedUrl.queryParams.length - b.parsedUrl.queryParams.length); + return (Object.keys(a.parsedUrl.queryParams).length - Object.keys(b.parsedUrl.queryParams).length); }; exports.queryParameterComparator = function(a, b){ if (b.matchingQueryParams === a.matchingQueryParams){ - return (a.nonMatchingQueryParams - b.nonMatchingQueryParams); + if (b.exactMatchingQueryParams === a.exactMatchingQueryParams){ + return (a.nonMatchingQueryParams - b.nonMatchingQueryParams); + } + return (b.exactMatchingQueryParams - a.exactMatchingQueryParams); } return (b.matchingQueryParams - a.matchingQueryParams); }; @@ -12,13 +17,22 @@ exports.queryParameterComparator = function(a, b){ exports.countMatchingQueryParms = function (handlers, reqQueryParams){ handlers.forEach(function(handler){ handler.matchingQueryParams = 0; + handler.exactMatchingQueryParams = 0; handler.nonMatchingQueryParams = 0; - handler.parsedUrl.queryParams.forEach(function(templateQueryParam){ - if (reqQueryParams.indexOf(templateQueryParam)>-1){ - handler.matchingQueryParams +=1; - } else { + var specQueryParams = handler.parsedUrl.queryParams; + for(var param in specQueryParams){ + var value = specQueryParams[param]; + if (reqQueryParams.hasOwnProperty(param)){ + var reqValue = reqQueryParams[param]; + if (_.isEqual(value, reqValue)){ + handler.matchingQueryParams += 1; + handler.exactMatchingQueryParams += 1; + }else if (value === ''){ + handler.matchingQueryParams += 1; + } + }else{ handler.nonMatchingQueryParams +=1; } - }); + } }); -}; \ No newline at end of file +}; diff --git a/package.json b/package.json index 4ab0b68..2948684 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,8 @@ "lodash": "^3.8.0", "optimist": "0.6.1", "path-to-regexp": "^1.0.3", - "tv4": "^1.1.9" + "tv4": "^1.1.9", + "qs": "^4.0.0" }, "devDependencies": { "grunt": "^0.4.5", diff --git a/test/api/query-parameter-test.js b/test/api/query-parameter-test.js index d6ce720..4b0bf52 100644 --- a/test/api/query-parameter-test.js +++ b/test/api/query-parameter-test.js @@ -75,4 +75,33 @@ describe('Query Parameters', function(){ }); }); + describe('/api/things?param1=12345{¶m2}', function(){ + it('should respond with response specified in a endpoint with "param1" and "param2" parameters', function(done){ + request.get('/api/query?param1=12345¶m2=2') + .expect(200) + .expect('Content-type', 'application/json;charset=UTF-8') + .expect({id: 'parameter1_12345_parameter2'}) + .end(helper.endCb(done)); + }); + }); + + describe('/api/things?param1=12345¶m1=6789', function(){ + it('should respond with response specified in a endpoint with "param1" parameter as array', function(done){ + request.get('/api/query?param1=12345¶m1=6789') + .expect(200) + .expect('Content-type', 'application/json;charset=UTF-8') + .expect({id: 'parameter1_12345_6789'}) + .end(helper.endCb(done)); + }); + }); + + describe('/api/things?param1[key1]=12345¶m1[key2]=6789', function(){ + it('should respond with response specified in a endpoint with "param1" parameter as object', function(done){ + request.get('/api/query?param1[key1]=12345¶m1[key2]=6789') + .expect(200) + .expect('Content-type', 'application/json;charset=UTF-8') + .expect({id: 'parameter1_key1_12345_key2_6789'}) + .end(helper.endCb(done)); + }); + }); }); diff --git a/test/example/md/query-parameters.md b/test/example/md/query-parameters.md index 76aa5be..144609f 100644 --- a/test/example/md/query-parameters.md +++ b/test/example/md/query-parameters.md @@ -82,3 +82,49 @@ See [Blueprint API - URI parameters section](https://github.com/apiaryio/api-blu { "id": "parameter2_parameter3" } + +## Things [/api/query?param1=12345{¶m2}] + ++ Parameters + + param1 (string, `12345`) ... Parameter for the request + + param2 (string, `12345`) ... Parameter for the request + +### Get with query parameter [GET] + ++ Response 200 (application/json;charset=UTF-8) + + + Body + + { + "id": "parameter1_12345_parameter2" + } + +## Things [/api/query?param1=12345¶m1=6789] + ++ Parameters + + param1 (array, [`12345`,`6789`]) ... Parameter for the request + +### Get with query parameter [GET] + ++ Response 200 (application/json;charset=UTF-8) + + + Body + + { + "id": "parameter1_12345_6789" + } + +## Things [/api/query?param1[key1]=12345¶m1[key2]=6789] + ++ Parameters + + param1 (object, { `key1`: `12345`, `key2`: `6789` }) ... Parameter for the request + +### Get with query parameter [GET] + ++ Response 200 (application/json;charset=UTF-8) + + + Body + + { + "id": "parameter1_key1_12345_key2_6789" + } diff --git a/test/unit/query-comparator-test.js b/test/unit/query-comparator-test.js index 727abf5..8a67b1b 100644 --- a/test/unit/query-comparator-test.js +++ b/test/unit/query-comparator-test.js @@ -6,17 +6,17 @@ describe('Query Comparator', function() { describe('noParamComparator', function() { it('Should sort by number of query parameters in the specification', function(){ var requests = [ - {parsedUrl: {queryParams: [1,2,4]}}, - {parsedUrl: {queryParams: [1,4]}}, - {parsedUrl: {queryParams: [1]}}, - {parsedUrl: {queryParams: []}} + {parsedUrl: {queryParams: {'param1': ''}}}, + {parsedUrl: {queryParams: {'param1': '', 'param2': '', 'param3': ''}}}, + {parsedUrl: {queryParams: {'param3': '', 'param4': ''}}}, + {parsedUrl: {queryParams: {'param2': '', 'param4': '', 'param1': '12345'}}} ]; var expected = [ - {parsedUrl: {queryParams: []}}, - {parsedUrl: {queryParams: [1]}}, - {parsedUrl: {queryParams: [1, 4]}}, - {parsedUrl: {queryParams: [1, 2, 4]}} + {parsedUrl: {queryParams: {'param1': ''}}}, + {parsedUrl: {queryParams: {'param3': '', 'param4': ''}}}, + {parsedUrl: {queryParams: {'param1': '', 'param2': '', 'param3': ''}}}, + {parsedUrl: {queryParams: {'param2': '', 'param4': '', 'param1': '12345'}}} ]; requests.sort(queryComparator.noParamComparator); @@ -27,17 +27,19 @@ describe('Query Comparator', function() { describe('queryParameterComparator', function() { it('Should sort by number of query parameters in the specification', function(){ var requests = [ - {matchingQueryParams: 1, nonMatchingQueryParams: 5}, - {matchingQueryParams: 2, nonMatchingQueryParams: 1}, - {matchingQueryParams: 2, nonMatchingQueryParams: 2}, - {matchingQueryParams: 4, nonMatchingQueryParams: 1} + {matchingQueryParams: 1, exactMatchingQueryParams: 0, nonMatchingQueryParams: 5}, + {matchingQueryParams: 2, exactMatchingQueryParams: 0, nonMatchingQueryParams: 2}, + {matchingQueryParams: 2, exactMatchingQueryParams: 0, nonMatchingQueryParams: 1}, + {matchingQueryParams: 2, exactMatchingQueryParams: 1, nonMatchingQueryParams: 2}, + {matchingQueryParams: 4, exactMatchingQueryParams: 0, nonMatchingQueryParams: 1} ]; var expected = [ - {matchingQueryParams: 4, nonMatchingQueryParams: 1}, - {matchingQueryParams: 2, nonMatchingQueryParams: 1}, - {matchingQueryParams: 2, nonMatchingQueryParams: 2}, - {matchingQueryParams: 1, nonMatchingQueryParams: 5} + {matchingQueryParams: 4, exactMatchingQueryParams: 0, nonMatchingQueryParams: 1}, + {matchingQueryParams: 2, exactMatchingQueryParams: 1, nonMatchingQueryParams: 2}, + {matchingQueryParams: 2, exactMatchingQueryParams: 0, nonMatchingQueryParams: 1}, + {matchingQueryParams: 2, exactMatchingQueryParams: 0, nonMatchingQueryParams: 2}, + {matchingQueryParams: 1, exactMatchingQueryParams: 0, nonMatchingQueryParams: 5} ]; requests.sort(queryComparator.queryParameterComparator); @@ -46,23 +48,90 @@ describe('Query Comparator', function() { }); describe('countMatchingQueryParms', function() { - it('Should count the number of matching and non matching query parameters against handlers query parameters', function(){ + var requestParams = { + 'param1': '12345', + 'param2': '6789', + 'param5': ['12345','6789'], + 'param6': {'key1': '12345', 'key2': '6789'} + }; + var handlers = null; - var handlers = [ - {parsedUrl: {queryParams: ['param1']}}, - {parsedUrl: {queryParams: ['param1', 'param2', 'param3']}}, - {parsedUrl: {queryParams: ['param3', 'param4']}} - ]; + beforeEach(function(){ + queryComparator.countMatchingQueryParms(handlers, requestParams); + }); - var requestParams = ['param1', 'param2']; + context('when no value is given', function() { + var handler = {parsedUrl: {queryParams: {'param1': ''}}}; - queryComparator.countMatchingQueryParms(handlers, requestParams); - assert.equal(handlers[0].matchingQueryParams, 1); - assert.equal(handlers[0].nonMatchingQueryParams, 0); - assert.equal(handlers[1].matchingQueryParams, 2); - assert.equal(handlers[1].nonMatchingQueryParams, 1); - assert.equal(handlers[2].matchingQueryParams, 0); - assert.equal(handlers[2].nonMatchingQueryParams, 2); + before(function(){ + handlers = [handler]; + }); + + it('Should count the number of matching and non matching query parameters against handlers query parameters', function(){ + assert.equal(handler.matchingQueryParams, 1); + assert.equal(handler.exactMatchingQueryParams, 0); + assert.equal(handler.nonMatchingQueryParams, 0); + }); + }); + + context('when multiples params are given', function() { + before(function(){ + handlers = [ + {parsedUrl: {queryParams: {'param1': '', 'param2': '', 'param3': ''}}}, + {parsedUrl: {queryParams: {'param3': '', 'param4': ''}}} + ]; + }); + + it('Should count the number of matching and non matching query parameters against handlers query parameters', function(){ + assert.equal(handlers[0].matchingQueryParams, 2); + assert.equal(handlers[0].exactMatchingQueryParams, 0); + assert.equal(handlers[0].nonMatchingQueryParams, 1); + assert.equal(handlers[1].matchingQueryParams, 0); + assert.equal(handlers[1].exactMatchingQueryParams, 0); + assert.equal(handlers[1].nonMatchingQueryParams, 2); + }); + }); + + context('when param is a string', function() { + var handler = {parsedUrl: {queryParams: {'param1': '12345', 'param2': '', 'param4': ''}}}; + + before(function(){ + handlers = [handler]; + }); + + it('Should count the number of matching and non matching query parameters against handlers query parameters', function(){ + assert.equal(handler.matchingQueryParams, 2); + assert.equal(handler.exactMatchingQueryParams, 1); + assert.equal(handler.nonMatchingQueryParams, 1); + }); + }); + + context('when param is an array', function() { + var handler = {parsedUrl: {queryParams: {'param5': ['12345','6789'], 'param4': ''}}}; + + before(function(){ + handlers = [handler]; + }); + + it('Should count the number of matching and non matching query parameters against handlers query parameters', function(){ + assert.equal(handler.matchingQueryParams, 1); + assert.equal(handler.exactMatchingQueryParams, 1); + assert.equal(handler.nonMatchingQueryParams, 1); + }); + }); + + context('when param is an object', function() { + var handler = {parsedUrl: {queryParams: {'param6': {'key1': '12345', 'key2': '6789'}, 'param4': ''}}}; + + before(function(){ + handlers = [handler]; + }); + + it('Should count the number of matching and non matching query parameters against handlers query parameters', function(){ + assert.equal(handler.matchingQueryParams, 1); + assert.equal(handler.exactMatchingQueryParams, 1); + assert.equal(handler.nonMatchingQueryParams, 1); + }); }); }); diff --git a/test/unit/url-parser-test.js b/test/unit/url-parser-test.js index 381cc83..534ac80 100644 --- a/test/unit/url-parser-test.js +++ b/test/unit/url-parser-test.js @@ -9,7 +9,7 @@ describe('URL Parser', function() { var parsed = urlParser.parse(url); assert.equal(parsed.url, url); - assert.deepEqual(parsed.queryParams, []); + assert.deepEqual(parsed.queryParams, {}); }); }); @@ -20,7 +20,7 @@ describe('URL Parser', function() { var parsed = urlParser.parse(url); assert.equal(parsed.url, '/api/:meow/:woof/things'); - assert.deepEqual(parsed.queryParams, []); + assert.deepEqual(parsed.queryParams, {}); }); }); @@ -31,7 +31,7 @@ describe('URL Parser', function() { var parsed = urlParser.parse(url); assert.equal(parsed.url, '/api/things'); - assert.deepEqual(parsed.queryParams, ['param']); + assert.deepEqual(parsed.queryParams, {'param': ''}); }); it('Should get path and query parameters: scenario 2', function(){ @@ -39,7 +39,7 @@ describe('URL Parser', function() { var parsed = urlParser.parse(url); assert.equal(parsed.url, '/api/things'); - assert.deepEqual(parsed.queryParams, ['param','param2']); + assert.deepEqual(parsed.queryParams, {'param': '', 'param2': ''}); }); it('Should get path and query parameters: scenario 3', function(){ @@ -47,7 +47,7 @@ describe('URL Parser', function() { var parsed = urlParser.parse(url); assert.equal(parsed.url, '/api/things'); - assert.deepEqual(parsed.queryParams, ['param','param2','param3']); + assert.deepEqual(parsed.queryParams, {'param': '','param2': '','param3': ''}); }); it('Should get path and query parameters: scenario 4', function(){ @@ -55,7 +55,31 @@ describe('URL Parser', function() { var parsed = urlParser.parse(url); assert.equal(parsed.url, '/api/things'); - assert.deepEqual(parsed.queryParams, ['param1','param2']); + assert.deepEqual(parsed.queryParams, {'param1': '','param2': ''}); + }); + + it('Should get path and query parameters: scenario 5', function(){ + var url = '/api/things?param1=12345{¶m2}'; + var parsed = urlParser.parse(url); + + assert.equal(parsed.url, '/api/things'); + assert.deepEqual(parsed.queryParams, {'param1': '12345','param2': ''}); + }); + + it('Should get path and query parameters: scenario 6', function(){ + var url = '/api/things?param1=12345¶m1=6789{¶m2}'; + var parsed = urlParser.parse(url); + + assert.equal(parsed.url, '/api/things'); + assert.deepEqual(parsed.queryParams, {'param1': ['12345','6789'],'param2': ''}); + }); + + it('Should get path and query parameters: scenario 7', function(){ + var url = '/api/things?param1[key1]=12345¶m1[key2]=6789{¶m2}'; + var parsed = urlParser.parse(url); + + assert.equal(parsed.url, '/api/things'); + assert.deepEqual(parsed.queryParams, {'param1': { 'key1': '12345','key2': '6789'},'param2': ''}); }); }); @@ -66,7 +90,7 @@ describe('URL Parser', function() { var parsed = urlParser.parse(url); assert.equal(parsed.url, '/api/:meow/things'); - assert.deepEqual(parsed.queryParams, ['param']); + assert.deepEqual(parsed.queryParams, {'param': ''}); }); });