From 50a7f72d651b4ee2091f34203b3271da256eb54c Mon Sep 17 00:00:00 2001 From: Adam Koniar Date: Mon, 19 Mar 2018 18:35:07 +0100 Subject: [PATCH] chore(test): Unification of error messages for better end to end testing. --- app.js | 8 ++ lib/routes/fruits.js | 16 +-- lib/validations/index.js | 45 +++----- public/index.html | 6 +- test/fruits-test.js | 226 ++++++++++++++++++++++++++++++++++++--- 5 files changed, 244 insertions(+), 57 deletions(-) diff --git a/app.js b/app.js index 3c924a33..6199ae01 100644 --- a/app.js +++ b/app.js @@ -30,6 +30,14 @@ const db = require('./lib/db'); const fruits = require('./lib/routes/fruits'); app.use(bodyParser.json()); +app.use(function (error, req, res, next) { + if (req.body === '' || (error instanceof SyntaxError && error.type === 'entity.parse.failed')) { + res.status(415); + return res.send('Invalid payload!'); + } else { + next(); + } +}); app.use(bodyParser.urlencoded({ extended: false })); app.use(express.static(path.join(__dirname, 'public'))); // expose the license.html at http[s]://[host]:[port]/licences/licenses.html diff --git a/lib/routes/fruits.js b/lib/routes/fruits.js index 935b61f3..0f727476 100644 --- a/lib/routes/fruits.js +++ b/lib/routes/fruits.js @@ -7,7 +7,7 @@ const validations = require('../validations'); const fruits = require('../api/fruits'); router.get('/fruits/:id', (request, response) => { - const {id} = request.params; + const { id } = request.params; fruits.find(id).then(result => { if (result.rowCount === 0) { response.status(404); @@ -27,8 +27,8 @@ router.get('/fruits', (request, response) => { }); }); -router.post('/fruits', validations.validateCreate, (request, response) => { - const {name, stock} = request.body; +router.post('/fruits', validations.validateCreateUpdateRequest, (request, response) => { + const { name, stock } = request.body; return fruits.create(name, stock).then((result) => { response.status(201); return response.send(result.rows[0]); @@ -38,10 +38,10 @@ router.post('/fruits', validations.validateCreate, (request, response) => { }); }); -router.put('/fruits/:id', validations.validateUpdate, (request, response) => { - const {name, stock} = request.body; - const {id} = request.params; - fruits.update({name, stock, id}).then((result) => { +router.put('/fruits/:id', validations.validateCreateUpdateRequest, (request, response) => { + const { name, stock } = request.body; + const { id } = request.params; + fruits.update({ name, stock, id }).then((result) => { if (result.rowCount === 0) { response.status(404); return response.send(`Unknown item ${id}`); @@ -54,7 +54,7 @@ router.put('/fruits/:id', validations.validateUpdate, (request, response) => { }); router.delete('/fruits/:id', (request, response) => { - const {id} = request.params; + const { id } = request.params; fruits.remove(id).then((result) => { if (result.rowCount === 0) { response.status(404); diff --git a/lib/validations/index.js b/lib/validations/index.js index b84333a8..bf7a733b 100644 --- a/lib/validations/index.js +++ b/lib/validations/index.js @@ -1,50 +1,31 @@ 'use strict'; -function validateCreate (request, response, next) { - // No need to check for no body, express will make body an empty object - const {name, stock, id} = request.body; - - if (!name) { - response.status(400); - return response.send('The name must not be null'); +function validateCreateUpdateRequest(request, response, next) { + if(Object.keys(request.body).length === 0){ + response.status(415); + return response.send('Invalid payload!'); } - - if (!stock) { - response.status(400); - return response.send('The stock must not be greater or equal to 0'); - } - - if (id) { - response.status(400); - return response.send('The created item already contains an id'); - } - - next(); -} - -function validateUpdate (request, response, next) { // No need to check for no body, express will make body an empty object - const {name, stock, id} = request.body; + const { name, stock, id } = request.body; if (!name) { - response.status(400); - return response.send('The name must not be null'); + response.status(422); + return response.send('The name is required!'); } - if (!stock) { - response.status(400); - return response.send('The stock must not be greater or equal to 0'); + if (stock == null || isNaN(stock) || stock < 0) { + response.status(422); + return response.send('The stock must be greater or equal to 0!'); } if (id && id !== request.params.id) { - response.status(400); - return response.send('The id cannot be changed'); + response.status(422); + return response.send('Id was invalidly set on request.'); } next(); } module.exports = { - validateCreate, - validateUpdate + validateCreateUpdateRequest }; diff --git a/public/index.html b/public/index.html index 91523022..dea8849b 100644 --- a/public/index.html +++ b/public/index.html @@ -81,7 +81,7 @@ //In case of edit fruits, populate form with fruit data $scope.edit = function (fruit) { $scope.form.name = fruit.name; - $scope.form.stock = fruit.stock; + $scope.form.stock = parseInt(fruit.stock); $scope.form.id = fruit.id; }; @@ -104,7 +104,7 @@ } function _error(response) { - alert(response.data.error || response.statusText); + alert(response.data || response.statusText); } //Clear the form @@ -131,7 +131,7 @@

Add/Edit a fruit

-
+
diff --git a/test/fruits-test.js b/test/fruits-test.js index d1664be9..c273f2bf 100644 --- a/test/fruits-test.js +++ b/test/fruits-test.js @@ -169,30 +169,39 @@ test('test POST fruit', (t) => { }); test('test POST fruit - error - no name', (t) => { + const fruitData = { + stock: 10 + }; + const app = proxyquire('../app', { './lib/db': mockDb }); supertest(app) .post('/api/fruits') - .expect(400) + .send(fruitData) + .expect(422) .then(response => { - t.equal(response.text, 'The name must not be null', 'has a need name message'); + t.equal(response.text, 'The name is required!', 'has a need name message'); t.end(); }); }); test('test POST fruit - error - no stock', (t) => { + const fruitData = { + name: 'Banana' + }; + const app = proxyquire('../app', { './lib/db': mockDb }); supertest(app) .post('/api/fruits') - .send({name: 'Banana'}) - .expect(400) + .send(fruitData) + .expect(422) .then(response => { - t.equal(response.text, 'The stock must not be greater or equal to 0', 'has a need stock message'); + t.equal(response.text, 'The stock must be greater or equal to 0!', 'has a need stock message'); t.end(); }); }); @@ -205,9 +214,9 @@ test('test POST fruit - error - id error', (t) => { supertest(app) .post('/api/fruits') .send({name: 'Banana', stock: 10, id: 22}) - .expect(400) + .expect(422) .then(response => { - t.equal(response.text, 'The created item already contains an id', 'has an id error message'); + t.equal(response.text, 'Id was invalidly set on request.', 'has an id error message'); t.end(); }); }); @@ -243,6 +252,108 @@ test('test POST fruit - error', (t) => { }); }); +test('test POST fruit - error - no payload', (t) => { + const app = proxyquire('../app', { + './lib/db': mockDb + }); + + supertest(app) + .post('/api/fruits') + .expect(415) + .then(response => { + t.equal(response.text, 'Invalid payload!', 'Payload must be set'); + t.end(); + }); +}); + +test('test POST fruit - error - invalid payload', (t) => { + const app = proxyquire('../app', { + './lib/db': mockDb + }); + + supertest(app) + .post('/api/fruits') + .set('Content-Type', 'application/json') + .send('Some text') + .expect(415) + .then(response => { + t.equal(response.text, 'Invalid payload!', 'Payload must be in JSON format'); + t.end(); + }); +}); + +test('test POST fruit - error - xml payload', (t) => { + const app = proxyquire('../app', { + './lib/db': mockDb + }); + const xmlFruitData = 'Banana10'; + supertest(app) + .post('/api/fruits') + .set('Content-Type', 'application/xml') + .send(xmlFruitData) + .expect(415) + .then(response => { + t.equal(response.text, 'Invalid payload!', 'Payload must be in JSON format'); + t.end(); + }); +}); + +test('test POST fruit - error - JSON Content-Type and XML body', (t) => { + const app = proxyquire('../app', { + './lib/db': mockDb + }); + const xmlFruitData = 'adam10'; + supertest(app) + .post('/api/fruits') + .set('Content-Type', 'application/json') + .send(xmlFruitData) + .expect(415) + .then(response => { + t.equal(response.text, 'Invalid payload!', 'Payload must be in JSON format'); + t.end(); + }); +}); + +test('test POST fruit - error - negative number of stock', (t) => { + const fruitData = { + name: 'Banana', + stock: -10 + }; + + const app = proxyquire('../app', { + './lib/db': mockDb + }); + + supertest(app) + .post('/api/fruits') + .send(fruitData) + .expect(422) + .then(response => { + t.equal(response.text, 'The stock must be greater or equal to 0!', 'has a need stock message'); + t.end(); + }); +}); + +test('test POST fruit - error - no numeric stock', (t) => { + const fruitData = { + name: 'Banana', + stock: 'two' + }; + + const app = proxyquire('../app', { + './lib/db': mockDb + }); + + supertest(app) + .post('/api/fruits') + .send(fruitData) + .expect(422) + .then(response => { + t.equal(response.text, 'The stock must be greater or equal to 0!', 'has a need stock message'); + t.end(); + }); +}); + test('test PUT fruit', (t) => { const fruitData = { name: 'Banana', @@ -254,7 +365,7 @@ test('test PUT fruit', (t) => { update: (options) => { t.equal(options.name, fruitData.name, `respone.body.name should be ${fruitData.name}`); t.equal(options.stock, fruitData.stock, `respone.body.stock should be ${fruitData.stock}`); - t.equal(options.id, fruitData.id, `respone.body.stock should be ${fruitData.stock}`); + t.equal(options.id, fruitData.id, `respone.body.id should be ${fruitData.stock}`); return Promise.resolve({rowCount: 1}); } }; @@ -279,15 +390,20 @@ test('test PUT fruit', (t) => { }); test('test PUT fruit - error - no name', (t) => { + const fruitData = { + stock: 10 + }; + const app = proxyquire('../app', { './lib/db': mockDb }); supertest(app) .put('/api/fruits/20') - .expect(400) + .expect(422) + .send(fruitData) .then(response => { - t.equal(response.text, 'The name must not be null', 'has a need name message'); + t.equal(response.text, 'The name is required!', 'has a need name message'); t.end(); }); }); @@ -300,9 +416,9 @@ test('test PUT fruit - error - no stock', (t) => { supertest(app) .put('/api/fruits/20') .send({name: 'name'}) - .expect(400) + .expect(422) .then(response => { - t.equal(response.text, 'The stock must not be greater or equal to 0', 'has a need stock message'); + t.equal(response.text, 'The stock must be greater or equal to 0!', 'has a need stock message'); t.end(); }); }); @@ -315,9 +431,9 @@ test('test PUT fruit - error - id error', (t) => { supertest(app) .put('/api/fruits/20') .send({name: 'Banana', stock: 10, id: '22'}) - .expect(400) + .expect(422) .then(response => { - t.equal(response.text, 'The id cannot be changed', 'id error message'); + t.equal(response.text, 'Id was invalidly set on request.', 'id error message'); t.end(); }); }); @@ -387,6 +503,88 @@ test('test PUT fruit - error', (t) => { }); }); +test('test PUT fruit - error - no payload', (t) => { + const app = proxyquire('../app', { + './lib/db': mockDb + }); + + supertest(app) + .put('/api/fruits/20') + .expect(415) + .then(response => { + t.equal(response.text, 'Invalid payload!', 'Payload must be set'); + t.end(); + }); +}); + +test('test PUT fruit - error - invalid payload', (t) => { + const app = proxyquire('../app', { + './lib/db': mockDb + }); + + supertest(app) + .put('/api/fruits/20') + .set('Content-Type', 'application/json') + .send('Some text') + .expect(415) + .then(response => { + t.equal(response.text, 'Invalid payload!', 'Payload must be in JSON format'); + t.end(); + }); +}); + +test('test PUT fruit - error - xml payload', (t) => { + const app = proxyquire('../app', { + './lib/db': mockDb + }); + const xmlFruitData = 'Banana10'; + supertest(app) + .put('/api/fruits/10') + .set('Content-Type', 'application/xml') + .send(xmlFruitData) + .expect(415) + .then(response => { + t.equal(response.text, 'Invalid payload!', 'Payload must be in JSON format'); + t.end(); + }); +}); + +test('test PUT fruit - error - JSON Content-Type and XML body', (t) => { + const app = proxyquire('../app', { + './lib/db': mockDb + }); + const xmlFruitData = 'adam10'; + supertest(app) + .put('/api/fruits/10') + .set('Content-Type', 'application/json') + .send(xmlFruitData) + .expect(415) + .then(response => { + t.equal(response.text, 'Invalid payload!', 'Payload must be in JSON format'); + t.end(); + }); +}); + +test('test PUT fruit - error - no numeric stock', (t) => { + const fruitData = { + name: 'Banana', + stock: 'two' + }; + + const app = proxyquire('../app', { + './lib/db': mockDb + }); + + supertest(app) + .put('/api/fruits/10') + .send(fruitData) + .expect(422) + .then(response => { + t.equal(response.text, 'The stock must be greater or equal to 0!', 'has a need stock message'); + t.end(); + }); +}); + test('test DELETE fruit', (t) => { const mockApi = { remove: (id) => {