From 0dcbd3220250ab889472d92433b24470d8a5e051 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 30 Jan 2017 00:06:08 -0500 Subject: [PATCH] DataValidator: accept classification results * src/validate/DataValidator.js (validate): New `classes' parameter. API BC break from previous commits. (populateStore, updateFailures): Generalize methods. * test/validate/DataValidatorTest.js: Add associated test. Refactor existing tests to adhere to new API/expectations. DEV-2206 --- src/validate/DataValidator.js | 65 ++++++++++++++------------ test/validate/DataValidatorTest.js | 74 ++++++++++++++++++++++++++---- 2 files changed, 102 insertions(+), 37 deletions(-) diff --git a/src/validate/DataValidator.js b/src/validate/DataValidator.js index fa56ec6..2d613f5 100644 --- a/src/validate/DataValidator.js +++ b/src/validate/DataValidator.js @@ -56,10 +56,10 @@ module.exports = Class( 'DataValidator', 'private _factory': null, /** - * Bucket diff store - * @type {Store} + * Various layers of the diff store + * @type {Object} */ - 'private _store_factory': null, + 'private _stores': {}, /** @@ -96,7 +96,7 @@ module.exports = Class( 'DataValidator', */ 'private _createStores': function( store_factory ) { - this._bucket_store = store_factory(); + this._stores = store_factory(); }, @@ -112,24 +112,28 @@ module.exports = Class( 'DataValidator', * @return {Promise} accepts with unspecified value once field monitor * has completed its update */ - 'public validate'( diff, validatef ) + 'public validate'( diff, classes, validatef ) { const _self = this; let failures = {}; - _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.updateFailures( diff, failures ); + return this._populateStore( classes, this._stores.cstore, 'indexes' ) + .then( () => this.updateFailures( diff, failures ) ); }, @@ -146,36 +150,39 @@ module.exports = Class( 'DataValidator', */ 'public updateFailures'( diff, failures ) { - const _self = this; - - return this._populateStore( diff ).then( () => - { - return _self._field_monitor.update( - _self._bucket_store, failures - ); - } ); + return this._populateStore( diff, this._stores.bstore ).then( () => + this._field_monitor.update( + this._stores.store, failures + ) + ); }, /** - * Populate store with diff + * Populate store with data * * This effectively converts a basic array into a `Store`. This is * surprisingly performant on v8. If the stores mix in traits, there * may be a slight performance hit for trait-overridden methods. * - * @param {Object} diff bucket diff + * @param {Object} data data to map onto store * * @return {Promise} when all items have been added to the store */ - 'private _populateStore'( diff ) + 'private _populateStore'( data, store, subkey ) { - var bstore = this._bucket_store; + if ( data === undefined ) + { + return Promise.resolve( [] ); + } - return Promise.all( - Object.keys( diff ).map( - key => bstore.add( key, diff[ key ] ) - ) - ); + const mapf = ( subkey !== undefined ) + ? key => store.add( key, data[ key ][ subkey ] ) + : key => store.add( key, data[ key ] ); + + return store.clear() + .then( () => Promise.all( + Object.keys( data ).map( mapf ) + ) ); }, } ); diff --git a/test/validate/DataValidatorTest.js b/test/validate/DataValidatorTest.js index 62478ee..f01eb0f 100644 --- a/test/validate/DataValidatorTest.js +++ b/test/validate/DataValidatorTest.js @@ -56,7 +56,11 @@ describe( 'DataValidator', () => const vmonitor = ValidStateMonitor(); const dep_factory = createMockDependencyFactory(); - const getStore = createStubStore(); + + const getStore = createStubStore(); + const { bstore } = getStore(); + + const mock_bstore = sinon.mock( bstore ); const mock_vmonitor = sinon.mock( vmonitor ); const mock_dep_factory = sinon.mock( dep_factory ); @@ -68,9 +72,10 @@ describe( 'DataValidator', () => foo: { 1: expected_failure } }; + // call to actual validator mock_vmonitor.expects( 'update' ) .once() - .withExactArgs( getStore(), expected_failures ) + .withExactArgs( getStore().store, expected_failures ) .returns( Promise.resolve( undefined ) ); mock_dep_factory.expects( 'createFieldFailure' ) @@ -78,6 +83,11 @@ describe( 'DataValidator', () => .withExactArgs( 'foo', 1, expected_value ) .returns( expected_failure ); + // clears previous diffs + mock_bstore.expects( 'clear' ) + .once() + .returns( Promise.resolve( bstore ) ); + return Sut( bvalidator, vmonitor, dep_factory, getStore ) .validate( diff ) .then( () => @@ -86,12 +96,54 @@ describe( 'DataValidator', () => mock_dep_factory.verify(); // cleared on call to err in above mock validator - return expect( getStore().get( 'foo' ) ) + return expect( getStore().bstore.get( 'foo' ) ) .to.eventually.deep.equal( [ 'a', undefined, 'c' ] ); } ); } ); + it( 'merges classification changes with diff', () => + { + // SUT will only care about the indexes + const classes = { + first: { indexes: [], is: false }, + second: { indexes: [ 0, 1 ], is: true }, + }; + + const bvalidator = createMockBucketValidator(); + const vmonitor = ValidStateMonitor(); + const dep_factory = createMockDependencyFactory(); + + const getStore = createStubStore(); + const { cstore } = getStore(); + + const mock_cstore = sinon.mock( cstore ); + + // clears previous diffs + mock_cstore.expects( 'clear' ) + .once() + .returns( Promise.resolve( cstore ) ); + + return Sut( bvalidator, vmonitor, dep_factory, getStore ) + .validate( {}, classes ) + .then( () => + { + // clear should have been called + mock_cstore.verify(); + + // keep in mind that we are using MemoryStore for this + // test (whereas a real implementation would probably be + // using a DiffStore) + return Promise.all( + Object.keys( classes ).map( key => + expect( cstore.get( key ) ) + .to.eventually.deep.equal( classes[ key ].indexes ) + ) + ); + } ); + } ); + + it( 'considers failures from external validator', () => { const expected_failure = {}; @@ -130,7 +182,7 @@ describe( 'DataValidator', () => sinon.mock( vmonitor ) .expects( 'update' ) .once() - .withExactArgs( getStore(), expected_failures ) + .withExactArgs( getStore().store, expected_failures ) .returns( Promise.resolve( undefined ) ); sinon.mock( dep_factory ) @@ -138,13 +190,13 @@ describe( 'DataValidator', () => .returns( expected_failure ); return Sut( bvalidator, vmonitor, dep_factory, getStore ) - .validate( diff, validatef ); + .validate( diff, {}, validatef ); } ); it( 'rejects if field monitor update rejects', () => { - const bvalidator = createMockBucketValidator( ( x, y, z ) => {} ); + const bvalidator = createMockBucketValidator(); const vmonitor = ValidStateMonitor(); const dep_factory = createMockDependencyFactory(); @@ -166,6 +218,8 @@ describe( 'DataValidator', () => function createMockBucketValidator( validatef ) { + validatef = validatef || ( ( x, y, z ) => {} ); + return BucketDataValidator.extend( { 'override public validate': validatef, @@ -186,7 +240,11 @@ function createMockDependencyFactory( map ) function createStubStore() { - const store = MemoryStore(); + const stores = { + store: MemoryStore(), + bstore: MemoryStore(), + cstore: MemoryStore(), + }; - return () => store; + return () => stores; }