From 8f591e79b3423f84ff14779f5f1e3db96c4c2915 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 10:54:14 +0100 Subject: [PATCH 01/10] 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 dd221d52f6631c3fa0ae0e3b2ec4f05883c4c3a5 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 11:54:50 +0100 Subject: [PATCH 02/10] 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 ce3dcfdbe7a7551048c989613fe86f2621649522 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 12:21:23 +0100 Subject: [PATCH 03/10] 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 c762222dea4340fdfe58ab550e0450536cfdcc1d Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 13:00:35 +0100 Subject: [PATCH 04/10] 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 f32b15c4919a5f15f3b0fbbeaf11f8935dd37c7a Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 13:02:27 +0100 Subject: [PATCH 05/10] 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 5edfd844be951fcc90602ea5f4d94b6fdd670437 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Sun, 22 May 2016 23:31:51 +0100 Subject: [PATCH 06/10] 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 39218d659e20683a26c8e77d58a4c06588becc74 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Fri, 27 May 2016 00:41:25 +0100 Subject: [PATCH 07/10] 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 84f30599a1d5f39a15f100c57282572c5747778f Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Fri, 27 May 2016 01:08:04 +0100 Subject: [PATCH 08/10] 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 e010ac82cf186ebbde5d530b42fd607d3f85ca25 Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Fri, 27 May 2016 09:54:17 +0100 Subject: [PATCH 09/10] `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); From 076d6ffab66cb6eb62133611561f7d75958c2eed Mon Sep 17 00:00:00 2001 From: Pedro Romano Date: Fri, 27 May 2016 14:16:25 +0100 Subject: [PATCH 10/10] Update changelog and bump version. --- CHANGELOG.md | 2 ++ package.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 804a5c2e..75dd99c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +- 2016-05-27 - v1.9.0 +- 2016-05-27 - Make parsed and validated filter available in request for handlers - 2016-05-24 - v1.8.0 - 2016-05-24 - HTTPS support - 2016-05-24 - v1.7.0 diff --git a/package.json b/package.json index 9d791eba..01bdf0e3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "jsonapi-server", - "version": "1.8.0", + "version": "1.9.0", "description": "A config driven NodeJS framework implementing json:api", "keywords": [ "jsonapi",