diff --git a/src/validate/DataValidator.js b/src/validate/DataValidator.js index 5b5c38b..56ae480 100644 --- a/src/validate/DataValidator.js +++ b/src/validate/DataValidator.js @@ -109,8 +109,8 @@ module.exports = Class( 'DataValidator', /** * Validate diff and update field monitor * - * If a validation is pending completion, all further `#validate` - * requests will be queued to prevent unexpected/inconsistent system + * If an operation is pending completion, all further requests to this + * object will be queued to prevent unexpected/inconsistent system * states and race conditions. * * The external validator `validatef` is a kluge while the system @@ -126,44 +126,40 @@ module.exports = Class( 'DataValidator', { const _self = this; - // queue requests if we're already processing - if ( this._pending ) + return this._onceReady( () => { - this._pending.then( - () => this.validate( diff, classes, validatef ) - ); + let failures = {}; - return this._pending; - } - - let failures = {}; - - if ( diff !== undefined ) - { - _self._bucket_validator.validate( diff, ( name, value, i ) => + if ( diff !== undefined ) { - diff[ name ][ i ] = undefined; + _self._bucket_validator.validate( diff, ( name, value, i ) => + { + diff[ name ][ i ] = undefined; - ( failures[ name ] = failures[ name ] || {} )[ i ] = - _self._factory.createFieldFailure( name, i, value ); - }, true ); + ( failures[ name ] = failures[ name ] || {} )[ i ] = + _self._factory.createFieldFailure( name, i, value ); + }, true ); - validatef && validatef( diff, failures ); - } + validatef && validatef( diff, failures ); + } - // XXX: this assumes that the above is synchronous - return this._pending = - this._populateStore( - classes, this._stores.cstore, 'indexes' - ) - .then( () => this.updateFailures( diff, failures ) ) - .then( () => this._pending = null ); + // XXX: this assumes that the above is synchronous + return this._pending = + this._populateStore( + classes, this._stores.cstore, 'indexes' + ) + .then( () => this._doUpdateFailures( diff, failures ) ); + } ); }, /** * Update failures from external validation * + * If an operation is pending completion, all further requests to this + * object will be queued to prevent unexpected/inconsistent system + * states and race conditions. + * * TODO: This is a transitional API---we should handle all validations, * not allow external systems to meddle in our affairs. * @@ -173,6 +169,23 @@ module.exports = Class( 'DataValidator', * @return {Promise} promise to populate internal store */ 'public updateFailures'( diff, failures ) + { + return this._onceReady( () => + { + return this._doUpdateFailures( diff, failures ); + } ); + }, + + + /** + * Update failures from external validation + * + * @param {Object} diff bucket diff + * @param {Object} failures failures per field name and index + * + * @return {Promise} promise to populate internal store + */ + 'private _doUpdateFailures': function( diff, failures ) { return this._populateStore( diff, this._stores.bstore ).then( () => this._field_monitor.update( @@ -198,10 +211,31 @@ module.exports = Class( 'DataValidator', 'public clearFailures'( failures ) { this._field_monitor.clearFailures( failures ); + return this; }, + /** + * Wait until all requests are complete and then trigger callback + * + * @param {Function} callback callback to trigger when ready + * + * @return {Promise} + */ + 'private _onceReady': function( callback ) + { + if ( this._pending ) + { + this._pending.then( callback ); + return this._pending; + } + + return this._pending = callback() + .then( () => this._pending = null ); + }, + + /** * Populate store with data * diff --git a/test/validate/DataValidatorTest.js b/test/validate/DataValidatorTest.js index e209a13..ed9a932 100644 --- a/test/validate/DataValidatorTest.js +++ b/test/validate/DataValidatorTest.js @@ -316,6 +316,42 @@ describe( 'DataValidator', () => ); } ); } ); + + + it( 'queues concurrent requests', () => + { + const { sut, vmonitor, getStore } = createStubs( { + vmonitor: sinon.createStubInstance( ValidStateMonitor ), + } ); + const { bstore } = getStore(); + + const faila = {}; + const failb = {}; + + let running_first = true; + + vmonitor.update = ( _, fail ) => + { + if ( fail === failb ) + { + if ( running_first === true ) + { + return Promise.reject( Error( + "Request not queued" + ) ); + } + } + + return Promise.resolve( true ); + }; + + return Promise.all( [ + sut.updateFailures( {}, faila ) + .then( () => running_first = false ), + + sut.updateFailures( {}, failb ), + ] ); + } ); } );