Refactored href-builder to reflect the code review notes

- prepareServerGet was changed to use querystring.stringify for addign the
  component parameters as URL query params.
- change the returned URL to include a trailing '/' before the URL query
  params (and thus making it to follow the same pattern as in client method)
- changed the client.js acceptance test according to the aforementioned changes
- added client-href-builder.js unit test

client/src/href-builder.js

@@ -52,24 +52,9 @@ module.exports = function(config){
 
     prepareServerGet: function(baseUrl, component, options) {
       var urlPath = component.name + (component.version ? '/' + component.version : '');
-      var queryString;
+      var qs = options.parameters ? ('/?' + querystring.stringify(options.parameters)) : '';
 
-      if (options.parameters) {
-        queryString = Object
-          .keys(options.parameters)
-          .map(function(key) {
-            return format('{0}={1}', key, encodeURIComponent(options.parameters[key]));
-          })
-          .reduce(function(a, b) {
-            if (!a) {
-              return b;
-            } else {
-              return format('{0}&{1}', a, b);
-            }
-          }, null);
-      }
-
-      return url.resolve(baseUrl, urlPath + (queryString ? '?' + queryString : ''));
+      return url.resolve(baseUrl, urlPath + qs);
     }
   };
 };

test/acceptance/client.js

@@ -654,7 +654,7 @@ describe('The node.js OC client', function(){
         it('should contain the error details', function(){
 
           var expectedRequestWithExtraParams = {
-            url: 'http://localhost:1234/hello-world/~1.0.0??hi=john',
+            url: 'http://localhost:1234/hello-world/~1.0.0/??hi=john',
             method: 'get',
             headers: {
               'accept-language': 'da, en-gb;q=0.8, en;q=0.7',
@@ -717,7 +717,7 @@ describe('The node.js OC client', function(){
         var error, result;
 
         var expectedRequest = {
-          url: 'http://localhost:3030/errors-component??errorType=timeout&timeout=1000',
+          url: 'http://localhost:3030/errors-component/??errorType=timeout&timeout=1000',
           method: 'get',
           headers: {
             'user-agent': 'oc-client-(.*?)',

test/unit/client-get-component-data.js

@@ -143,4 +143,4 @@ describe('client : get-component-data', () => {
     });
   });
 
-});
+});

test/unit/client-href-builder.js

@@ -0,0 +1,63 @@
+'use strict';
+
+var expect = require('chai').expect;
+var hrefBuilderPrototype = require('../../client/src/href-builder');
+
+describe('client : href-builder :', () => {
+
+  describe('server method - ', () => {
+    describe('when the server rendering endpoint is set in both the options and in the configuration', () => {
+      it('should get the result from the options', () => {
+        let hrefBuilder = new hrefBuilderPrototype({registries: {serverRendering: 'from configuration'}});
+        expect(hrefBuilder.server({registries: {serverRendering: 'from options'}})).to.equal('from options');
+      });
+    });
+
+    describe('when the server rendering endpoint is not set in the options param', () => {
+      it('it should get the result from the configuration', () => {
+        let hrefBuilder = new hrefBuilderPrototype({registries: {serverRendering: 'from configuration'}});
+        expect(hrefBuilder.server({})).to.equal('from configuration');
+      });
+    });
+  });
+
+  describe('prepareServerGet method - ', () => {
+    describe('when only the component name is set', () => {
+      it('it should return a valid request for the component', () => {
+        let hrefBuilder = new hrefBuilderPrototype({});
+        expect(hrefBuilder.prepareServerGet('http://localhost:3030', {name: 'hello-world'}, {}))
+            .to.equal('http://localhost:3030/hello-world');
+      });
+    });
+
+    describe('when the component name and version are set', () => {
+      it('it should return a valid request for the component', () => {
+        let hrefBuilder = new hrefBuilderPrototype({});
+        expect(hrefBuilder.prepareServerGet('http://localhost:3030', {name: 'hello-world', version: '1.0.0'}, {}))
+            .to.equal('http://localhost:3030/hello-world/1.0.0');
+      });
+    });
+
+    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 hrefBuilder = new hrefBuilderPrototype({});
+
+        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 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, options))
+            .to.equal('http://localhost:3030/hello-world/1.0.0/?p1=v1&p2=v%202');
+      });
+    });
+  });
+});