Fix for Custom eaders of one component can appear to another. (#328)

The problem was introduced with #326 -
In get-component.js the declaration of responseHeaders variable happens in a
more global scope than it should be. For this reason every invocation of
renderer function sees the changes in the headers made by previously requested
components.
The fix is to just move the declaration one scope inner.

- Added a unit test to validate the fix - it tries to load two components
subsequently. The first one does provide custom headers while the second
one - don't. Then we check and expect to see there are no custom headers
in the result for the second one.

Author: i-b-dimitrov

src/registry/routes/helpers/get-component.js

@@ -26,13 +26,13 @@ module.exports = function(conf, repository){
       cache = new Cache({
         verbose: !!conf.verbosity,
         refreshInterval: conf.refreshInterval
-      }),
-      responseHeaders;
+      });
 
   var renderer = function(options, cb){
 
     var nestedRenderer = new NestedRenderer(renderer, options.conf),
-        retrievingInfo = new GetComponentRetrievingInfo(options);
+        retrievingInfo = new GetComponentRetrievingInfo(options),
+        responseHeaders;
 
     var getLanguage = function(){
       var paramOverride = !!options.parameters && options.parameters['__ocAcceptLanguage'];

test/fixtures/mocked-components/another-response-headers.js

@@ -0,0 +1,27 @@
+'use strict';
+
+module.exports = {
+  package: {
+    name: 'another-response-headers-component',
+    version: '1.0.0',
+    oc: {
+      container: false,
+      renderInfo: false,
+      files: {
+        template: {
+          type: 'jade',
+          hashKey: '8c1fbd954f2b0d8cd5cf11c885fed4805225749f',
+          src: 'template.js'
+        },
+        dataProvider: {
+          type: 'node.js',
+          hashKey: '123456',
+          src: 'server.js'
+        }
+      }
+    }
+  },
+  data: '"use strict";module.exports.data = function(ctx, cb){ctx.setHeader("another-test-header","another-test-value"); cb(null, {done:true});};',
+  view: 'var oc=oc||{};oc.components=oc.components||{},oc.components["8c1fbd954f2b0d8cd5cf11c885fed4805225749f"]' +
+        '=function(){var o=[];return o.push("<div>hello</div>"),o.join("")};'
+};

test/fixtures/mocked-components/index.js

@@ -8,5 +8,7 @@ module.exports = {
   'plugin-component': require('./plugin'),
   'timeout-component': require('./timeout'),
   'undefined-component': require('./undefined'),
-  'response-headers-component': require('./response-headers')
+  'simple-component': require('./simple'),
+  'response-headers-component': require('./response-headers'),
+  'another-response-headers-component': require('./another-response-headers')
 };

test/fixtures/mocked-components/simple.js

@@ -0,0 +1,27 @@
+'use strict';
+
+module.exports = {
+  package: {
+    name: 'simple-component',
+    version: '1.0.0',
+    oc: {
+      container: false,
+      renderInfo: false,
+      files: {
+        template: {
+          type: 'jade',
+          hashKey: '8c1fbd954f2b0d8cd5cf11c885fed4805225749f',
+          src: 'template.js'
+        },
+        dataProvider: {
+          type: 'node.js',
+          hashKey: '123457',
+          src: 'server.js'
+        }
+      }
+    }
+  },
+  data: '"use strict";module.exports.data = function(ctx, cb){cb(null, {done:true});};',
+  view: 'var oc=oc||{};oc.components=oc.components||{},oc.components["8c1fbd954f2b0d8cd5cf11c885fed4805225749f"]' +
+        '=function(){var o=[];return o.push("<div>hello</div>"),o.join("")};'
+};

test/unit/registry-routes-component.js

@@ -419,7 +419,6 @@ describe('registry : routes : component', function(){
   });
 
   describe('when getting a component with server.js that sets custom headers', function() {
-
     var code, response, headers;
 
     before(function(done) {
@@ -466,4 +465,104 @@ describe('registry : routes : component', function(){
     });
   });
 
+  describe('when getting a simple component with server.js after headers component no custom headers should be set', function() {
+    var code={}, 
+      response={}, 
+      headers={};
+
+    before(function(done) {
+      var headersComponent = mockedComponents['another-response-headers-component'];
+      var simpleComponent = mockedComponents['simple-component'];
+
+      mockedRepository = {
+        getCompiledView: sinon.stub(),
+        getComponent: sinon.stub(),
+        getDataProvider: sinon.stub(),
+        getStaticFilePath: sinon.stub().returns('//my-cdn.com/files/')
+      };
+
+      //Custom repository initialization to give us two components when invoked twice...
+      //...the firts one with custom headers and the second - without.
+      mockedRepository.getCompiledView.onCall(0).yields(null, headersComponent.view);
+      mockedRepository.getCompiledView.onCall(1).yields(null, simpleComponent.view);
+
+      mockedRepository.getComponent.onCall(0).yields(null, headersComponent.package);
+      mockedRepository.getComponent.onCall(1).yields(null, simpleComponent.package);
+
+      mockedRepository.getDataProvider.onCall(0).yields(null, headersComponent.data);
+      mockedRepository.getDataProvider.onCall(1).yields(null, simpleComponent.data);
+
+      componentRoute = new ComponentRoute({}, mockedRepository);
+      resJsonStub = sinon.stub();
+
+      var resJson = function(index, callback) { 
+        return function(calledCode, calledResponse) {
+          code[index] = calledCode;
+          response[index] = calledResponse;
+          callback && callback();
+        };
+      };
+
+      var resSet = function(index) { 
+        return function(calledHeaders) {
+          headers[index] = calledHeaders;
+        };
+      };
+
+      var requestComponent = function(componentName, resultIndex) {
+        return new Promise(function(resolve, reject) {
+          componentRoute({
+            headers: {},
+            params: { componentName: componentName, componentVersion: '1.X.X' }
+          }, {
+            conf: {
+              baseUrl: 'http://component.com/',
+              executionTimeout: 0.1
+            },
+            json: resJson(resultIndex, function() { resolve(); }),
+            set: resSet(resultIndex)
+          });
+        });
+      };
+
+      requestComponent('another-response-headers-component', 0)
+      .then(requestComponent('simple-component', 1))
+      .then(function() { done(); });
+    });
+
+    //The first part of the test - checking that another-response-headers-component 
+    //should return a response with custom headers
+    it('should return 200 status code for the first component', function() {
+      expect(code[0]).to.be.equal(200);
+    });
+
+    it('should return "response-headers-component" name for the first component\'s name and request version', function() {
+      expect(response[0].name).to.equal('another-response-headers-component');
+      expect(response[0].requestVersion).to.equal('1.X.X');
+    });
+
+    it('should set response headers for the first component', function() {
+      expect(response[0].headers).to.not.be.null;      
+      expect(response[0].headers['another-test-header']).to.equal('another-test-value');
+      expect(headers[0]).to.not.be.null;      
+      expect(headers[0]['another-test-header']).to.equal('another-test-value');
+    });
+
+    //The second part of the test - validating that simple-component should not return 
+    //the custom headers previously set by another-headers-component.
+    it('should return 200 status code for the first component', function() {
+      expect(code[1]).to.be.equal(200);
+    });
+
+    it('should return "simple-component" name for the first component\'s name and request version', function() {
+      expect(response[1].name).to.equal('simple-component');
+      expect(response[1].requestVersion).to.equal('1.X.X');
+    });
+
+    it('should not set custom response', function() {
+      expect(response[1].headers).to.be.undefined;      
+      expect(headers[1]).to.be.undefined;      
+    });
+  });
+
 });