Merge pull request Tabcorp#7 from TabDigital/collect-error-messages

Report groups of errors accurately.

LICENSE.md

@@ -0,0 +1,21 @@
+The MIT License (MIT)
+
+Copyright (c) 2014 Tabcorp
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.

README.md

@@ -26,7 +26,7 @@ var validate = sware({
 server.post('/users', validate, controller);
 ```
 
-`strummer-middleware` can validate 3 areas of the request:
+`strummer-middleware` can validate 4 areas of the request:
 
 ```js
 sware({

index.js

@@ -8,21 +8,34 @@ var AREAS = {
 };
 
 module.exports = function(checks) {
-
     // first validate the middleware options
     validateOptions(checks);
 
     // return actual middleware
     // unwind loop for better performance
     return function(req, res, next) {
-        var err = errorsFrom(checks, req, 'params')
-               || errorsFrom(checks, req, 'query')
-               || errorsFrom(checks, req, 'body')
-               || errorsFrom(checks, req, 'headers')
-               || null;
-        next(err);
-    };
+        var errors = [];
+        var collectedDetails = [];
+        var collectedAreas = [];
+
+        Object.keys(AREAS).forEach(function(area) {
+            var err = errorsFrom(checks, req, area);
+            if (err) {
+                errors.push(err);
+                collectedAreas.push(area);
+                Array.prototype.push.apply(collectedDetails, err.details);
+            }
+        });
 
+        if (errors.length > 1) {
+            var msg = 'There were combined errors in ' + (collectedAreas.join(', ')) + '.';
+            var err = new Error(msg);
+            err.details = collectedDetails;
+            next(err);
+        } else {
+            next(errors[0]);
+        }
+    };
 };
 
 function validateOptions(checks) {
@@ -47,7 +60,7 @@ function isMatcher(check) {
 
 function errorsFrom(checks, req, area) {
     if (checks[area]) {
-        var result = checks[area].match(null, req[area]);
+        var result = checks[area].match(area, req[area]);
         if (result.length > 0) {
             var err = new Error(AREAS[area]);
             err.details = result;

test/index-test.js

@@ -57,7 +57,7 @@ describe('middleware validation', function() {
             expect(err).to.be.ok;
             expect(err.message).to.equal('Invalid request payload');
             expect(err.details).to.eql([{
-                path: null,
+                path: 'body',
                 value: 'foo',
                 message: 'should not be foo'
             }]);
@@ -70,7 +70,7 @@ describe('middleware validation', function() {
         };
         var validate = middleware({ body: fixtures.success() });
         validate(req, {}, function(err) {
-            expect(err).to.equal(null);
+            expect(err).to.be.falsy;
         });
     });
 
@@ -86,12 +86,12 @@ describe('middleware validation', function() {
             headers: spy
         });
         validate(req, {}, function(err) {
-            expect(err).to.equal(null);
+            expect(err).to.be.falsy;
             expect(spy.count()).to.equal(4);
         });
     });
 
-    it('stops at the first failing check', function() {
+    it('collects all errors and reports them in one error', function() {
         var req = {
             params: 'p',
             query: 'q',
@@ -106,13 +106,41 @@ describe('middleware validation', function() {
         });
         validate(req, {}, function(err) {
             expect(err).to.be.ok;
-            expect(err.message).to.equal('Invalid request parameters');
-            expect(err.details).to.eql([{
-                path: null,
-                value: 'p',
-                message: 'should not be p'
-            }]);
+            expect(err.message).to.equal('There were combined errors in params, query, body, headers.');
+
+            Object.keys(req).forEach(function(area, i){
+                expect(err.details[i].path).to.equal(area);
+                expect(err.details[i].value).to.equal(req[area]);
+                expect(err.details[i].message).to.equal('should not be ' + req[area]);
+            });
         });
     });
 
+    it('elegantly copes with interspersed failures', function() {
+        var req = {
+            params: 'p',
+            query: 'q',
+            body: 'b',
+            headers: 'h'
+        };
+        var validate = middleware({
+            params: fixtures.failure(),
+            query: fixtures.success(),
+            body: fixtures.success(),
+            headers: fixtures.failure()
+        });
+        validate(req, {}, function(err) {
+            expect(err).to.be.ok;
+            expect(err.message).to.equal('There were combined errors in params, headers.');
+
+            expect(err.details[0].path).to.equal('params');
+            expect(err.details[0].value).to.equal('p');
+            expect(err.details[0].message).to.equal('should not be p');
+
+            expect(err.details[1].path).to.equal('headers');
+            expect(err.details[1].value).to.equal('h');
+            expect(err.details[1].message).to.equal('should not be h');
+        });
+    });
 });
+