From 8c81b3063f916f7c60ae638e18c6661f036b7a38 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 10:54:14 +0100 Subject: [PATCH 1/9] Move filter validation into its own module. --- lib/filter.js | 31 +++++++++++++++++++++++++++++++ lib/routes/search.js | 26 ++------------------------ 2 files changed, 33 insertions(+), 24 deletions(-) create mode 100644 lib/filter.js diff --git a/lib/filter.js b/lib/filter.js new file mode 100644 index 00000000..cb51db97 --- /dev/null +++ b/lib/filter.js @@ -0,0 +1,31 @@ +/* @flow weak */ +"use strict"; +var filter = module.exports = { }; + + +filter.validateParams = function(request, callback) { + if (!request.params.filter) return callback(); + + for (var i in request.params.filter) { + if (request.params.filter[i] instanceof Object) continue; + if (!request.resourceConfig.attributes[i]) { + return callback({ + status: "403", + code: "EFORBIDDEN", + title: "Invalid filter", + detail: request.resourceConfig.resource + " do not have property " + i + }); + } + var relationSettings = request.resourceConfig.attributes[i]._settings; + if (relationSettings && relationSettings.__as) { + return callback({ + status: "403", + code: "EFORBIDDEN", + title: "Request validation failed", + detail: "Requested relation \"" + i + "\" is a foreign reference and does not exist on " + request.params.type + }); + } + } + + return callback(); +}; diff --git a/lib/routes/search.js b/lib/routes/search.js index c45402d7..0e10e5de 100644 --- a/lib/routes/search.js +++ b/lib/routes/search.js @@ -5,6 +5,7 @@ var searchRoute = module.exports = { }; var async = require("async"); var helper = require("./helper.js"); var router = require("../router.js"); +var filter = require("../filter.js"); var pagination = require("../pagination.js"); var postProcess = require("../postProcess.js"); var responseHelper = require("../responseHelper.js"); @@ -27,30 +28,7 @@ searchRoute.register = function() { helper.validate(request.params, resourceConfig.searchParams, callback); }, function validateFilterParams(callback) { - if (!request.params.filter) return callback(); - - for (var i in request.params.filter) { - if (request.params.filter[i] instanceof Object) continue; - if (!request.resourceConfig.attributes[i]) { - return callback({ - status: "403", - code: "EFORBIDDEN", - title: "Invalid filter", - detail: request.resourceConfig.resource + " do not have property " + i - }); - } - var relationSettings = request.resourceConfig.attributes[i]._settings; - if (relationSettings && relationSettings.__as) { - return callback({ - status: "403", - code: "EFORBIDDEN", - title: "Request validation failed", - detail: "Requested relation \"" + i + "\" is a foreign reference and does not exist on " + request.params.type - }); - } - } - - return callback(); + filter.validateParams(request, callback); }, function validatePaginationParams(callback) { pagination.validatePaginationParams(request); From b95828edbb254f6e372d49f209726cc4a5d14694 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 11:54:50 +0100 Subject: [PATCH 2/9] More accurate error reporting and testing. --- lib/filter.js | 6 +++--- test/get-resource.js | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/filter.js b/lib/filter.js index cb51db97..3337713c 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -13,7 +13,7 @@ filter.validateParams = function(request, callback) { status: "403", code: "EFORBIDDEN", title: "Invalid filter", - detail: request.resourceConfig.resource + " do not have property " + i + detail: request.resourceConfig.resource + " do not have attribute or relationship '" + i + "'" }); } var relationSettings = request.resourceConfig.attributes[i]._settings; @@ -21,8 +21,8 @@ filter.validateParams = function(request, callback) { return callback({ status: "403", code: "EFORBIDDEN", - title: "Request validation failed", - detail: "Requested relation \"" + i + "\" is a foreign reference and does not exist on " + request.params.type + title: "Invalid filter", + detail: "Filter relationship '" + i + "' is a foreign reference and does not exist on " + request.resourceConfig.resource }); } } diff --git a/test/get-resource.js b/test/get-resource.js index ecca0138..2b373564 100644 --- a/test/get-resource.js +++ b/test/get-resource.js @@ -97,7 +97,10 @@ describe("Testing jsonapi-server", function() { }, function(err, res, json) { assert.equal(err, null); json = helpers.validateError(json); - assert.equal(res.statusCode, "403", "Expecting 403"); + assert.equal(res.statusCode, "403", "Expecting 403 FORBIDDEN"); + var error = json.errors[0]; + assert.equal(error.code, "EFORBIDDEN"); + assert.equal(error.title, "Invalid filter"); done(); }); }); @@ -584,7 +587,11 @@ describe("Testing jsonapi-server", function() { assert.equal(err, null); json = helpers.validateError(json); - assert.equal(res.statusCode, "403", "Expecting 403 EFORBIDDEN"); + assert.equal(res.statusCode, "403", "Expecting 403 FORBIDDEN"); + var error = json.errors[0]; + assert.equal(error.code, "EFORBIDDEN"); + assert.equal(error.title, "Invalid filter"); + assert(error.detail.match("do not have attribute or relationship")); done(); }); }); @@ -598,7 +605,11 @@ describe("Testing jsonapi-server", function() { assert.equal(err, null); json = helpers.validateError(json); - assert.equal(res.statusCode, "403", "Expecting 403 EFORBIDDEN"); + assert.equal(res.statusCode, "403", "Expecting 403 FORBIDDEN"); + var error = json.errors[0]; + assert.equal(error.code, "EFORBIDDEN"); + assert.equal(error.title, "Invalid filter"); + assert(error.detail.match("is a foreign reference and does not exist on")); done(); }); }); From 1407d6e5e5795aa694925490c6f728afa35b980e Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 12:21:23 +0100 Subject: [PATCH 3/9] Extract filter validations into helper functions. Extract individual filter validations into helper functions for more modular and readable code. --- lib/filter.js | 49 ++++++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/lib/filter.js b/lib/filter.js index 3337713c..79967f77 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -3,28 +3,39 @@ var filter = module.exports = { }; +filter._resourceDoesNotHaveProperty = function(resourceConfig, key) { + if (resourceConfig.attributes[key]) return null; + return { + status: "403", + code: "EFORBIDDEN", + title: "Invalid filter", + detail: resourceConfig.resource + " do not have attribute or relationship '" + key + "'" + }; +}; + +filter._relationshipIsForeign = function(resourceConfig, key) { + var relationSettings = resourceConfig.attributes[key]._settings; + if (!relationSettings || !relationSettings.__as) return null; + return { + status: "403", + code: "EFORBIDDEN", + title: "Invalid filter", + detail: "Filter relationship '" + key + "' is a foreign reference and does not exist on " + resourceConfig.resource + }; +}; + filter.validateParams = function(request, callback) { if (!request.params.filter) return callback(); - for (var i in request.params.filter) { - if (request.params.filter[i] instanceof Object) continue; - if (!request.resourceConfig.attributes[i]) { - return callback({ - status: "403", - code: "EFORBIDDEN", - title: "Invalid filter", - detail: request.resourceConfig.resource + " do not have attribute or relationship '" + i + "'" - }); - } - var relationSettings = request.resourceConfig.attributes[i]._settings; - if (relationSettings && relationSettings.__as) { - return callback({ - status: "403", - code: "EFORBIDDEN", - title: "Invalid filter", - detail: "Filter relationship '" + i + "' is a foreign reference and does not exist on " + request.resourceConfig.resource - }); - } + var error; + for (var key in request.params.filter) { + if (request.params.filter[key] instanceof Object) continue; + + error = filter._resourceDoesNotHaveProperty(request.resourceConfig, key); + if (error) return callback(error); + + error = filter._relationshipIsForeign(request.resourceConfig, key); + if (error) return callback(error); } return callback(); From acae19875c9b99c3861ad078f3894599095a7c12 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 13:00:35 +0100 Subject: [PATCH 4/9] Don't skip validating multiple values for a key. We only meant to skip validation for nested filters, but we were actually skipping array values (multiple values for a property) which should not be skipped. --- lib/filter.js | 4 +++- test/get-resource.js | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/filter.js b/lib/filter.js index 79967f77..7122c5ab 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -29,7 +29,9 @@ filter.validateParams = function(request, callback) { var error; for (var key in request.params.filter) { - if (request.params.filter[key] instanceof Object) continue; + var filterElement = request.params.filter[key]; + + if (!Array.isArray(filterElement) && filterElement instanceof Object) continue; error = filter._resourceDoesNotHaveProperty(request.resourceConfig, key); if (error) return callback(error); diff --git a/test/get-resource.js b/test/get-resource.js index 2b373564..44fc4584 100644 --- a/test/get-resource.js +++ b/test/get-resource.js @@ -105,6 +105,22 @@ describe("Testing jsonapi-server", function() { }); }); + it("unknown multiple attribute should error", function(done) { + var url = "http://localhost:16006/rest/articles?filter[foo]=bar&filter[foo]=baz"; + helpers.request({ + method: "GET", + url: url + }, function(err, res, json) { + assert.equal(err, null); + json = helpers.validateError(json); + assert.equal(res.statusCode, "403", "Expecting 403 FORBIDDEN"); + var error = json.errors[0]; + assert.equal(error.code, "EFORBIDDEN"); + assert.equal(error.title, "Invalid filter"); + done(); + }); + }); + it("equality for strings", function(done) { var url = "http://localhost:16006/rest/articles?filter[title]=How%20to%20AWS"; helpers.request({ From f1d09839a1e7b47c7a06783d293db82ada5bd184 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 13:02:27 +0100 Subject: [PATCH 5/9] Filters should always be valid in post-processing. Filters should always be valid when we get to the post-processing stage so remove the redundant check. --- lib/postProcessing/filter.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/postProcessing/filter.js b/lib/postProcessing/filter.js index 083ea084..8248939c 100644 --- a/lib/postProcessing/filter.js +++ b/lib/postProcessing/filter.js @@ -16,14 +16,6 @@ filter.action = function(request, response, callback) { var filters = { }; for (var i in allFilters) { - if (!request.resourceConfig.attributes[i]) { - return callback({ - status: "403", - code: "EFORBIDDEN", - title: "Invalid filter", - detail: request.resourceConfig.resource + " do not have property " + i - }); - } if (allFilters[i] instanceof Array) { allFilters[i] = allFilters[i].join(","); } From 8fdf375a10944a75394cab0446acf2f7f1df8016 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 23:31:51 +0100 Subject: [PATCH 6/9] Parse and Joi validate filters. Parse and validate using Joi the filter elements, so this work is only done in one place and the parsed filters can be provided to the handlers, avoid the need to redo that work there as is currently needed. --- lib/filter.js | 105 ++++++++++++++++++++++++++++++-- lib/postProcessing/filter.js | 56 ++++------------- lib/routes/find.js | 4 ++ test/get-resource-id-related.js | 2 +- test/get-resource.js | 17 ++++++ 5 files changed, 136 insertions(+), 48 deletions(-) diff --git a/lib/filter.js b/lib/filter.js index 7122c5ab..bc00f834 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -3,6 +3,10 @@ var filter = module.exports = { }; +var FILTER_OPERATORS = ["<", ">", "~", ":"]; +var STRING_ONLY_OPERATORS = ["~", ":"]; + + filter._resourceDoesNotHaveProperty = function(resourceConfig, key) { if (resourceConfig.attributes[key]) return null; return { @@ -24,21 +28,114 @@ filter._relationshipIsForeign = function(resourceConfig, key) { }; }; +filter._splitElement = function(element) { + if (!element) return null; + if (FILTER_OPERATORS.indexOf(element[0]) !== -1) { + return { operator: element[0], value: element.substring(1) }; + } + return { operator: null, value: element }; +}; + +filter._stringOnlyOperator = function(operator, attributeConfig) { + if (!operator || !attributeConfig) return null; + if (STRING_ONLY_OPERATORS.indexOf(operator) !== -1 && attributeConfig._type !== "string") { + return "operator " + operator + " can only be applied to string attributes"; + } + return null; +}; + +filter._parseScalarFilterElement = function(attributeConfig, scalarElement) { + if (!scalarElement) return { error: "invalid or empty filter element" }; + + var splitElement = filter._splitElement(scalarElement); + if (!splitElement) return { error: "empty filter" }; + + var error = filter._stringOnlyOperator(splitElement.operator, attributeConfig); + if (error) return { error: error }; + + if (attributeConfig._settings) { // relationship attribute: no further validation + return { result: splitElement }; + } + + var validateResult = attributeConfig.validate(splitElement.value); + if (validateResult.error) { + return { error: validateResult.error.message }; + } + + var validatedElement = { operator: splitElement.operator, value: validateResult.value }; + return { result: validatedElement }; +}; + +filter._parseFilterElementHelper = function(attributeConfig, filterElement) { + if (!filterElement) return { error: "invalid or empty filter element" }; + + var parsedElements = [].concat(filterElement).map(function(scalarElement) { + return filter._parseScalarFilterElement(attributeConfig, scalarElement); + }); + + if (parsedElements.length === 1) return parsedElements[0]; + + var errors = parsedElements.reduce(function(combined, element) { + if (!combined) { + if (!element.error) return combined; + return [ element.error ]; + } + return combined.concat(element.error); + }, null); + + if (errors) return { error: errors }; + + var results = parsedElements.map(function(element) { + return element.result; + }); + + return { result: results }; +}; + +filter._parseFilterElement = function(attributeName, attributeConfig, filterElement) { + var helperResult = filter._parseFilterElementHelper(attributeConfig, filterElement); + + if (helperResult.error) { + return { + error: { + status: "403", + code: "EFORBIDDEN", + title: "Invalid filter", + detail: "Filter value for key '" + attributeName + "' is invalid: " + helperResult.error + } + }; + } + return { result: helperResult.result }; +}; + filter.validateParams = function(request, callback) { if (!request.params.filter) return callback(); + var resourceConfig = request.resourceConfig; + + var processedFilter = { }; var error; + var filterElement; + var parsedFilterElement; + for (var key in request.params.filter) { - var filterElement = request.params.filter[key]; + filterElement = request.params.filter[key]; - if (!Array.isArray(filterElement) && filterElement instanceof Object) continue; + if (!Array.isArray(filterElement) && filterElement instanceof Object) continue; // skip deep filters - error = filter._resourceDoesNotHaveProperty(request.resourceConfig, key); + error = filter._resourceDoesNotHaveProperty(resourceConfig, key); if (error) return callback(error); - error = filter._relationshipIsForeign(request.resourceConfig, key); + error = filter._relationshipIsForeign(resourceConfig, key); if (error) return callback(error); + + parsedFilterElement = filter._parseFilterElement(key, resourceConfig.attributes[key], filterElement); + if (parsedFilterElement.error) return callback(parsedFilterElement.error); + + processedFilter[key] = [].concat(parsedFilterElement.result); } + request.processedFilter = processedFilter; + return callback(); }; diff --git a/lib/postProcessing/filter.js b/lib/postProcessing/filter.js index 8248939c..5f6b03fc 100644 --- a/lib/postProcessing/filter.js +++ b/lib/postProcessing/filter.js @@ -8,32 +8,20 @@ var _ = { }; var debug = require("../debugging.js"); -var FILTER_OPERATORS = ["<", ">", "~", ":"]; - filter.action = function(request, response, callback) { - var allFilters = _.assign({ }, request.params.filter); - if (!allFilters) return callback(); - - var filters = { }; - for (var i in allFilters) { - if (allFilters[i] instanceof Array) { - allFilters[i] = allFilters[i].join(","); - } - if (typeof allFilters[i] === "string") { - filters[i] = allFilters[i]; - } - } + var filters = request.processedFilter; + if (!filters) return callback(); if (response.data instanceof Array) { for (var j = 0; j < response.data.length; j++) { - if (!filter._filterKeepObject(response.data[j], filters, request.resourceConfig.attributes)) { + if (!filter._filterKeepObject(response.data[j], filters)) { debug.filter("removed", filters, JSON.stringify(response.data[j].attributes)); response.data.splice(j, 1); j--; } } } else if (response.data instanceof Object) { - if (!filter._filterKeepObject(response.data, filters, request.resourceConfig.attributes)) { + if (!filter._filterKeepObject(response.data, filters)) { debug.filter("removed", filters, JSON.stringify(response.data.attributes)); response.data = null; } @@ -42,27 +30,10 @@ filter.action = function(request, response, callback) { return callback(); }; -filter._splitFilterElement = function(filterElementStr) { - if (FILTER_OPERATORS.indexOf(filterElementStr[0]) !== -1) { - return { operator: filterElementStr[0], value: filterElementStr.substring(1) }; - } - return { operator: null, value: filterElementStr }; -}; - -filter._filterMatches = function(filterElementStr, attributeValue, attributeConfig) { - var filterElement = filter._splitFilterElement(filterElementStr); - var validationResult = attributeConfig.validate(filterElement.value); - if (validationResult.error) { - debug.filter("invalid filter condition value:", validationResult.error); - return false; - } - filterElement.value = validationResult.value; +filter._filterMatches = function(filterElement, attributeValue) { if (!filterElement.operator) { return _.isEqual(attributeValue, filterElement.value); } - if (["~", ":"].indexOf(filterElement.operator) !== -1 && typeof filterElement.value !== "string") { - return false; - } var filterFunction = { ">": function filterGreaterThan(attrValue, filterValue) { return attrValue > filterValue; @@ -81,15 +52,14 @@ filter._filterMatches = function(filterElementStr, attributeValue, attributeConf return result; }; -filter._filterKeepObject = function(someObject, filters, attributesConfig) { +filter._filterKeepObject = function(someObject, filters) { for (var filterName in filters) { - var whitelist = filters[filterName].split(","); - var attributeConfig = attributesConfig[filterName]; + var whitelist = filters[filterName]; if (someObject.attributes.hasOwnProperty(filterName) || (filterName === "id")) { var attributeValue = someObject.attributes[filterName]; if (filterName === "id") attributeValue = someObject.id; - var attributeMatches = filter._attributesMatchesOR(attributeValue, attributeConfig, whitelist); + var attributeMatches = filter._attributesMatchesOR(attributeValue, whitelist); if (!attributeMatches) return false; } else if (someObject.relationships.hasOwnProperty(filterName)) { var relationships = someObject.relationships[filterName]; @@ -102,10 +72,10 @@ filter._filterKeepObject = function(someObject, filters, attributesConfig) { return true; }; -filter._attributesMatchesOR = function(attributeValue, attributeConfig, whitelist) { +filter._attributesMatchesOR = function(attributeValue, whitelist) { var matchOR = false; - whitelist.forEach(function(filterElementStr) { - if (filter._filterMatches(filterElementStr, attributeValue, attributeConfig)) { + whitelist.forEach(function(filterElement) { + if (filter._filterMatches(filterElement, attributeValue)) { matchOR = true; } }); @@ -123,8 +93,8 @@ filter._relationshipMatchesOR = function(relationships, whitelist) { return relation.id; }); - whitelist.forEach(function(filterElementStr) { - if (data.indexOf(filterElementStr) !== -1) { + whitelist.forEach(function(filterElement) { + if (data.indexOf(filterElement.value) !== -1) { matchOR = true; } }); diff --git a/lib/routes/find.js b/lib/routes/find.js index 14e6e972..218fd559 100644 --- a/lib/routes/find.js +++ b/lib/routes/find.js @@ -5,6 +5,7 @@ var findRoute = module.exports = { }; var async = require("async"); var helper = require("./helper.js"); var router = require("../router.js"); +var filter = require("../filter.js"); var postProcess = require("../postProcess.js"); var responseHelper = require("../responseHelper.js"); @@ -21,6 +22,9 @@ findRoute.register = function() { function(callback) { helper.verifyRequest(request, resourceConfig, res, "find", callback); }, + function validateFilterParams(callback) { + filter.validateParams(request, callback); + }, function(callback) { resourceConfig.handlers.find(request, callback); }, diff --git a/test/get-resource-id-related.js b/test/get-resource-id-related.js index b4d0d3ec..1f531f91 100644 --- a/test/get-resource-id-related.js +++ b/test/get-resource-id-related.js @@ -111,7 +111,7 @@ describe("Testing jsonapi-server", function() { json = helpers.validateJson(json); assert.equal(res.statusCode, "200", "Expecting 200 OK"); - assert.deepEqual(json.data, null); + assert(!json.data); done(); }); diff --git a/test/get-resource.js b/test/get-resource.js index 44fc4584..27ebf744 100644 --- a/test/get-resource.js +++ b/test/get-resource.js @@ -121,6 +121,23 @@ describe("Testing jsonapi-server", function() { }); }); + it("value of wrong type should error", function(done) { + var url = "http://localhost:16006/rest/photos?filter[raw]=bob"; + helpers.request({ + method: "GET", + url: url + }, function(err, res, json) { + assert.equal(err, null); + json = helpers.validateError(json); + assert.equal(res.statusCode, "403", "Expecting 403 FORBIDDEN"); + var error = json.errors[0]; + assert.equal(error.code, "EFORBIDDEN"); + assert.equal(error.title, "Invalid filter"); + assert(error.detail.match("Filter value for key '.*?' is invalid")); + done(); + }); + }); + it("equality for strings", function(done) { var url = "http://localhost:16006/rest/articles?filter[title]=How%20to%20AWS"; helpers.request({ From a948a337386d82e55aa682fb88a528fe9aa379b3 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Fri, 27 May 2016 00:41:25 +0100 Subject: [PATCH 7/9] Pass query string to related resources. The test was passing previously by accident. Once the filter is corrected to include a valid email address, it breaks because the filter isn't being passed to the related resource rerouted request. Fix the test and pass the query string (including the filter) to related resources. --- lib/postProcess.js | 6 +++++- test/get-resource-id-related.js | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/postProcess.js b/lib/postProcess.js index 1c9a675c..c6e050f4 100644 --- a/lib/postProcess.js +++ b/lib/postProcess.js @@ -49,7 +49,11 @@ postProcess._fetchRelatedResources = function(request, mainResource, callback) { var ids = resourcesToFetch[type]; var urlJoiner = "&filter[id]="; ids = urlJoiner + ids.join(urlJoiner); - return jsonApi._apiConfig.pathPrefix + type + "/?" + ids; + var uri = jsonApi._apiConfig.pathPrefix + type + "/?" + ids; + if (request.route.query) { + uri += "&" + request.route.query; + } + return uri; }); async.map(resourcesToFetch, function(related, done) { diff --git a/test/get-resource-id-related.js b/test/get-resource-id-related.js index 1f531f91..ae0ff0ed 100644 --- a/test/get-resource-id-related.js +++ b/test/get-resource-id-related.js @@ -102,7 +102,7 @@ describe("Testing jsonapi-server", function() { }); it("with filter", function(done) { - var url = "http://localhost:16006/rest/articles/de305d54-75b4-431b-adb2-eb6b9e546014/author?filter[email]=email"; + var url = "http://localhost:16006/rest/articles/de305d54-75b4-431b-adb2-eb6b9e546014/author?filter[email]=email@example.com"; helpers.request({ method: "GET", url: url From 74710d8123d7f7dda493a762de0254cf082a4101 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Fri, 27 May 2016 01:08:04 +0100 Subject: [PATCH 8/9] Rename `filter.validate` to `parseAndValidate`. The more generic nature of its functionality warrants the rename. --- lib/filter.js | 2 +- lib/routes/find.js | 4 ++-- lib/routes/search.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/filter.js b/lib/filter.js index bc00f834..2c18b193 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -108,7 +108,7 @@ filter._parseFilterElement = function(attributeName, attributeConfig, filterElem return { result: helperResult.result }; }; -filter.validateParams = function(request, callback) { +filter.parseAndValidate = function(request, callback) { if (!request.params.filter) return callback(); var resourceConfig = request.resourceConfig; diff --git a/lib/routes/find.js b/lib/routes/find.js index 218fd559..606ea621 100644 --- a/lib/routes/find.js +++ b/lib/routes/find.js @@ -22,8 +22,8 @@ findRoute.register = function() { function(callback) { helper.verifyRequest(request, resourceConfig, res, "find", callback); }, - function validateFilterParams(callback) { - filter.validateParams(request, callback); + function parseAndValidateFilter(callback) { + filter.parseAndValidate(request, callback); }, function(callback) { resourceConfig.handlers.find(request, callback); diff --git a/lib/routes/search.js b/lib/routes/search.js index 0e10e5de..d7fb7194 100644 --- a/lib/routes/search.js +++ b/lib/routes/search.js @@ -27,8 +27,8 @@ searchRoute.register = function() { function(callback) { helper.validate(request.params, resourceConfig.searchParams, callback); }, - function validateFilterParams(callback) { - filter.validateParams(request, callback); + function parseAndValidateFilter(callback) { + filter.parseAndValidate(request, callback); }, function validatePaginationParams(callback) { pagination.validatePaginationParams(request); From 43578718779f9dee1b87b09caef6e6755e6eae91 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Fri, 27 May 2016 09:54:17 +0100 Subject: [PATCH 9/9] `filter.parseAndvalidate` is actually synchronous. --- lib/filter.js | 12 ++++++------ lib/routes/find.js | 2 +- lib/routes/search.js | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/filter.js b/lib/filter.js index 2c18b193..fe5eeb60 100644 --- a/lib/filter.js +++ b/lib/filter.js @@ -108,8 +108,8 @@ filter._parseFilterElement = function(attributeName, attributeConfig, filterElem return { result: helperResult.result }; }; -filter.parseAndValidate = function(request, callback) { - if (!request.params.filter) return callback(); +filter.parseAndValidate = function(request) { + if (!request.params.filter) return null; var resourceConfig = request.resourceConfig; @@ -124,18 +124,18 @@ filter.parseAndValidate = function(request, callback) { if (!Array.isArray(filterElement) && filterElement instanceof Object) continue; // skip deep filters error = filter._resourceDoesNotHaveProperty(resourceConfig, key); - if (error) return callback(error); + if (error) return error; error = filter._relationshipIsForeign(resourceConfig, key); - if (error) return callback(error); + if (error) return error; parsedFilterElement = filter._parseFilterElement(key, resourceConfig.attributes[key], filterElement); - if (parsedFilterElement.error) return callback(parsedFilterElement.error); + if (parsedFilterElement.error) return parsedFilterElement.error; processedFilter[key] = [].concat(parsedFilterElement.result); } request.processedFilter = processedFilter; - return callback(); + return null; }; diff --git a/lib/routes/find.js b/lib/routes/find.js index 606ea621..a66c4ed9 100644 --- a/lib/routes/find.js +++ b/lib/routes/find.js @@ -23,7 +23,7 @@ findRoute.register = function() { helper.verifyRequest(request, resourceConfig, res, "find", callback); }, function parseAndValidateFilter(callback) { - filter.parseAndValidate(request, callback); + return callback(filter.parseAndValidate(request)); }, function(callback) { resourceConfig.handlers.find(request, callback); diff --git a/lib/routes/search.js b/lib/routes/search.js index d7fb7194..1d18d3b5 100644 --- a/lib/routes/search.js +++ b/lib/routes/search.js @@ -28,7 +28,7 @@ searchRoute.register = function() { helper.validate(request.params, resourceConfig.searchParams, callback); }, function parseAndValidateFilter(callback) { - filter.parseAndValidate(request, callback); + return callback(filter.parseAndValidate(request)); }, function validatePaginationParams(callback) { pagination.validatePaginationParams(request);