From 0a8329b405370e81b759e1e79906270d5e0a7371 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 21 Apr 2016 16:23:41 -0400 Subject: [PATCH 1/2] Support multiple validation failure causes * src/validate/Failure.js (__construct): Takes an array of causes. (getCauses): Now returns an array of causes. * src/validate/ValidStateMonitor.js: Recognize fixes on array of causes. * test/validate/FailureTest.js: Updated * test/validate/ValidStateMonitorTest.js: Updated --- src/validate/Failure.js | 55 ++++++++++++++++------ src/validate/ValidStateMonitor.js | 63 ++++++++++++++++++-------- test/validate/FailureTest.js | 32 +++++++++---- test/validate/ValidStateMonitorTest.js | 43 +++++++++++++++--- 4 files changed, 145 insertions(+), 48 deletions(-) diff --git a/src/validate/Failure.js b/src/validate/Failure.js index c46435b..579eef1 100644 --- a/src/validate/Failure.js +++ b/src/validate/Failure.js @@ -41,38 +41,65 @@ module.exports = Class( 'Failure', 'private _reason': '', /** - * Field that caused the error - * @type {?Field} + * Fields that caused the error + * @type {?Array.} */ - 'private _cause': null, + 'private _causes': null, /** - * Create failure with optional reason and cause + * Create failure with optional reason and causes * * The field FIELD is the target of the failure, which might have - * been caused by another field CAUSE. If cause is omitted, it is + * been caused by another field CAUSES. If cause is omitted, it is * assumed to be FIELD. The string REASON describes the failure. * * @param {Field} field failed field * @param {string=} reason description of failure - * @param {Field=} cause field that triggered the failure + * @param {Field=} causes field that triggered the failure */ - __construct: function( field, reason, cause ) + __construct: function( field, reason, causes ) { if ( !Class.isA( Field, field ) ) { throw TypeError( "Field expected" ); } - if ( ( cause !== undefined ) && !Class.isA( Field, cause ) ) + if ( causes !== undefined ) { - throw TypeError( "Field expected for cause" ); + this._checkCauses( causes ); } this._field = field; this._reason = ( reason === undefined ) ? '' : ''+reason; - this._cause = cause || field; + this._causes = causes || [ field ]; + }, + + + /** + * Validate cause data types + * + * Ensures that CAUSES is an array of Field objects; otherwise, throws + * a TypeError. + * + * @param {Array.} causes failure causes + * + * @return {undefined} + */ + 'private _checkCauses': function( causes ) + { + if ( Object.prototype.toString.call( causes ) !== '[object Array]' ) + { + throw TypeError( "Array of causes expected" ); + } + + for ( var i in causes ) + { + if ( !Class.isA( Field, causes[ i ] ) ) + { + throw TypeError( "Field expected for causes" ); + } + } }, @@ -101,14 +128,14 @@ module.exports = Class( 'Failure', /** * Retrieve field that caused the failure * - * Unless a separate cause was provided during instantiation, the + * Unless a separate causes was provided during instantiation, the * failure is assumed to have been caused by the target field itself. * - * @return {Field} cause of failure + * @return {Array.} causes of failure */ - 'public getCause': function() + 'public getCauses': function() { - return this._cause; + return this._causes; }, diff --git a/src/validate/ValidStateMonitor.js b/src/validate/ValidStateMonitor.js index d32b7f1..a333376 100644 --- a/src/validate/ValidStateMonitor.js +++ b/src/validate/ValidStateMonitor.js @@ -186,14 +186,44 @@ module.exports = Class( 'ValidStateMonitor' ) var past_fail = past[ name ], fail = failures[ name ]; - // we must check each individual index because it is possible that - // not every index was modified or fixed (we must loop through like - // this because this is treated as a hash table, not an array) - for ( var i in past_fail ) + has_fixed = has_fixed || this._checkFailureFix( + name, fail, past_fail, data, fixed + ); + } + + return ( has_fixed ) + ? fixed + : null; + }, + + + /** + * Check past failure fixes + * + * @param {string} name failing field name + * @param {Array} fail failing field index/value + * @param {Array} past_fail past failures for field name + * @param {Object} data validated data + * @param {Object} fixed destination for fixed field data + * + * @return {boolean} whether a field was fixed + */ + 'private _checkFailureFix': function( name, fail, past_fail, data, fixed ) + { + var has_fixed = false; + + // we must check each individual index because it is possible that + // not every index was modified or fixed (we must loop through like + // this because this is treated as a hash table, not an array) + for ( var i in past_fail ) + { + var causes = this._getCauses( i, past_fail ); + + for ( var cause_i in causes ) { - var cause = this._getCause( name, i, past_fail ), - cause_name = cause[ 0 ], - cause_index = cause[ 1 ], + var cause = causes[ cause_i ], + cause_name = cause.getName(), + cause_index = cause.getIndex(), field = data[ cause_name ]; // if datum is unchanged, ignore it @@ -218,13 +248,12 @@ module.exports = Class( 'ValidStateMonitor' ) has_fixed = true; delete past_fail[ i ]; + break; } } } - return ( has_fixed ) - ? fixed - : null; + return has_fixed; }, @@ -235,24 +264,18 @@ module.exports = Class( 'ValidStateMonitor' ) * This maintains backwards-compatibility for the old string-based * system. * - * @param {string} name field name - * @param {number} index field index - * + * @param {number} index field index * @param {Array.} past_fail previous failure * - * @return {Array} name/index tuple of cause field + * @return {Array} list of causes */ - 'private _getCause': function( name, index, past_fail ) + 'private _getCauses': function( index, past_fail ) { var failure = past_fail[ index ]; if ( Class.isA( Failure, failure ) ) { - var cause = failure.getCause(); - - return [ cause.getName(), cause.getIndex() ]; + return failure.getCauses(); } - - return [ name, index ]; } } ); diff --git a/test/validate/FailureTest.js b/test/validate/FailureTest.js index e9abd3f..9b97e1a 100644 --- a/test/validate/FailureTest.js +++ b/test/validate/FailureTest.js @@ -43,12 +43,28 @@ describe( 'Failure', function() } ); - it( 'throws error when not given a Field for cause', function() + it( 'throws error when not given a Field for causes', function() { expect( function() { + // not an array Sut( DummyField(), '', {} ); } ).to.throw( TypeError ); + + expect( function() + { + // one not a Field + Sut( DummyField(), '', [ DummyField(), {} ] ); + } ).to.throw( TypeError ); + } ); + + + it( 'does not throw error for empty clause list', function() + { + expect( function() + { + Sut( DummyField(), '', [] ); + } ).to.not.throw( TypeError ); } ); @@ -83,14 +99,14 @@ describe( 'Failure', function() } ); - describe( '#getCause', function() + describe( '#getCauses', function() { - it( 'returns original cause field', function() + it( 'returns original cause fields', function() { - var cause = DummyField(); + var causes = [ DummyField(), DummyField() ]; - expect( Sut( DummyField(), '', cause ).getCause() ) - .to.equal( cause ); + expect( Sut( DummyField(), '', causes ).getCauses() ) + .to.equal( causes ); } ); @@ -99,8 +115,8 @@ describe( 'Failure', function() { var field = DummyField(); - expect( Sut( field ).getCause() ) - .to.equal( field ); + expect( Sut( field ).getCauses() ) + .to.deep.equal( [ field ] ); } ); } ); diff --git a/test/validate/ValidStateMonitorTest.js b/test/validate/ValidStateMonitorTest.js index 084adf7..52f3705 100644 --- a/test/validate/ValidStateMonitorTest.js +++ b/test/validate/ValidStateMonitorTest.js @@ -214,7 +214,7 @@ describe( 'ValidStateMonitor', function() var data = { cause: [ 'bar' ] }, field = Field( 'foo', 0 ), cause = Field( 'cause', 0 ), - fail = Failure( field, 'reason', cause ); + fail = Failure( field, 'reason', [ cause ] ); Sut() .on( 'fix', function( fixed ) @@ -235,7 +235,33 @@ describe( 'ValidStateMonitor', function() var data = { cause: [ undefined, 'bar' ] }, field = Field( 'foo', 0 ), cause = Field( 'cause', 1 ), - fail = Failure( field, 'reason', cause ); + fail = Failure( field, 'reason', [ cause ] ); + + Sut() + .on( 'fix', function( fixed ) + { + expect( fixed ) + .to.deep.equal( { foo: [ 'bar' ] } ); + + done(); + } ) + .update( data, { foo: [ fail ] } ) + .update( data, {} ); + } ); + + + it( 'considers any number of causes', function( done ) + { + // different index + var data = { cause_fix: [ undefined, 'bar' ] }, + field = Field( 'foo', 0 ), + cause1 = Field( 'cause_no', 1 ), + cause2 = Field( 'cause_fix', 1 ), + fail = Failure( + field, + 'reason', + [ cause1, cause2 ] + ); Sut() .on( 'fix', function( fixed ) @@ -253,10 +279,15 @@ describe( 'ValidStateMonitor', function() it( 'recognizes non-fix', function() { // no cause data - var data = { noncause: [ undefined, 'bar' ] }, - field = Field( 'foo', 0 ), - cause = Field( 'cause', 1 ), - fail = Failure( field, 'reason', cause ); + var data = { noncause: [ undefined, 'bar' ] }, + field = Field( 'foo', 0 ), + cause1 = Field( 'cause', 1 ), + cause2 = Field( 'cause', 2 ), + fail = Failure( + field, + 'reason', + [ cause1, cause2 ] + ); Sut() .on( 'fix', nocall ) From b3d3f4b7cdaaa1d8113359f800f645eef6d05f82 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 21 Apr 2016 16:28:34 -0400 Subject: [PATCH 2/2] Remove support for string-style validation failures * src/validate/ValidStateMonitor (_getCauses): Removed Now requires that each failure be a Failure object. --- src/validate/ValidStateMonitor.js | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/src/validate/ValidStateMonitor.js b/src/validate/ValidStateMonitor.js index a333376..c3ddc20 100644 --- a/src/validate/ValidStateMonitor.js +++ b/src/validate/ValidStateMonitor.js @@ -217,7 +217,7 @@ module.exports = Class( 'ValidStateMonitor' ) // this because this is treated as a hash table, not an array) for ( var i in past_fail ) { - var causes = this._getCauses( i, past_fail ); + var causes = past_fail[ i ] && past_fail[ i ].getCauses(); for ( var cause_i in causes ) { @@ -254,28 +254,5 @@ module.exports = Class( 'ValidStateMonitor' ) } return has_fixed; - }, - - - /** - * Produces name and index of the field causing a failure, or NAME - * and INDEX if unavailable - * - * This maintains backwards-compatibility for the old string-based - * system. - * - * @param {number} index field index - * @param {Array.} past_fail previous failure - * - * @return {Array} list of causes - */ - 'private _getCauses': function( index, past_fail ) - { - var failure = past_fail[ index ]; - - if ( Class.isA( Failure, failure ) ) - { - return failure.getCauses(); - } } } );