From ddffb4e301860e0e1cacddae043a801ea38a43c8 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 19 Apr 2016 11:29:14 -0400 Subject: [PATCH 1/4] No failure event if failures have not changed in ValidStateMonitor * src/validate/ValidStateMonitor.js (mergeFailures): Return count of new failures * test/validate/ValidStateMonitorTest.js: Respective test added --- src/validate/ValidStateMonitor.js | 14 +++++++++----- test/validate/ValidStateMonitorTest.js | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/validate/ValidStateMonitor.js b/src/validate/ValidStateMonitor.js index 586146e..cc79c93 100644 --- a/src/validate/ValidStateMonitor.js +++ b/src/validate/ValidStateMonitor.js @@ -55,11 +55,10 @@ module.exports = Class( 'ValidStateMonitor' ) */ 'public update': function( data, failures ) { - var fixed = this.detectFixes( data, this._failures, failures ); + var fixed = this.detectFixes( data, this._failures, failures ), + count_new = this.mergeFailures( this._failures, failures ); - this.mergeFailures( this._failures, failures ); - - if ( this.hasFailures() ) + if ( this.hasFailures() && ( count_new > 0 ) ) { this.emit( 'failure', this._failures ); } @@ -136,10 +135,12 @@ module.exports = Class( 'ValidStateMonitor' ) * @param {Object} past past failures to merge with * @param {Object} failures new failures * - * @return {undefined} + * @return {number} number of new failures */ 'virtual protected mergeFailures': function( past, failures ) { + var count_new = 0; + for ( var name in failures ) { past[ name ] = past[ name ] || []; @@ -148,8 +149,11 @@ module.exports = Class( 'ValidStateMonitor' ) for ( var i in failures[ name ] ) { past[ name ][ i ] = failures[ name ][ i ]; + count_new++; } } + + return count_new; }, diff --git a/test/validate/ValidStateMonitorTest.js b/test/validate/ValidStateMonitorTest.js index ea743d0..54fcad7 100644 --- a/test/validate/ValidStateMonitorTest.js +++ b/test/validate/ValidStateMonitorTest.js @@ -151,6 +151,22 @@ describe( 'ValidStateMonitor', function() } ) .update( data, {} ); } ); + + + it( 'does not trigger failure event for existing', function() + { + var called = 0; + + Sut() + .on( 'failure', function() + { + called++; + } ) + .update( {}, { foo: [ 'bar' ] } ) + .update( {}, {} ); // do not trigger failure event + + expect( called ).to.equal( 1 ); + } ); } ); From ac5d04e9a46aeab8e9e80ab76ddff24e526e7a30 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 20 Apr 2016 00:24:21 -0400 Subject: [PATCH 2/4] Add validation Failure * src/validate/Failure.js: Added * test/validate/FailureTtest.js: Added --- src/validate/Failure.js | 127 +++++++++++++++++++++++++++++++++++ test/validate/FailureTest.js | 118 ++++++++++++++++++++++++++++++++ 2 files changed, 245 insertions(+) create mode 100644 src/validate/Failure.js create mode 100644 test/validate/FailureTest.js diff --git a/src/validate/Failure.js b/src/validate/Failure.js new file mode 100644 index 0000000..c46435b --- /dev/null +++ b/src/validate/Failure.js @@ -0,0 +1,127 @@ +/** + * Validation failure + * + * Copyright (C) 2016 LoVullo Associates, Inc. + * + * This file is part of liza. + * + * liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var Class = require( 'easejs' ).Class, + Field = require( '../field/Field' ); + + +/** + * Represents a field validation failure + */ +module.exports = Class( 'Failure', +{ + /** + * Failure field + * @type {Field} + */ + 'private _field': null, + + /** + * Validation failure reason + * @type {string} + */ + 'private _reason': '', + + /** + * Field that caused the error + * @type {?Field} + */ + 'private _cause': null, + + + /** + * Create failure with optional reason and cause + * + * The field FIELD is the target of the failure, which might have + * been caused by another field CAUSE. 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 + */ + __construct: function( field, reason, cause ) + { + if ( !Class.isA( Field, field ) ) + { + throw TypeError( "Field expected" ); + } + + if ( ( cause !== undefined ) && !Class.isA( Field, cause ) ) + { + throw TypeError( "Field expected for cause" ); + } + + this._field = field; + this._reason = ( reason === undefined ) ? '' : ''+reason; + this._cause = cause || field; + }, + + + /** + * Retrieve target of the failure + * + * @return {Field} target field + */ + 'public getField': function() + { + return this._field; + }, + + + /** + * Retrieve a description of the failure, or the empty string + * + * @return {string} failure description + */ + 'public getReason': function() + { + return this._reason; + }, + + + /** + * Retrieve field that caused the failure + * + * Unless a separate cause was provided during instantiation, the + * failure is assumed to have been caused by the target field itself. + * + * @return {Field} cause of failure + */ + 'public getCause': function() + { + return this._cause; + }, + + + /** + * Produce failure reason when converted to a string + * + * This allows the failure to be used in place of the traditional system + * of error strings. + * + * @return {string} + */ + __toString: function() + { + return this._reason; + } +} ); diff --git a/test/validate/FailureTest.js b/test/validate/FailureTest.js new file mode 100644 index 0000000..e9abd3f --- /dev/null +++ b/test/validate/FailureTest.js @@ -0,0 +1,118 @@ +/** + * Test validation failure + * + * Copyright (C) 2016 LoVullo Associates, Inc. + * + * This file is part of liza. + * + * liza is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +var root = require( '../../' ), + Sut = root.validate.Failure, + expect = require( 'chai' ).expect; + +var DummyField = require( 'easejs' ).Class + .implement( root.field.Field ) + .extend( +{ + getName: function() {}, + getIndex: function() {}, +} ); + + +describe( 'Failure', function() +{ + it( 'throws error when not given Field for failure', function() + { + expect( function() + { + Sut( {} ); + } ).to.throw( TypeError ); + } ); + + + it( 'throws error when not given a Field for cause', function() + { + expect( function() + { + Sut( DummyField(), '', {} ); + } ).to.throw( TypeError ); + } ); + + + describe( '#getField', function() + { + it( 'returns original field', function() + { + var field = DummyField(); + + expect( Sut( field ).getField() ) + .to.equal( field ); + } ); + } ); + + + describe( '#getReason', function() + { + it( 'returns original failure reason', function() + { + var reason = 'solar flares'; + + expect( Sut( DummyField(), reason ).getReason() ) + .to.equal( reason ); + } ); + + + it( 'returns empty string by default', function() + { + expect( Sut( DummyField() ).getReason() ) + .to.equal( '' ); + } ); + } ); + + + describe( '#getCause', function() + { + it( 'returns original cause field', function() + { + var cause = DummyField(); + + expect( Sut( DummyField(), '', cause ).getCause() ) + .to.equal( cause ); + } ); + + + // in other words: field caused itself to fail + it( 'returns field by default', function() + { + var field = DummyField(); + + expect( Sut( field ).getCause() ) + .to.equal( field ); + } ); + } ); + + + describe( 'when converted to a string', function() + { + it( 'produces failure reason', function() + { + var reason = 'bogons'; + + expect( ''+Sut( DummyField(), reason ) ) + .to.equal( reason ); + } ); + } ); +} ); From f784db5f2bafe3200c1130478f91317c0ff97ffb Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 20 Apr 2016 11:06:54 -0400 Subject: [PATCH 3/4] Ensure that Failure objects will work transparently in ValidStateMonitor This is to facilitate a transition to Failure without a BC break. * test/validate/ValidStateMonitorTest.js: Ensure that replacing messages with Failure will continue to operate as expected. --- test/validate/ValidStateMonitorTest.js | 102 +++++++++++++++++-------- 1 file changed, 70 insertions(+), 32 deletions(-) diff --git a/test/validate/ValidStateMonitorTest.js b/test/validate/ValidStateMonitorTest.js index 54fcad7..6ce9dcd 100644 --- a/test/validate/ValidStateMonitorTest.js +++ b/test/validate/ValidStateMonitorTest.js @@ -19,8 +19,12 @@ * along with this program. If not, see . */ -var Sut = require( '../../' ).validate.ValidStateMonitor, - expect = require( 'chai' ).expect; +var root = require( '../../' ), + Sut = root.validate.ValidStateMonitor, + expect = require( 'chai' ).expect, + Failure = root.validate.Failure, + Field = root.field.BucketField; + var nocall = function( type ) { @@ -30,6 +34,16 @@ var nocall = function( type ) }; }; +var mkfail = function( name, arr ) +{ + return arr.map( function( value, i ) + { + return ( value === undefined ) + ? undefined + : Failure( Field( name, i ), value ); + } ); +}; + describe( 'ValidStateMonitor', function() { @@ -49,7 +63,7 @@ describe( 'ValidStateMonitor', function() Sut() .on( 'failure', nocall( 'failure' ) ) .on( 'fix', nocall( 'fix' ) ) - .update( { foo: [ 'bar' ] }, {} ); + .update( { foo: mkfail( 'foo', [ 'bar' ] ) }, {} ); } ); @@ -59,40 +73,45 @@ describe( 'ValidStateMonitor', function() { it( 'marks failures even when given no data', function( done ) { + var fail = mkfail( 'foo', [ 'bar', 'baz' ] ); + Sut() - .on( 'failure', function( failures ) + .on( 'failure', function( failures ) { expect( failures ) - .to.deep.equal( { foo: [ 'bar', 'baz' ] } ); + .to.deep.equal( { foo: [ fail[ 0 ], fail[ 1 ] ] } ); done(); } ) .on( 'fix', nocall( 'fix' ) ) - .update( {}, { foo: [ 'bar', 'baz' ] } ); + .update( {}, { foo: fail } ); } ); it( 'marks failures with index gaps', function( done ) { + var fail = mkfail( 'foo', [ undefined, 'baz' ] ); + Sut() - .on( 'failure', function( failures ) + .on( 'failure', function( failures ) { expect( failures ) - .to.deep.equal( { foo: [ undefined, 'baz' ] } ); + .to.deep.equal( { foo: [ undefined, fail[ 1 ] ] } ); done(); } ) .on( 'fix', nocall( 'fix' ) ) - .update( {}, { foo: [ undefined, 'baz' ] } ); + .update( {}, { foo: fail } ); } ); it( 'retains past failures when setting new', function( done ) { - var sut = Sut(); + var sut = Sut(), + fail = mkfail( 'foo', [ 'bar', 'baz' ] ); var test_first = function( failures ) { expect( failures ) - .to.deep.equal( { foo: [ undefined, 'baz' ] } ); + .to.deep.equal( { foo: [ undefined, fail[ 1 ] ] } ); sut.once( 'failure', test_second ); }; @@ -100,7 +119,7 @@ describe( 'ValidStateMonitor', function() var test_second = function( failures ) { expect( failures ) - .to.deep.equal( { foo: [ 'bar', 'baz' ] } ); + .to.deep.equal( { foo: [ fail[ 0 ], fail[ 1 ] ] } ); done(); }; @@ -108,8 +127,25 @@ describe( 'ValidStateMonitor', function() sut .once( 'failure', test_first ) .on( 'fix', nocall( 'fix' ) ) - .update( {}, { foo: [ undefined, 'baz' ] } ) - .update( {}, { foo: [ 'bar' ] } ); + .update( {}, { foo: [ undefined, fail[ 1 ] ] } ) + .update( {}, { foo: [ fail[ 0 ] ] } ); + } ); + + + // deprecated + it( 'accepts failures as string for BC', function( done ) + { + var fail = [ 'foo', 'bar' ]; + + Sut() + .on( 'failure', function( failures ) + { + expect( failures ) + .to.deep.equal( { foo: fail } ); + done(); + } ) + .on( 'fix', nocall( 'fix' ) ) + .update( {}, { foo: fail } ); } ); } ); @@ -118,7 +154,8 @@ describe( 'ValidStateMonitor', function() { it( 'removes non-failures if field is present', function( done ) { - var data = { foo: [ 'bardata', 'baz' ] }; + var data = { foo: [ 'bardata', 'baz' ] }, + fail = mkfail( 'foo', [ 'bar', 'baz' ] ); Sut() .on( 'fix', function( fixed ) @@ -127,16 +164,16 @@ describe( 'ValidStateMonitor', function() .to.deep.equal( { foo: [ 'bardata' ] } ); done(); } ) - .update( data, { foo: [ 'bar', 'baz' ] } ) - .update( data, { foo: [ '', 'baz' ] } ); + .update( data, { foo: [ fail[ 0 ], fail[ 1 ] ] } ) + .update( data, { foo: [ undefined, fail[ 1 ] ] } ); } ); it( 'keeps failures if field is missing', function( done ) { - var data = { - bar: [ 'baz', 'quux' ], - }; + var data = { bar: [ 'baz', 'quux' ] }, + fail_foo = mkfail( 'foo', [ 'bar', 'baz' ] ), + fail_bar = mkfail( 'bar', [ 'moo', 'cow' ] ); Sut() .on( 'fix', function( fixed ) @@ -146,8 +183,8 @@ describe( 'ValidStateMonitor', function() done(); } ) .update( data, { - foo: [ 'bar', 'baz' ], // does not exist in data - bar: [ 'moo', 'cow' ], + foo: fail_foo, // does not exist in data + bar: fail_bar, } ) .update( data, {} ); } ); @@ -162,7 +199,7 @@ describe( 'ValidStateMonitor', function() { called++; } ) - .update( {}, { foo: [ 'bar' ] } ) + .update( {}, { foo: mkfail( 'foo', [ 'bar' ] ) } ) .update( {}, {} ); // do not trigger failure event expect( called ).to.equal( 1 ); @@ -172,19 +209,18 @@ describe( 'ValidStateMonitor', function() it( 'can emit both failure and fix', function( done ) { - var data = { - bar: [ 'baz', 'quux' ], - }; + var data = { bar: [ 'baz', 'quux' ] }, + fail_foo = mkfail( 'foo', [ 'bar' ] ); Sut() .update( data, { - bar: [ 'moo', 'cow' ] // fail + bar: mkfail( 'bar', [ 'moo', 'cow' ] ) // fail } ) .on( 'failure', function( failed ) { expect( failed ) .to.deep.equal( { - foo: [ 'bar' ], + foo: fail_foo, } ); } ) .on( 'fix', function( fixed ) @@ -194,7 +230,7 @@ describe( 'ValidStateMonitor', function() done(); } ) .update( data, { - foo: [ 'bar' ], // fail + foo: fail_foo, // fail // fixes bar } ); } ); @@ -214,11 +250,13 @@ describe( 'ValidStateMonitor', function() it( 'retrieves current failures', function() { + var fail = mkfail( 'foo', [ 'fail' ] ); + expect( Sut() - .update( {}, { foo: [ 'fail' ] } ) + .update( {}, { foo: fail } ) .getFailures() - ).to.deep.equal( { foo: [ 'fail' ] } ); + ).to.deep.equal( { foo: fail } ); } ); } ); @@ -236,7 +274,7 @@ describe( 'ValidStateMonitor', function() { expect( Sut() - .update( {}, { foo: [ 'fail' ] } ) + .update( {}, { foo: mkfail( 'foo', [ 'bar' ] ) } ) .hasFailures() ).to.be.true; } ); From f624710451b5955ee6d96fcc2fdf351dc491cc82 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 20 Apr 2016 12:11:17 -0400 Subject: [PATCH 4/4] Consider field failure cause when checking fixes This maintains BC with the old string-based system. * src/validate/ValidStateMonitor.js (_getCause): Added. (detectFixes): Consider failure cause if available when checking for fixes. --- src/validate/ValidStateMonitor.js | 58 +++++++++++++++++++------ test/validate/ValidStateMonitorTest.js | 60 ++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 12 deletions(-) diff --git a/src/validate/ValidStateMonitor.js b/src/validate/ValidStateMonitor.js index cc79c93..d32b7f1 100644 --- a/src/validate/ValidStateMonitor.js +++ b/src/validate/ValidStateMonitor.js @@ -20,7 +20,8 @@ */ var Class = require( 'easejs' ).Class, - EventEmitter = require( 'events' ).EventEmitter; + EventEmitter = require( 'events' ).EventEmitter, + Failure = require( './Failure' ); /** @@ -182,15 +183,7 @@ module.exports = Class( 'ValidStateMonitor' ) for ( var name in past ) { - // we're only interested in detecting fixes on the data that has - // been set - if ( !( data[ name ] ) ) - { - continue; - } - - var field = data[ name ], - past_fail = past[ name ], + var past_fail = past[ name ], fail = failures[ name ]; // we must check each individual index because it is possible that @@ -198,17 +191,29 @@ module.exports = Class( 'ValidStateMonitor' ) // this because this is treated as a hash table, not an array) for ( var i in past_fail ) { + var cause = this._getCause( name, i, past_fail ), + cause_name = cause[ 0 ], + cause_index = cause[ 1 ], + field = data[ cause_name ]; + + // if datum is unchanged, ignore it + if ( field === undefined ) + { + continue; + } + // to be marked as fixed, there must both me no failure and // there must be data for this index for the field in question // (if the field wasn't touched, then of course there's no // failure!) if ( ( fail === undefined ) - || ( !( fail[ i ] ) && ( field[ i ] !== undefined ) ) + || ( !( fail[ cause_index ] ) + && ( field[ cause_index ] !== undefined ) ) ) { // looks like it has been resolved ( fixed[ name ] = fixed[ name ] || [] )[ i ] = - data[ name ][ i ]; + field[ cause_index ] has_fixed = true; @@ -220,5 +225,34 @@ module.exports = Class( 'ValidStateMonitor' ) return ( has_fixed ) ? fixed : null; + }, + + + /** + * 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 {string} name field name + * @param {number} index field index + * + * @param {Array.} past_fail previous failure + * + * @return {Array} name/index tuple of cause field + */ + 'private _getCause': function( name, index, past_fail ) + { + var failure = past_fail[ index ]; + + if ( Class.isA( Failure, failure ) ) + { + var cause = failure.getCause(); + + return [ cause.getName(), cause.getIndex() ]; + } + + return [ name, index ]; } } ); diff --git a/test/validate/ValidStateMonitorTest.js b/test/validate/ValidStateMonitorTest.js index 6ce9dcd..084adf7 100644 --- a/test/validate/ValidStateMonitorTest.js +++ b/test/validate/ValidStateMonitorTest.js @@ -204,6 +204,66 @@ describe( 'ValidStateMonitor', function() expect( called ).to.equal( 1 ); } ); + + + describe( 'given a cause', function() + { + it( 'considers when recognizing fix', function( done ) + { + // same index + var data = { cause: [ 'bar' ] }, + field = Field( 'foo', 0 ), + cause = Field( 'cause', 0 ), + 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 different cause index', function( done ) + { + // different index + var data = { cause: [ undefined, 'bar' ] }, + field = Field( 'foo', 0 ), + cause = Field( 'cause', 1 ), + 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( '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 ); + + Sut() + .on( 'fix', nocall ) + .update( data, { foo: [ fail ] } ) + .update( data, {} ); + } ); + } ); } );