Skip to content

Commit

Permalink
333 - Fixed the logic for setting the custom headers in case of POST …
Browse files Browse the repository at this point in the history 
…and GET requests

- for GET it will put the custom headers in the HTTP response headers
- for POST it will put the custom headers in the response body
- the setCustomHeaders was moved out of component.js to get-component.js under
  the name filterCustomHeaders. It's because in case of POST request the code
  path is different and goes to component(s).js instead of component.js. Now we
  do the filtering in one and the same place.
- fixed a bug where we used to not take into account the custom headers in
  case of a unrendered component. For this reason the actual setting of
  response headers was moved from line 205 up to line 175.
- fixed the registry acceptance tests. In their former variant when the GET
  tests passes they were used to leave the customHeadersToSkipOnWeakVersion
  settings so the POST tests were starting directly with this setting on.
  That's why new before/after sections were defined to reinitialize the
  registry before the tests and then recover it after them.
  • Loading branch information
@i-b-dimitrov
i-b-dimitrov committed Jan 17, 2017
1 parent 8c4d71f commit b2f17e4
Show file tree
Hide file tree
Showing 4 changed files with 177 additions and 90 deletions.
22 changes: 6 additions & 16 deletions src/registry/routes/component.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,6 @@ module.exports = function(conf, repository){

var getComponent = new GetComponentHelper(conf, repository);

var setCustomHeaders = function(req, res, componentResponse) {
var headersToSet = componentResponse.headers;

if (req.params.componentVersion !== componentResponse.version &&
!_.isEmpty(res.conf.customHeadersToSkipOnWeakVersion))
{
headersToSet = _.omit(headersToSet, res.conf.customHeadersToSkipOnWeakVersion);
}

if (!_.isEmpty(headersToSet)) {
res.set(headersToSet);
}
};

return function(req, res){
getComponent({
conf: res.conf,
Expand All @@ -34,8 +20,12 @@ module.exports = function(conf, repository){
res.errorDetails = result.response.error;
}

if (result.response.headers) {
setCustomHeaders(req, res, result.response);
if (!_.isEmpty(result.response.headers)) {
res.set(result.response.headers);

if (req.method === 'GET') {
delete result.response.headers;
}
}

return res.json(result.status, result.response);
Expand Down
21 changes: 17 additions & 4 deletions src/registry/routes/helpers/get-component.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,16 @@ module.exports = function(conf, repository){
});
}

var filterCustomHeaders = function(headers, requestedVersion, actualVersion) {
if (!_.isEmpty(headers) &&
!_.isEmpty(conf.customHeadersToSkipOnWeakVersion) &&
requestedVersion !== actualVersion)
{
headers = _.omit(headers, conf.customHeadersToSkipOnWeakVersion);
}
return headers;
};

var returnComponent = function(err, data){

if(componentCallbackDone){ return; }
Expand Down Expand Up @@ -162,6 +172,13 @@ module.exports = function(conf, repository){
renderMode: renderMode
});

if (responseHeaders) {
responseHeaders = filterCustomHeaders(responseHeaders, requestedComponent.version, component.version);
if (!_.isEmpty(responseHeaders)) {
response.headers = responseHeaders;
}
}

if (isUnrendered) {
callback({
status: 200,
Expand Down Expand Up @@ -202,10 +219,6 @@ module.exports = function(conf, repository){
});
}

if (responseHeaders) {
response.headers = responseHeaders;
}

callback({
status: 200,
response: _.extend(response, { html: html })
Expand Down
217 changes: 151 additions & 66 deletions test/acceptance/registry.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var expect = require('chai').expect;
var path = require('path');
var request = require('minimal-request');
var url = require('url');
var _ = require('underscore');

describe('registry', function(){

Expand All @@ -15,16 +16,16 @@ describe('registry', function(){
conf;

var next = function(done){
return function(e, r, h){
return function(e, r, d){
error = e;
result = r;
headers = h;
headers = d.response.headers;
done();
};
};

before(function(done){
conf = {
var getDefaultTestConfiguration = function() {
return {
local: true,
path: path.resolve('test/fixtures/components'),
port: 3030,
Expand All @@ -33,9 +34,15 @@ describe('registry', function(){
verbosity: 0,
dependencies: ['underscore']
};
};

var initializeRegistry = function(configuration, cb) {
registry = new oc.Registry(configuration);
registry.start(cb);
};

registry = new oc.Registry(conf);
registry.start(done);
before(function(done){
initializeRegistry(getDefaultTestConfiguration(), done);
});

after(function(done){ registry.close(done); });
Expand All @@ -53,48 +60,9 @@ describe('registry', function(){

describe('GET /hello-world-custom-headers', function() {

var httpGetRequest = function(options, callback) {
var callbackDone = false;
var requestData = url.parse(options.url);
requestData.headers = options.headers || {};
requestData.method = 'get';

var req = require('http').request(requestData).on('response', function(response) {
var body = [];

response.on('data', function(chunk){
body.push(chunk);
}).on('end', function(){
body = Buffer.concat(body);

if(!callbackDone){
callbackDone = true;
var error = response.statusCode !== 200 ? response.statusCode : null;

if (options.json) {
try {
callback(error, JSON.parse(body), response.headers);
} catch(e){
return callback('json parsing error');
}
} else {
callback(error, body, response.headers);
}
}
});
}).on('error', function(e){
if(!callbackDone){
callbackDone = true;
callback(e);
}
});

req.end();
};

describe('with the default configuration (no customHeadersToSkipOnWeakVersion defined) and strong version 1.0.0', function() {
before(function(done) {
httpGetRequest({
request({
url: 'http://localhost:3030/hello-world-custom-headers/1.0.0',
json: true
}, next(done));
Expand All @@ -103,15 +71,15 @@ describe('registry', function(){
it('should return the component with custom headers', function() {
expect(result.version).to.equal('1.0.0');
expect(result.name).to.equal('hello-world-custom-headers');
expect(result.headers).to.eql({'cache-control': 'public max-age=3600', 'test-header': 'Test-Value'});
expect(result.headers).to.be.undefined;
expect(headers).to.have.property('cache-control', 'public max-age=3600');
expect(headers).to.have.property('test-header', 'Test-Value');
});
});

describe('with the default configuration (no customHeadersToSkipOnWeakVersion defined) and weak version 1.x.x', function() {
before(function(done) {
httpGetRequest({
request({
url: 'http://localhost:3030/hello-world-custom-headers/1.x.x',
json: true
}, next(done));
Expand All @@ -120,7 +88,7 @@ describe('registry', function(){
it('should return the component with custom headers', function() {
expect(result.version).to.equal('1.0.0');
expect(result.name).to.equal('hello-world-custom-headers');
expect(result.headers).to.eql({'cache-control': 'public max-age=3600', 'test-header': 'Test-Value'});
expect(result.headers).to.be.undefined;
expect(headers).to.have.property('cache-control', 'public max-age=3600');
expect(headers).to.have.property('test-header', 'Test-Value');
});
Expand All @@ -129,25 +97,18 @@ describe('registry', function(){
describe('with a custom configuration with customHeadersToSkipOnWeakVersion defined', function() {
before(function(done) {
registry.close();
initializeRegistry(_.extend(getDefaultTestConfiguration(), {customHeadersToSkipOnWeakVersion: ['Cache-Control']}), done);
});

conf = {
local: true,
path: path.resolve('test/fixtures/components'),
port: 3030,
baseUrl: 'http://localhost:3030/',
env: { name: 'local' },
verbosity: 0,
dependencies: ['underscore'],
customHeadersToSkipOnWeakVersion: ['Cache-Control']
};

registry = new oc.Registry(conf);
registry.start(done);
after(function(done){
registry.close(function() {
initializeRegistry(getDefaultTestConfiguration(), done);
});
});

describe('when strong version is requested 1.0.0', function() {
before(function(done) {
httpGetRequest({
request({
url: 'http://localhost:3030/hello-world-custom-headers/1.0.0',
json: true
}, next(done));
Expand All @@ -156,15 +117,15 @@ describe('registry', function(){
it('should return the component with the custom headers', function() {
expect(result.version).to.equal('1.0.0');
expect(result.name).to.equal('hello-world-custom-headers');
expect(result.headers).to.eql({'cache-control': 'public max-age=3600', 'test-header': 'Test-Value'});
expect(result.headers).to.be.undefined;
expect(headers).to.have.property('cache-control', 'public max-age=3600');
expect(headers).to.have.property('test-header', 'Test-Value');
});
});

describe('when weak version is requested 1.x.x', function() {
before(function(done) {
httpGetRequest({
request({
url: 'http://localhost:3030/hello-world-custom-headers/1.x.x',
json: true
}, next(done));
Expand All @@ -173,14 +134,138 @@ describe('registry', function(){
it('should skip Cache-Control header', function() {
expect(result.version).to.equal('1.0.0');
expect(result.name).to.equal('hello-world-custom-headers');
expect(result.headers).to.eql({'cache-control': 'public max-age=3600', 'test-header': 'Test-Value'});
expect(result.headers).to.be.undefined;
expect(headers).to.not.have.property('cache-control');
expect(headers).to.have.property('test-header', 'Test-Value');
});
});
});
});

describe('POST /hello-world-custom-headers', function() {

describe('with the default configuration (no customHeadersToSkipOnWeakVersion defined) and strong version 1.0.0', function() {
before(function(done) {
request({
url: 'http://localhost:3030',
json: true,
method: 'post',
body: {
components: [{
name: 'hello-world-custom-headers',
version: '1.0.0'
}]
}
}, next(done));
});

it('should not set HTTP custom headers', function() {
expect(headers).to.not.have.property('cache-control');
expect(headers).to.not.have.property('test-header');
});

it('should return the component with custom headers', function() {
expect(result[0].response.version).to.equal('1.0.0');
expect(result[0].response.name).to.equal('hello-world-custom-headers');
expect(result[0].response.headers).to.be.deep.equal({'cache-control': 'public max-age=3600', 'test-header': 'Test-Value'});
});
});

describe('with the default configuration (no customHeadersToSkipOnWeakVersion defined) and weak version 1.x.x', function() {
before(function(done) {
request({
url: 'http://localhost:3030',
json: true,
method: 'post',
body: {
components: [{
name: 'hello-world-custom-headers',
version: '1.x.x'
}]
}
}, next(done));
});

it('should not set HTTP custom headers', function() {
expect(headers).to.not.have.property('cache-control');
expect(headers).to.not.have.property('test-header');
});

it('should return the component with custom headers in the response body', function() {
expect(result[0].response.version).to.equal('1.0.0');
expect(result[0].response.name).to.equal('hello-world-custom-headers');
expect(result[0].response.headers).to.be.deep.equal({'cache-control': 'public max-age=3600', 'test-header': 'Test-Value'});
});
});

describe('with a custom configuration with customHeadersToSkipOnWeakVersion defined', function() {
before(function(done) {
registry.close();
initializeRegistry(_.extend(getDefaultTestConfiguration(), {customHeadersToSkipOnWeakVersion: ['Cache-Control']}), done);
});

after(function(done){
registry.close(function() {
initializeRegistry(getDefaultTestConfiguration(), done);
});
});

describe('when strong version is requested 1.0.0', function() {
before(function(done) {
request({
url: 'http://localhost:3030',
json: true,
method: 'post',
body: {
components: [{
name: 'hello-world-custom-headers',
version: '1.0.0'
}]
}
}, next(done));
});

it('should not set HTTP custom headers', function() {
expect(headers).to.not.have.property('cache-control');
expect(headers).to.not.have.property('test-header');
});

it('should return the component with the custom headers', function() {
expect(result[0].response.version).to.equal('1.0.0');
expect(result[0].response.name).to.equal('hello-world-custom-headers');
expect(result[0].response.headers).to.be.deep.equal({'cache-control': 'public max-age=3600', 'test-header': 'Test-Value'});
});
});

describe('when weak version is requested 1.x.x', function() {
before(function(done) {
request({
url: 'http://localhost:3030',
json: true,
method: 'post',
body: {
components: [{
name: 'hello-world-custom-headers',
version: '1.x.x'
}]
}
}, next(done));
});

it('should not set HTTP custom headers', function() {
expect(headers).to.not.have.property('cache-control');
expect(headers).to.not.have.property('test-header');
});

it('should skip Cache-Control header', function() {
expect(result[0].response.version).to.equal('1.0.0');
expect(result[0].response.name).to.equal('hello-world-custom-headers');
expect(result[0].response.headers).to.be.deep.equal({'test-header': 'Test-Value'});
});
});
});
});

describe('GET /', function(){

before(function(done){
Expand Down
Loading

0 comments on commit b2f17e4

Please sign in to comment.