From 200e65b2e11dec2dbe129cb45144e0354800d07f Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Mon, 3 Apr 2017 11:09:52 -0700 Subject: [PATCH 01/10] use component parameters to build query string --- client/src/get-components-data.js | 2 +- client/src/href-builder.js | 4 ++-- package.json | 3 ++- test/unit/client-get-component-data.js | 5 +++-- test/unit/client-href-builder.js | 10 ++++------ 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/client/src/get-components-data.js b/client/src/get-components-data.js index 2b534ee08..8e43d337c 100644 --- a/client/src/get-components-data.js +++ b/client/src/get-components-data.js @@ -62,7 +62,7 @@ module.exports = function(config){ var performGet = function(endpoint, serverRendering, options, callback) { var component = serverRendering.components[0]; - var requestUrl = hrefBuilder.prepareServerGet(endpoint, component, options); + var requestUrl = hrefBuilder.prepareServerGet(endpoint, component); var requestDetails = { url: requestUrl, diff --git a/client/src/href-builder.js b/client/src/href-builder.js index bac133946..00b32b426 100644 --- a/client/src/href-builder.js +++ b/client/src/href-builder.js @@ -50,9 +50,9 @@ module.exports = function(config){ } }, - prepareServerGet: function(baseUrl, component, options) { + prepareServerGet: function(baseUrl, component) { var urlPath = component.name + (component.version ? '/' + component.version : ''); - var qs = options.parameters ? ('/?' + querystring.stringify(options.parameters)) : ''; + var qs = component.parameters ? ('/?' + querystring.stringify(component.parameters)) : ''; return url.resolve(baseUrl, urlPath + qs); } diff --git a/package.json b/package.json index 14ac4c31d..8e6725631 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,8 @@ "oc": "./src/oc-cli.js" }, "scripts": { - "test": "grunt test" + "test": "grunt test", + "test:unit": "mocha test/unit" }, "engines": { "node": ">=4" diff --git a/test/unit/client-get-component-data.js b/test/unit/client-get-component-data.js index 91652ce1b..8ebec07b0 100644 --- a/test/unit/client-get-component-data.js +++ b/test/unit/client-get-component-data.js @@ -37,7 +37,8 @@ describe('client : get-component-data', () => { const component = { name: 'hello-world', - version: '1.0.0' + version: '1.0.0', + parameters: options.parameters }; getComponentData([{ @@ -54,7 +55,7 @@ describe('client : get-component-data', () => { it('href-builder prepareServerGet method is invoked', () => { sinon.assert.calledOnce(serverStub); sinon.assert.calledOnce(prepareServerGetStub); - sinon.assert.calledWith(prepareServerGetStub, baseUrl, {name: 'hello-world', version: '1.0.0'}, options); + sinon.assert.calledWith(prepareServerGetStub, baseUrl, { name: 'hello-world', version: '1.0.0', parameters: options.parameters }); }); it('a GET request is made with the URL returned by href-builder.prepareServerGet method', () => { diff --git a/test/unit/client-href-builder.js b/test/unit/client-href-builder.js index a529a5622..461060e51 100644 --- a/test/unit/client-href-builder.js +++ b/test/unit/client-href-builder.js @@ -40,22 +40,20 @@ describe('client : href-builder :', () => { describe('when there is one component parameter set in the options', () => { it('it should return a valid request for the component with the parameter set as URL query param', () => { - let options = {parameters: {p1: 'v1'}}; - let component = {name: 'hello-world', version: '1.0.0'}; + let component = { name: 'hello-world', version: '1.0.0', parameters: { p1: 'v1' } }; let hrefBuilder = new hrefBuilderPrototype({}); - expect(hrefBuilder.prepareServerGet('http://localhost:3030', component, options)) + expect(hrefBuilder.prepareServerGet('http://localhost:3030', component)) .to.equal('http://localhost:3030/hello-world/1.0.0/?p1=v1'); }); }); describe('when there are more than one component parameters set in the options', () => { it('it should return a valid request for the component with the parameters set as URL query params', () => { - let options = {parameters: {p1: 'v1', p2: 'v 2'}}; - let component = {name: 'hello-world', version: '1.0.0'}; + let component = { name: 'hello-world', version: '1.0.0', parameters: { p1: 'v1', p2: 'v 2' } }; let hrefBuilder = new hrefBuilderPrototype({}); - expect(hrefBuilder.prepareServerGet('http://localhost:3030', component, options)) + expect(hrefBuilder.prepareServerGet('http://localhost:3030', component)) .to.equal('http://localhost:3030/hello-world/1.0.0/?p1=v1&p2=v%202'); }); }); From 3d5df1b454541f26fa74bcefc56a92298c57d3ff Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Mon, 3 Apr 2017 15:29:12 -0700 Subject: [PATCH 02/10] add merge object function --- client/src/get-components-data.js | 2 +- client/src/href-builder.js | 12 ++++++++++-- client/src/merge-objects.js | 9 +++++++++ test/unit/client-href-builder.js | 11 +++++++++++ test/unit/client-merge-objects.js | 23 +++++++++++++++++++++++ 5 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 client/src/merge-objects.js create mode 100644 test/unit/client-merge-objects.js diff --git a/client/src/get-components-data.js b/client/src/get-components-data.js index 8e43d337c..2b534ee08 100644 --- a/client/src/get-components-data.js +++ b/client/src/get-components-data.js @@ -62,7 +62,7 @@ module.exports = function(config){ var performGet = function(endpoint, serverRendering, options, callback) { var component = serverRendering.components[0]; - var requestUrl = hrefBuilder.prepareServerGet(endpoint, component); + var requestUrl = hrefBuilder.prepareServerGet(endpoint, component, options); var requestDetails = { url: requestUrl, diff --git a/client/src/href-builder.js b/client/src/href-builder.js index 00b32b426..be2864b3f 100644 --- a/client/src/href-builder.js +++ b/client/src/href-builder.js @@ -4,6 +4,7 @@ var querystring = require('querystring'); var format = require('stringformat'); var url = require('url'); var settings = require('./settings'); +var mergeObjects = require('./merge-objects'); module.exports = function(config){ return { @@ -50,9 +51,16 @@ module.exports = function(config){ } }, - prepareServerGet: function(baseUrl, component) { + prepareServerGet: function(baseUrl, component, options) { var urlPath = component.name + (component.version ? '/' + component.version : ''); - var qs = component.parameters ? ('/?' + querystring.stringify(component.parameters)) : ''; + + component = component || {}; + options = options || {}; + + var qs = ''; + if (component.parameters || options.parameters) { + qs = '/?' + querystring.stringify(mergeObjects(component.parameters, options.parameters)); + } return url.resolve(baseUrl, urlPath + qs); } diff --git a/client/src/merge-objects.js b/client/src/merge-objects.js new file mode 100644 index 000000000..56453bbd4 --- /dev/null +++ b/client/src/merge-objects.js @@ -0,0 +1,9 @@ +'use strict'; + +// info: use Object.assign() w/ ES6 +module.exports = function (obj1, obj2) { + var obj3 = {}; + for (var attrname in obj1) { obj3[attrname] = obj1[attrname]; } + for (var attrname in obj2) { obj3[attrname] = obj2[attrname]; } + return obj3; +} diff --git a/test/unit/client-href-builder.js b/test/unit/client-href-builder.js index 461060e51..1e29e0c11 100644 --- a/test/unit/client-href-builder.js +++ b/test/unit/client-href-builder.js @@ -57,5 +57,16 @@ describe('client : href-builder :', () => { .to.equal('http://localhost:3030/hello-world/1.0.0/?p1=v1&p2=v%202'); }); }); + + describe('when there are parameters in both "options" and "component"', () => { + it('it should return a valid request for the component with the parameters set as URL query params', () => { + let options = { parameters: { p1: 'v1', p2: 'v 2' } }; + let component = { name: 'hello-world', version: '1.0.0', parameters: { message: 'hello' } }; + let hrefBuilder = new hrefBuilderPrototype({}); + + expect(hrefBuilder.prepareServerGet('http://localhost:3030', component, options)) + .to.equal('http://localhost:3030/hello-world/1.0.0/?p1=v1&p2=v%202&message=hello'); + }); + }); }); }); diff --git a/test/unit/client-merge-objects.js b/test/unit/client-merge-objects.js new file mode 100644 index 000000000..4dfa3b5bc --- /dev/null +++ b/test/unit/client-merge-objects.js @@ -0,0 +1,23 @@ +'use strict'; + +var expect = require('chai').expect; +var mergeObjects = require('../../client/src/merge-objects'); + +describe('client : merge-objects :', () => { + const scenarios = [ + { describe: 'obj1 is empty and obj2 is empty', obj1: {}, obj2: {}, obj3: {} }, + { describe: 'obj1 is not empty and obj2 is empty', obj1: { p1: 'aaa' }, obj2: {}, obj3: { p1: 'aaa' } }, + { describe: 'obj1 is empty and obj2 is not empty', obj1: {}, obj2: { p2: 'bbb' }, obj3: { p2: 'bbb' } }, + { describe: 'obj1 is not empty and obj2 is not empty', obj1: { p1: 'aaa' }, obj2: { p2: 'bbb' }, obj3: { p1: 'aaa', p2: 'bbb' } }, + { describe: 'obj1 is undefined and obj2 is undefined', obj1: undefined, obj2: undefined, obj3: {} }, + { describe: 'obj1 is null and obj2 is null', obj1: null, obj2: null, obj3: {} } + ]; + + scenarios.forEach((scenario) => { + describe(`when ${scenario.describe}`, () => { + it(`then obj3 to equal ${JSON.stringify(scenario.obj3)}`, () => { + expect(mergeObjects(scenario.obj1, scenario.obj2)).to.deep.equal(scenario.obj3); + }); + }); + }); +}); From 6b244e15d105731f9833ae41f88f4baa41feb545 Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Mon, 3 Apr 2017 15:55:48 -0700 Subject: [PATCH 03/10] fix test --- test/unit/client-get-component-data.js | 5 ++--- test/unit/client-href-builder.js | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/test/unit/client-get-component-data.js b/test/unit/client-get-component-data.js index 8ebec07b0..4d857bbe7 100644 --- a/test/unit/client-get-component-data.js +++ b/test/unit/client-get-component-data.js @@ -37,8 +37,7 @@ describe('client : get-component-data', () => { const component = { name: 'hello-world', - version: '1.0.0', - parameters: options.parameters + version: '1.0.0' }; getComponentData([{ @@ -55,7 +54,7 @@ describe('client : get-component-data', () => { it('href-builder prepareServerGet method is invoked', () => { sinon.assert.calledOnce(serverStub); sinon.assert.calledOnce(prepareServerGetStub); - sinon.assert.calledWith(prepareServerGetStub, baseUrl, { name: 'hello-world', version: '1.0.0', parameters: options.parameters }); + sinon.assert.calledWith(prepareServerGetStub, baseUrl, { name: 'hello-world', version: '1.0.0'}, options); }); it('a GET request is made with the URL returned by href-builder.prepareServerGet method', () => { diff --git a/test/unit/client-href-builder.js b/test/unit/client-href-builder.js index 1e29e0c11..8b7f7c28d 100644 --- a/test/unit/client-href-builder.js +++ b/test/unit/client-href-builder.js @@ -65,7 +65,7 @@ describe('client : href-builder :', () => { let hrefBuilder = new hrefBuilderPrototype({}); expect(hrefBuilder.prepareServerGet('http://localhost:3030', component, options)) - .to.equal('http://localhost:3030/hello-world/1.0.0/?p1=v1&p2=v%202&message=hello'); + .to.equal('http://localhost:3030/hello-world/1.0.0/?message=hello&p1=v1&p2=v%202'); }); }); }); From d45c760747e33900c783551dd2799f76a52bb2b2 Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Mon, 3 Apr 2017 15:56:52 -0700 Subject: [PATCH 04/10] fix test part II --- test/unit/client-get-component-data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/client-get-component-data.js b/test/unit/client-get-component-data.js index 4d857bbe7..91652ce1b 100644 --- a/test/unit/client-get-component-data.js +++ b/test/unit/client-get-component-data.js @@ -54,7 +54,7 @@ describe('client : get-component-data', () => { it('href-builder prepareServerGet method is invoked', () => { sinon.assert.calledOnce(serverStub); sinon.assert.calledOnce(prepareServerGetStub); - sinon.assert.calledWith(prepareServerGetStub, baseUrl, { name: 'hello-world', version: '1.0.0'}, options); + sinon.assert.calledWith(prepareServerGetStub, baseUrl, {name: 'hello-world', version: '1.0.0'}, options); }); it('a GET request is made with the URL returned by href-builder.prepareServerGet method', () => { From e3aefc172e8370a1aeb95fb8319729681804f9d8 Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Mon, 3 Apr 2017 17:24:45 -0700 Subject: [PATCH 05/10] fix test, fix lint --- client/src/merge-objects.js | 14 +++++++++++--- test/unit/client-href-builder.js | 10 ++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/client/src/merge-objects.js b/client/src/merge-objects.js index 56453bbd4..34b89d22a 100644 --- a/client/src/merge-objects.js +++ b/client/src/merge-objects.js @@ -1,9 +1,17 @@ 'use strict'; +function addProperties(source, destination) { + for (var key in source) { + if (source.hasOwnProperty(key)) { + destination[key] = source[key]; + } + } +} + // info: use Object.assign() w/ ES6 module.exports = function (obj1, obj2) { var obj3 = {}; - for (var attrname in obj1) { obj3[attrname] = obj1[attrname]; } - for (var attrname in obj2) { obj3[attrname] = obj2[attrname]; } + addProperties(obj1, obj3); + addProperties(obj2, obj3); return obj3; -} +}; diff --git a/test/unit/client-href-builder.js b/test/unit/client-href-builder.js index 8b7f7c28d..179f7ca0b 100644 --- a/test/unit/client-href-builder.js +++ b/test/unit/client-href-builder.js @@ -40,20 +40,22 @@ describe('client : href-builder :', () => { describe('when there is one component parameter set in the options', () => { it('it should return a valid request for the component with the parameter set as URL query param', () => { - let component = { name: 'hello-world', version: '1.0.0', parameters: { p1: 'v1' } }; + let options = {parameters: {p1: 'v1'}}; + let component = {name: 'hello-world', version: '1.0.0'}; let hrefBuilder = new hrefBuilderPrototype({}); - expect(hrefBuilder.prepareServerGet('http://localhost:3030', component)) + expect(hrefBuilder.prepareServerGet('http://localhost:3030', component, options)) .to.equal('http://localhost:3030/hello-world/1.0.0/?p1=v1'); }); }); describe('when there are more than one component parameters set in the options', () => { it('it should return a valid request for the component with the parameters set as URL query params', () => { - let component = { name: 'hello-world', version: '1.0.0', parameters: { p1: 'v1', p2: 'v 2' } }; + let options = {parameters: {p1: 'v1', p2: 'v 2'}}; + let component = {name: 'hello-world', version: '1.0.0'}; let hrefBuilder = new hrefBuilderPrototype({}); - expect(hrefBuilder.prepareServerGet('http://localhost:3030', component)) + expect(hrefBuilder.prepareServerGet('http://localhost:3030', component, options)) .to.equal('http://localhost:3030/hello-world/1.0.0/?p1=v1&p2=v%202'); }); }); From c6f48befccf83a1f3873c9438d460bda05eace9a Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Mon, 3 Apr 2017 18:32:57 -0700 Subject: [PATCH 06/10] remove test:unit npm script --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index 8e6725631..14ac4c31d 100644 --- a/package.json +++ b/package.json @@ -7,8 +7,7 @@ "oc": "./src/oc-cli.js" }, "scripts": { - "test": "grunt test", - "test:unit": "mocha test/unit" + "test": "grunt test" }, "engines": { "node": ">=4" From 7ea328c16e41415bba2ddbbfc1d4dde9a40c9f2b Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Mon, 3 Apr 2017 20:37:03 -0700 Subject: [PATCH 07/10] housekeeping --- client/src/href-builder.js | 3 --- test/unit/client-merge-objects.js | 6 +++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/client/src/href-builder.js b/client/src/href-builder.js index be2864b3f..6b09eb602 100644 --- a/client/src/href-builder.js +++ b/client/src/href-builder.js @@ -54,9 +54,6 @@ module.exports = function(config){ prepareServerGet: function(baseUrl, component, options) { var urlPath = component.name + (component.version ? '/' + component.version : ''); - component = component || {}; - options = options || {}; - var qs = ''; if (component.parameters || options.parameters) { qs = '/?' + querystring.stringify(mergeObjects(component.parameters, options.parameters)); diff --git a/test/unit/client-merge-objects.js b/test/unit/client-merge-objects.js index 4dfa3b5bc..b0f0a7286 100644 --- a/test/unit/client-merge-objects.js +++ b/test/unit/client-merge-objects.js @@ -1,7 +1,7 @@ 'use strict'; -var expect = require('chai').expect; -var mergeObjects = require('../../client/src/merge-objects'); +const expect = require('chai').expect; +const mergeObjects = require('../../client/src/merge-objects'); describe('client : merge-objects :', () => { const scenarios = [ @@ -15,7 +15,7 @@ describe('client : merge-objects :', () => { scenarios.forEach((scenario) => { describe(`when ${scenario.describe}`, () => { - it(`then obj3 to equal ${JSON.stringify(scenario.obj3)}`, () => { + it(`then obj3 is equal to ${JSON.stringify(scenario.obj3)}`, () => { expect(mergeObjects(scenario.obj1, scenario.obj2)).to.deep.equal(scenario.obj3); }); }); From a36f255a6b86f33785cdeaf373c96af1e3d1bb7b Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Mon, 3 Apr 2017 21:26:12 -0700 Subject: [PATCH 08/10] add scenario w/ parameter in common --- client/src/merge-objects.js | 2 +- test/unit/client-merge-objects.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/client/src/merge-objects.js b/client/src/merge-objects.js index 34b89d22a..748887737 100644 --- a/client/src/merge-objects.js +++ b/client/src/merge-objects.js @@ -2,7 +2,7 @@ function addProperties(source, destination) { for (var key in source) { - if (source.hasOwnProperty(key)) { + if (source.hasOwnProperty(key) && !destination[key]) { destination[key] = source[key]; } } diff --git a/test/unit/client-merge-objects.js b/test/unit/client-merge-objects.js index b0f0a7286..aeb7598a0 100644 --- a/test/unit/client-merge-objects.js +++ b/test/unit/client-merge-objects.js @@ -10,7 +10,8 @@ describe('client : merge-objects :', () => { { describe: 'obj1 is empty and obj2 is not empty', obj1: {}, obj2: { p2: 'bbb' }, obj3: { p2: 'bbb' } }, { describe: 'obj1 is not empty and obj2 is not empty', obj1: { p1: 'aaa' }, obj2: { p2: 'bbb' }, obj3: { p1: 'aaa', p2: 'bbb' } }, { describe: 'obj1 is undefined and obj2 is undefined', obj1: undefined, obj2: undefined, obj3: {} }, - { describe: 'obj1 is null and obj2 is null', obj1: null, obj2: null, obj3: {} } + { describe: 'obj1 is null and obj2 is null', obj1: null, obj2: null, obj3: {} }, + { describe: 'obj1 and obj2 have a property in common', obj1: { a: 1, b: 2 }, obj2: { b: 3, c: 4 }, obj3: { a: 1, b: 2, c: 4 } } ]; scenarios.forEach((scenario) => { From 13b91a8595be5a087ab373494957101328457e16 Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Tue, 4 Apr 2017 07:53:23 -0700 Subject: [PATCH 09/10] add scenario w/ common parameter --- test/unit/client-href-builder.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/unit/client-href-builder.js b/test/unit/client-href-builder.js index 179f7ca0b..d01b1aebc 100644 --- a/test/unit/client-href-builder.js +++ b/test/unit/client-href-builder.js @@ -70,5 +70,16 @@ describe('client : href-builder :', () => { .to.equal('http://localhost:3030/hello-world/1.0.0/?message=hello&p1=v1&p2=v%202'); }); }); + + describe('when there are common parameters in both "options" and "component"', () => { + it('it should return a valid request for the component with the parameters set as URL query params', () => { + let options = { parameters: { p1: 'v1', p2: 'v 2' } }; + let component = { name: 'hello-world', version: '1.0.0', parameters: { message: 'hello', p1: 'v3' } }; + let hrefBuilder = new hrefBuilderPrototype({}); + + expect(hrefBuilder.prepareServerGet('http://localhost:3030', component, options)) + .to.equal('http://localhost:3030/hello-world/1.0.0/?message=hello&p1=v3&p2=v%202'); + }); + }); }); }); From db3cb1f424c5605a2a1ccbc678628006918fb839 Mon Sep 17 00:00:00 2001 From: mattia richetto Date: Tue, 4 Apr 2017 10:00:34 -0700 Subject: [PATCH 10/10] move merge-objects into utils --- client/src/href-builder.js | 2 +- client/src/{ => utils}/merge-objects.js | 0 test/unit/client-merge-objects.js | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename client/src/{ => utils}/merge-objects.js (100%) diff --git a/client/src/href-builder.js b/client/src/href-builder.js index 6b09eb602..7a4ab6f8b 100644 --- a/client/src/href-builder.js +++ b/client/src/href-builder.js @@ -4,7 +4,7 @@ var querystring = require('querystring'); var format = require('stringformat'); var url = require('url'); var settings = require('./settings'); -var mergeObjects = require('./merge-objects'); +var mergeObjects = require('./utils/merge-objects'); module.exports = function(config){ return { diff --git a/client/src/merge-objects.js b/client/src/utils/merge-objects.js similarity index 100% rename from client/src/merge-objects.js rename to client/src/utils/merge-objects.js diff --git a/test/unit/client-merge-objects.js b/test/unit/client-merge-objects.js index aeb7598a0..2ee51fa10 100644 --- a/test/unit/client-merge-objects.js +++ b/test/unit/client-merge-objects.js @@ -1,7 +1,7 @@ 'use strict'; const expect = require('chai').expect; -const mergeObjects = require('../../client/src/merge-objects'); +const mergeObjects = require('../../client/src/utils/merge-objects'); describe('client : merge-objects :', () => { const scenarios = [