From c5194b4ec5efc6469b0d5753cb0b044ce822b059 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 20 Feb 2017 12:08:55 -0500 Subject: [PATCH] StagingBucket: Do not process non-changes Since changes trigger any event observers---which can be expensive---it is ideal to ignore sets that do not result in any changes to the bucket. This also resolves issues with systems that are confused by empty diffs. * src/bucket/StagingBucket.js (_hasChanged): Add method. (setValues): Use it. * test/bucket/StagingBucketTest.js: Add test case. DEV-2299 --- src/bucket/StagingBucket.js | 47 ++++++++ test/bucket/StagingBucketTest.js | 185 +++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 test/bucket/StagingBucketTest.js diff --git a/src/bucket/StagingBucket.js b/src/bucket/StagingBucket.js index ec2751d..fdd4197 100644 --- a/src/bucket/StagingBucket.js +++ b/src/bucket/StagingBucket.js @@ -161,6 +161,48 @@ module.exports = Class( 'StagingBucket' ) }, + /** + * Determine whether values have changed + * + * If all values are identical to the current bucket values (relative to + * `merge_index`), returns `false`. Otherwise, this stops at the first + * recognized change and returns `true`. + * + * @param {Object.} data key/value data or diff + * @param {boolean} merge_index compare indexes individually + * + * @return {boolean} whether a change was recognized + */ + 'private _hasChanged': function( data, merge_index ) + { + for ( var name in data ) + { + var values = data[ name ]; + var cur = this._curdata[ name ] || []; + + if ( !merge_index && ( values.length !== cur.length ) ) + { + return true; + } + + for ( var index in values ) + { + if ( merge_index && ( values[ index ] === undefined ) ) + { + continue; + } + + if ( values[ index ] !== cur[ index ] ) + { + return true; + } + } + } + + return false; + }, + + /** * Explicitly sets the contents of the bucket * @@ -174,6 +216,11 @@ module.exports = Class( 'StagingBucket' ) */ 'virtual public setValues': function( data, merge_index, merge_null ) { + if ( !this._hasChanged( data, merge_index ) ) + { + return; + } + this.emit( this.__self.$('EVENT_STAGING_PRE_UPDATE'), data ); for ( name in data ) diff --git a/test/bucket/StagingBucketTest.js b/test/bucket/StagingBucketTest.js new file mode 100644 index 0000000..b90d359 --- /dev/null +++ b/test/bucket/StagingBucketTest.js @@ -0,0 +1,185 @@ +/** + * Test of staging key/value store + * + * Copyright (C) 2017 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 . + * + * @todo This needs tests for the rest of StagingBucket + */ + +"use strict"; + +const { Class } = require( 'easejs' ); +const root = require( '../../' ); +const expect = require( 'chai' ).expect; + +const { + Bucket, + StagingBucket: Sut +} = root.bucket; + + +describe( 'StagingBucket', () => +{ + describe( 'pre-update event', () => + { + it( 'allows updating data before set', () => + { + const sut = Sut( createStubBucket() ); + + const data = { + foo: [ 'bar', 'baz' ], + }; + + sut.on( 'preStagingUpdate', data => + { + data.foo[ 1 ] = 'quux'; + } ); + + // triggers setValues + sut.setValues( data ); + + expect( sut.getDataByName( 'foo' ) ) + .to.deep.equal( [ 'bar', 'quux' ] ); + } ); + + + [ + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [ 'bar', 'baz' ] }, + merge_index: true, + is_change: false, + }, + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [ 'bar', 'baz' ] }, + merge_index: false, + is_change: false, + }, + + // actual changes + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [ 'change', 'baz' ] }, + merge_index: true, + is_change: true, + }, + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [ 'bar', 'change' ] }, + merge_index: true, + is_change: true, + }, + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [ undefined, 'change' ] }, + merge_index: true, + is_change: true, + }, + + // single-index changes make sense only if merge_index + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [ undefined, 'baz' ] }, + merge_index: true, + is_change: false, + }, + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [ 'bar', undefined ] }, + merge_index: true, + is_change: false, + }, + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [ 'bar', null ] }, + merge_index: true, + is_change: true, + }, + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [] }, + merge_index: true, + is_change: false, + }, + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [] }, + merge_index: false, + is_change: true, + }, + { + initial: { foo: [ 'bar' ] }, + update: { foo: [ 'bar', undefined ] }, + merge_index: false, + is_change: true, + }, + + // only interpreted as a diff if merge_index + { + initial: { foo: [ 'bar', 'baz' ] }, + update: { foo: [ 'bar', undefined ] }, + merge_index: false, + is_change: true, + }, + + // no index at all + { + initial: { foo: [ 'bar', 'baz' ] }, + update: {}, + merge_index: true, + is_change: false, + }, + ].forEach( ( { initial, update, merge_index, is_change }, i ) => + { + it( `is emitted only when data is changed (${i})`, () => + { + const sut = Sut( createStubBucket() ); + let called = false; + + sut.setValues( initial, merge_index ); + + sut.on( 'preStagingUpdate', () => called = true ); + sut.setValues( update, merge_index ); + + expect( called ).to.equal( is_change ); + } ); + } ); + } ); +} ); + + +function createStubBucket( bucket_obj ) +{ + return Class.implement( Bucket ).extend( + { + 'public getData'() + { + return bucket_obj; + }, + + 'public setValues'( data, merge_index, merge_null ) {}, + 'public overwriteValues'( data ) {}, + 'public clear'() {}, + 'public each'( callback ) {}, + 'public getDataByName'( name ) {}, + 'public getDataJson'() {}, + 'public filter'( pred, callback) {}, + 'on'() {}, + } )(); +}