From 7a73dfd9bc7b089b5c03537500a4765f86460124 Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Thu, 11 Oct 2018 23:05:49 +0200 Subject: [PATCH] Extended all shared validator refs #9866 - there was a missing step in the shared validator - we have to differentiate between data validation for browse/read and data validation for add/edit - furthermore, the data validation for add/edit was missing and was not copied over from v0.1 (check structure of incoming body) - adds the ability to require properties from req.body.docName[0] --- core/server/api/shared/validators/handle.js | 8 +- .../server/api/shared/validators/input/all.js | 74 ++++++++--- .../unit/api/shared/validators/handle_spec.js | 5 +- .../api/shared/validators/input/all_spec.js | 120 ++++++++++++++++-- 4 files changed, 177 insertions(+), 30 deletions(-) diff --git a/core/server/api/shared/validators/handle.js b/core/server/api/shared/validators/handle.js index 1d86963abb..a21a3fe960 100644 --- a/core/server/api/shared/validators/handle.js +++ b/core/server/api/shared/validators/handle.js @@ -26,9 +26,15 @@ module.exports.input = (apiConfig, apiValidators, frame) => { // ##### SHARED ALL VALIDATION tasks.push(function allShared() { - return sharedValidators.all(apiConfig, frame); + return sharedValidators.all.all(apiConfig, frame); }); + if (sharedValidators.all[apiConfig.method]) { + tasks.push(function allShared() { + return sharedValidators.all[apiConfig.method](apiConfig, frame); + }); + } + // ##### API VERSION VALIDATION if (apiValidators.all) { diff --git a/core/server/api/shared/validators/input/all.js b/core/server/api/shared/validators/input/all.js index b5921a512f..fd0153b732 100644 --- a/core/server/api/shared/validators/input/all.js +++ b/core/server/api/shared/validators/input/all.js @@ -62,22 +62,66 @@ const validate = (config, attrs) => { return errors; }; -module.exports = function validateAll(apiConfig, frame) { - debug('validate all'); +module.exports = { + all(apiConfig, frame) { + debug('validate all'); - let validationErrors = validate(apiConfig.options, frame.options); + let validationErrors = validate(apiConfig.options, frame.options); - if (!_.isEmpty(validationErrors)) { - return Promise.reject(validationErrors[0]); + if (!_.isEmpty(validationErrors)) { + return Promise.reject(validationErrors[0]); + } + + return Promise.resolve(); + }, + + browse(apiConfig, frame) { + debug('validate browse'); + + let validationErrors = []; + + if (frame.data) { + validationErrors = validate(apiConfig.data, frame.data); + } + + if (!_.isEmpty(validationErrors)) { + return Promise.reject(validationErrors[0]); + } + }, + + read() { + debug('validate read'); + this.browse(...arguments); + }, + + add(apiConfig, frame) { + debug('validate add'); + + if (_.isEmpty(frame.data) || _.isEmpty(frame.data[apiConfig.docName]) || _.isEmpty(frame.data[apiConfig.docName][0])) { + return Promise.reject(new common.errors.BadRequestError({ + message: common.i18n.t('errors.api.utils.noRootKeyProvided', {docName: apiConfig.docName}) + })); + } + + const jsonpath = require('jsonpath'); + + if (apiConfig.data) { + const missedDataProperties = _.filter(apiConfig.data, (value, key) => { + return jsonpath.query(frame.data[apiConfig.docName][0], key).length === 0; + }); + + if (missedDataProperties.length) { + return Promise.reject(new common.errors.ValidationError({ + message: common.i18n.t('notices.data.validation.index.validationFailed', { + validationName: 'FieldIsRequired' + }) + })); + } + } + }, + + edit() { + debug('validate edit'); + this.add(...arguments); } - - if (frame.data) { - validationErrors = validate(apiConfig.data, frame.data); - } - - if (!_.isEmpty(validationErrors)) { - return Promise.reject(validationErrors[0]); - } - - return Promise.resolve(); }; diff --git a/core/test/unit/api/shared/validators/handle_spec.js b/core/test/unit/api/shared/validators/handle_spec.js index 20a2354b85..dd5ed9ffe0 100644 --- a/core/test/unit/api/shared/validators/handle_spec.js +++ b/core/test/unit/api/shared/validators/handle_spec.js @@ -29,7 +29,9 @@ describe('Unit: api/shared/validators/handle', function () { it('ensure validators are called', function () { const getStub = sandbox.stub(); - sandbox.stub(shared.validators.input, 'all').get(() => {return getStub;}); + const addStub = sandbox.stub(); + sandbox.stub(shared.validators.input.all, 'all').get(() => {return getStub;}); + sandbox.stub(shared.validators.input.all, 'add').get(() => {return addStub;}); const apiValidators = { all: { @@ -46,6 +48,7 @@ describe('Unit: api/shared/validators/handle', function () { return shared.validators.handle.input({docName: 'posts', method: 'add'}, apiValidators, {context: {}}) .then(() => { getStub.calledOnce.should.be.true(); + addStub.calledOnce.should.be.true(); apiValidators.all.add.calledOnce.should.be.true(); apiValidators.posts.add.calledOnce.should.be.true(); apiValidators.users.add.called.should.be.false(); diff --git a/core/test/unit/api/shared/validators/input/all_spec.js b/core/test/unit/api/shared/validators/input/all_spec.js index 1556230db8..ff10c6ed59 100644 --- a/core/test/unit/api/shared/validators/input/all_spec.js +++ b/core/test/unit/api/shared/validators/input/all_spec.js @@ -9,7 +9,7 @@ describe('Unit: api/shared/validators/input/all', function () { sandbox.restore(); }); - describe('validate options', function () { + describe('all', function () { it('default', function () { const frame = { options: { @@ -29,7 +29,7 @@ describe('Unit: api/shared/validators/input/all', function () { } }; - return shared.validators.input.all(apiConfig, frame) + return shared.validators.input.all.all(apiConfig, frame) .then(() => { should.exist(frame.options.page); should.exist(frame.options.slug); @@ -54,7 +54,7 @@ describe('Unit: api/shared/validators/input/all', function () { } }; - return shared.validators.input.all(apiConfig, frame) + return shared.validators.input.all.all(apiConfig, frame) .then(() => { should.exist(frame.options.page); should.exist(frame.options.slug); @@ -79,7 +79,7 @@ describe('Unit: api/shared/validators/input/all', function () { } }; - return shared.validators.input.all(apiConfig, frame) + return shared.validators.input.all.all(apiConfig, frame) .then(Promise.reject) .catch((err) => { should.exist(err); @@ -100,7 +100,7 @@ describe('Unit: api/shared/validators/input/all', function () { } }; - return shared.validators.input.all(apiConfig, frame) + return shared.validators.input.all.all(apiConfig, frame) .then(Promise.reject) .catch((err) => { should.exist(err); @@ -122,7 +122,7 @@ describe('Unit: api/shared/validators/input/all', function () { } }; - return shared.validators.input.all(apiConfig, frame) + return shared.validators.input.all.all(apiConfig, frame) .then(Promise.reject) .catch((err) => { should.exist(err); @@ -130,7 +130,7 @@ describe('Unit: api/shared/validators/input/all', function () { }); }); - describe('validate data', function () { + describe('browse', function () { it('default', function () { const frame = { options: { @@ -143,11 +143,9 @@ describe('Unit: api/shared/validators/input/all', function () { const apiConfig = {}; - return shared.validators.input.all(apiConfig, frame) - .then(() => { - should.exist(frame.options.context); - should.exist(frame.data.status); - }); + shared.validators.input.all.browse(apiConfig, frame); + should.exist(frame.options.context); + should.exist(frame.data.status); }); it('fails', function () { @@ -162,10 +160,106 @@ describe('Unit: api/shared/validators/input/all', function () { const apiConfig = {}; - return shared.validators.input.all(apiConfig, frame) + return shared.validators.input.all.browse(apiConfig, frame) + .then(Promise.reject) .catch((err) => { should.exist(err); }); }); }); + + describe('read', function () { + it('default', function () { + sandbox.stub(shared.validators.input.all, 'browse'); + + const frame = { + options: { + context: {} + } + }; + + const apiConfig = {}; + + shared.validators.input.all.read(apiConfig, frame); + shared.validators.input.all.browse.calledOnce.should.be.true(); + }); + }); + + describe('add', function () { + it('fails', function () { + const frame = { + data: {} + }; + + const apiConfig = { + docName: 'docName' + }; + + return shared.validators.input.all.add(apiConfig, frame) + .then(Promise.reject) + .catch((err) => { + should.exist(err); + }); + }); + + it('fails', function () { + const frame = { + data: { + docName: true + } + }; + + const apiConfig = { + docName: 'docName' + }; + + return shared.validators.input.all.add(apiConfig, frame) + .then(Promise.reject) + .catch((err) => { + should.exist(err); + }); + }); + + it('fails', function () { + const frame = { + data: { + docName: [{ + a: 'b' + }] + } + }; + + const apiConfig = { + docName: 'docName', + data: { + b: { + required: true + } + } + }; + + return shared.validators.input.all.add(apiConfig, frame) + .then(Promise.reject) + .catch((err) => { + should.exist(err); + }); + }); + + it('success', function () { + const frame = { + data: { + docName: [{ + a: 'b' + }] + } + }; + + const apiConfig = { + docName: 'docName' + }; + + const result = shared.validators.input.all.add(apiConfig, frame); + (result instanceof Promise).should.not.be.true(); + }); + }); });