From c2edf29405459f225bff767da7eae061b39a94b3 Mon Sep 17 00:00:00 2001 From: Alex Li <7alex7li@gmail.com> Date: Wed, 5 Aug 2020 13:58:56 -0400 Subject: [PATCH 1/4] Fixed bug with set saving to the wrong object and set not running before process was called --- dist/data-layer-helper.js | 10 ++++----- dist/data-layer-helper.js.map | 4 ++-- src/helper/helper.js | 23 ++++++++++---------- test/process_test.js | 23 ++++++++++++++++++++ test/setCommand_test.js | 41 +++++++++++++++++------------------ 5 files changed, 61 insertions(+), 40 deletions(-) diff --git a/dist/data-layer-helper.js b/dist/data-layer-helper.js index 031b632b..672bb4f6 100644 --- a/dist/data-layer-helper.js +++ b/dist/data-layer-helper.js @@ -10,8 +10,8 @@ var f=/\[object (Boolean|Number|String|Function|Array|Date|RegExp|Arguments)\]/;function g(a){return null==a?String(a):(a=f.exec(Object.prototype.toString.call(Object(a))))?a[1].toLowerCase():"object"}function k(a,b){return Object.prototype.hasOwnProperty.call(Object(a),b)}function m(a){if(!a||"object"!=g(a)||a.nodeType||a==a.window)return!1;try{if(a.constructor&&!k(a,"constructor")&&!k(a.constructor.prototype,"isPrototypeOf"))return!1}catch(c){return!1}for(var b in a);return void 0===b||k(a,b)};/* Copyright 2012 Google Inc. All rights reserved. */ function p(a,b,c){b=void 0===b?{}:b;"function"===typeof b?b={listener:b,j:void 0===c?!1:c,l:!0,h:{}}:b={listener:b.listener||function(){},j:b.listenToPast||!1,l:void 0===b.processNow?!0:b.processNow,h:b.commandProcessors||{}};this.a=a;this.o=b.listener;this.m=b.j;this.g=!1;this.c={};this.f=[];this.b=b.h;this.i=q(this);b.l&&this.process()} -p.prototype.process=function(){r(this,this.a,!this.m);this.registerProcessor("set",function(){if(1===arguments.length&&"object"===g(arguments[0]))u(arguments[0],this);else if(2===arguments.length&&"string"===g(arguments[0])){var c=v(arguments[0],arguments[1]);u(c,this)}});var a=this.a.push,b=this;this.a.push=function(){var c=[].slice.call(arguments,0),d=a.apply(b.a,c);r(b,c);return d}}; -p.prototype.get=function(a){var b=this.c;a=a.split(".");for(var c=0;c {}), + listener: options['listener'] || (() => { + }), listenToPast: options['listenToPast'] || false, processNow: options['processNow'] === undefined ? true : options['processNow'], @@ -183,26 +184,24 @@ class DataLayerHelper { process() { if (this.processed_) { logError(`Process has already been ran. This method should only ` + - `run a single time to prepare the helper.`, LogLevel.ERROR); + `run a single time to prepare the helper.`, LogLevel.ERROR); } - // Process the existing/past states. - this.processStates_(this.dataLayer_, !(this.listenToPast_)); - // Register a processor for set command. this.registerProcessor('set', function() { - const model = this; - + let toMerge = {}; if (arguments.length === 1 && type(arguments[0]) === 'object') { - merge_(arguments[0], model); + toMerge = arguments[0]; } else if (arguments.length === 2 && - type(arguments[0]) === 'string') { + type(arguments[0]) === 'string') { // Maintain consistency with how objects are merged // outside of the set command (overwrite or recursively merge). - const obj = expandKeyValue_(arguments[0], arguments[1]); - merge_(obj, model); + toMerge = expandKeyValue_(arguments[0], arguments[1]); } + return toMerge; }); + // Process the existing/past states. + this.processStates_(this.dataLayer_, !(this.listenToPast_)); // Mark helper as having been processed. this.processed_ = true; @@ -229,7 +228,7 @@ class DataLayerHelper { */ get(key) { let target = this.model_; - const split = key.split('.'); + const split = key === '' ? [] : key.split('.'); for (let i = 0; i < split.length; i++) { if (target[split[i]] === undefined) return undefined; target = target[split[i]]; diff --git a/test/process_test.js b/test/process_test.js index 8dd8ce02..60136b1f 100644 --- a/test/process_test.js +++ b/test/process_test.js @@ -39,4 +39,27 @@ describe('The `process` function of helper', () => { expect(helper.processed_).toBe(true); }); + + describe('with the built in set command', function() { + let commandAPI; + beforeEach(() => { + commandAPI = function() { + dataLayer.push(arguments); + }; + }); + + it('registers set commands pushed before process', () => { + commandAPI('set', 'two', 2); + helper.process(); + + expect(helper.get('two')).toBe(2); + }); + + it('registers set commands pushed after process', () => { + helper.process(); + commandAPI('set', 'two', 2); + + expect(helper.get('two')).toBe(2); + }); + }); }); diff --git a/test/setCommand_test.js b/test/setCommand_test.js index 62e5fffe..f8310d3c 100644 --- a/test/setCommand_test.js +++ b/test/setCommand_test.js @@ -18,8 +18,7 @@ describe('The set command', () => { beforeEach(() => { dataLayer = []; dlh = new DataLayerHelper(dataLayer); - commandAPI('set', 'bar', 'foo'); - targetModel = Object.assign({}, dlh.abstractModelInterface_); + targetModel = {}; }); describe('with 2 argument format', () => { @@ -27,39 +26,39 @@ describe('The set command', () => { commandAPI('set', 'foo', 'bar'); targetModel['foo'] = 'bar'; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); }); it('updates model using dot-notation without corrupting it.', () => { commandAPI('set', 'foobar.barfoo.val', 'testVal'); targetModel['foobar'] = {'barfoo': {'val': 'testVal'}}; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); }); - it('results in no-op with unecpected number of arguments(0 args)', () => { + it('results in no-op with unexpected number of arguments(0 args)', () => { commandAPI('set'); - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); }); - it('results in no-op with unecpected number of arguments: 3 args', () => { + it('results in no-op with unexpected number of arguments: 3 args', () => { commandAPI('set', 'foo', 'bar', 'extra'); - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); }); - it('results in no-op with an unecpected type for 1st argument', () => { + it('results in no-op with an unexpected type for 1st argument', () => { commandAPI('set', 2, 'bar'); - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); }); it('updates existing key with new val', () => { commandAPI('set', 'foo', 'newBar'); targetModel['foo'] = 'newBar'; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); }); }); @@ -69,7 +68,7 @@ describe('The set command', () => { commandAPI('set', {'yes': 'no'}); targetModel['yes'] = 'no'; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); } ); @@ -81,7 +80,7 @@ describe('The set command', () => { targetModel['hello'] = 'world'; targetModel['goodbye'] = 'bluesky'; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); } ); @@ -90,12 +89,12 @@ describe('The set command', () => { commandAPI('set', {'yes': {'yes': 'no', 'no': 'yes'}}); targetModel['yes'] = {'yes': 'no', 'no': 'yes'}; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); commandAPI('set', {'yes': {'yes': 'yes'}}); targetModel['yes'] = {'yes': 'yes', 'no': 'yes'}; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); } ); @@ -104,7 +103,7 @@ describe('The set command', () => { commandAPI('set', {'array': [1, 2, 3]}); targetModel['array'] = [1, 2, 3]; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); const testArray = []; testArray[3] = 4; @@ -113,13 +112,13 @@ describe('The set command', () => { commandAPI('set', {'array': testArray}); targetModel['array'] = [1, 2, 3, 4, 5, 6]; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); const testArray2 = [undefined, undefined, undefined]; commandAPI('set', {'array': testArray2}); targetModel['array'] = [undefined, undefined, undefined, 4, 5, 6]; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); } ); @@ -128,12 +127,12 @@ describe('The set command', () => { commandAPI('set', {'object': {'test': 'value'}}); targetModel['object'] = {'test': 'value'}; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); commandAPI('set', {'object': {'value': 'test'}}); targetModel['object'] = {'value': 'test', 'test': 'value'}; - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); } ); @@ -141,7 +140,7 @@ describe('The set command', () => { () => { commandAPI('set', 'no op'); - expect(dlh.abstractModelInterface_).toEqual(targetModel); + expect(dlh.get('')).toEqual(targetModel); } ); }); From e985adecf4ed2e0b9152622ac459cca467f77026 Mon Sep 17 00:00:00 2001 From: Alex Li <7alex7li@gmail.com> Date: Wed, 5 Aug 2020 15:45:36 -0400 Subject: [PATCH 2/4] Remove get command feature --- src/helper/helper.js | 2 +- test/setCommand_test.js | 47 +++++++++++++++++++++-------------------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/helper/helper.js b/src/helper/helper.js index 09800b91..ba56da04 100755 --- a/src/helper/helper.js +++ b/src/helper/helper.js @@ -228,7 +228,7 @@ class DataLayerHelper { */ get(key) { let target = this.model_; - const split = key === '' ? [] : key.split('.'); + const split = key.split('.'); for (let i = 0; i < split.length; i++) { if (target[split[i]] === undefined) return undefined; target = target[split[i]]; diff --git a/test/setCommand_test.js b/test/setCommand_test.js index f8310d3c..3afa507a 100644 --- a/test/setCommand_test.js +++ b/test/setCommand_test.js @@ -21,44 +21,45 @@ describe('The set command', () => { targetModel = {}; }); + describe('with 2 argument format', () => { it('updates model without corrupting it.', () => { commandAPI('set', 'foo', 'bar'); targetModel['foo'] = 'bar'; - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); }); it('updates model using dot-notation without corrupting it.', () => { commandAPI('set', 'foobar.barfoo.val', 'testVal'); targetModel['foobar'] = {'barfoo': {'val': 'testVal'}}; - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); }); it('results in no-op with unexpected number of arguments(0 args)', () => { commandAPI('set'); - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); }); it('results in no-op with unexpected number of arguments: 3 args', () => { commandAPI('set', 'foo', 'bar', 'extra'); - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); }); it('results in no-op with an unexpected type for 1st argument', () => { commandAPI('set', 2, 'bar'); - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); }); it('updates existing key with new val', () => { commandAPI('set', 'foo', 'newBar'); targetModel['foo'] = 'newBar'; - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); }); }); @@ -68,20 +69,20 @@ describe('The set command', () => { commandAPI('set', {'yes': 'no'}); targetModel['yes'] = 'no'; - expect(dlh.get('')).toEqual(targetModel); - } + expect(dlh.model_).toEqual(targetModel); + }, ); it('updates model without corruption with multiple key-val pairs', () => { commandAPI('set', - {'yes': 'no', 'hello': 'world', 'goodbye': 'bluesky'}); + {'yes': 'no', 'hello': 'world', 'goodbye': 'bluesky'}); targetModel['yes'] = 'no'; targetModel['hello'] = 'world'; targetModel['goodbye'] = 'bluesky'; - expect(dlh.get('')).toEqual(targetModel); - } + expect(dlh.model_).toEqual(targetModel); + }, ); it('updates model without corruption with nested key-val pairs', @@ -89,13 +90,13 @@ describe('The set command', () => { commandAPI('set', {'yes': {'yes': 'no', 'no': 'yes'}}); targetModel['yes'] = {'yes': 'no', 'no': 'yes'}; - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); commandAPI('set', {'yes': {'yes': 'yes'}}); targetModel['yes'] = {'yes': 'yes', 'no': 'yes'}; - expect(dlh.get('')).toEqual(targetModel); - } + expect(dlh.model_).toEqual(targetModel); + }, ); it('updates model by merging existing array with a new one', @@ -103,7 +104,7 @@ describe('The set command', () => { commandAPI('set', {'array': [1, 2, 3]}); targetModel['array'] = [1, 2, 3]; - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); const testArray = []; testArray[3] = 4; @@ -112,14 +113,14 @@ describe('The set command', () => { commandAPI('set', {'array': testArray}); targetModel['array'] = [1, 2, 3, 4, 5, 6]; - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); const testArray2 = [undefined, undefined, undefined]; commandAPI('set', {'array': testArray2}); targetModel['array'] = [undefined, undefined, undefined, 4, 5, 6]; - expect(dlh.get('')).toEqual(targetModel); - } + expect(dlh.model_).toEqual(targetModel); + }, ); it('updates model by merging existing object with a new one', @@ -127,21 +128,21 @@ describe('The set command', () => { commandAPI('set', {'object': {'test': 'value'}}); targetModel['object'] = {'test': 'value'}; - expect(dlh.get('')).toEqual(targetModel); + expect(dlh.model_).toEqual(targetModel); commandAPI('set', {'object': {'value': 'test'}}); targetModel['object'] = {'value': 'test', 'test': 'value'}; - expect(dlh.get('')).toEqual(targetModel); - } + expect(dlh.model_).toEqual(targetModel); + }, ); it('results in no-op when single argument is not an object', () => { commandAPI('set', 'no op'); - expect(dlh.get('')).toEqual(targetModel); - } + expect(dlh.model_).toEqual(targetModel); + }, ); }); }); From a360a96ac4eb98763066dfad17eb95dc19eb727d Mon Sep 17 00:00:00 2001 From: Alex Li <7alex7li@gmail.com> Date: Fri, 7 Aug 2020 09:39:39 -0400 Subject: [PATCH 3/4] Update per dustin's comments --- src/helper/helper.js | 6 ++---- test/setCommand_test.js | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/helper/helper.js b/src/helper/helper.js index ba56da04..86ecd2ee 100755 --- a/src/helper/helper.js +++ b/src/helper/helper.js @@ -96,8 +96,7 @@ class DataLayerHelper { }; } else { options = { - listener: options['listener'] || (() => { - }), + listener: options['listener'] || (() => {}), listenToPast: options['listenToPast'] || false, processNow: options['processNow'] === undefined ? true : options['processNow'], @@ -192,8 +191,7 @@ class DataLayerHelper { let toMerge = {}; if (arguments.length === 1 && type(arguments[0]) === 'object') { toMerge = arguments[0]; - } else if (arguments.length === 2 && - type(arguments[0]) === 'string') { + } else if (arguments.length === 2 && type(arguments[0]) === 'string') { // Maintain consistency with how objects are merged // outside of the set command (overwrite or recursively merge). toMerge = expandKeyValue_(arguments[0], arguments[1]); diff --git a/test/setCommand_test.js b/test/setCommand_test.js index 3afa507a..1b9eaaca 100644 --- a/test/setCommand_test.js +++ b/test/setCommand_test.js @@ -21,7 +21,6 @@ describe('The set command', () => { targetModel = {}; }); - describe('with 2 argument format', () => { it('updates model without corrupting it.', () => { commandAPI('set', 'foo', 'bar'); @@ -145,4 +144,22 @@ describe('The set command', () => { }, ); }); + + it('does not modify the abstractModelInterface', () => { + commandAPI('set', 'a key', 'value'); + commandAPI('set', {'b key': 'value'}); + + expect(dlh.abstractModelInterface_).toEqual({ + get: jasmine.any(Function), + set: jasmine.any(Function), + }); + }); + + it('creates values accessible by the abstract model interface', () => { + commandAPI('set', 'a key', 'value'); + commandAPI('set', {'b key': 'value2'}); + + expect(dlh.abstractModelInterface_.get('a key')).toEqual('value'); + expect(dlh.abstractModelInterface_.get('b key')).toEqual('value2'); + }); }); From f037e847afde673e5b905f120829a3553669e89d Mon Sep 17 00:00:00 2001 From: Alex Li <7alex7li@gmail.com> Date: Mon, 10 Aug 2020 16:45:39 -0400 Subject: [PATCH 4/4] Fix indentation --- src/helper/data-layer-helper.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/helper/data-layer-helper.js b/src/helper/data-layer-helper.js index 87ae6807..d5bdb5fc 100755 --- a/src/helper/data-layer-helper.js +++ b/src/helper/data-layer-helper.js @@ -192,8 +192,7 @@ class DataLayerHelper { if (arguments.length === 1 && type(arguments[0]) === 'object') { merge(arguments[0], model); - } else if (arguments.length === 2 && - type(arguments[0]) === 'string') { + } else if (arguments.length === 2 && type(arguments[0]) === 'string') { // Maintain consistency with how objects are merged // outside of the set command (overwrite or recursively merge). const obj = expandKeyValue(arguments[0], arguments[1]);