Skip to content

Commit

Permalink
oc-client warmup improvements (#288)
Browse files Browse the repository at this point in the history
* Warmup error details and stuff

* Naming improvements
  • Loading branch information
matteofigus authored Sep 19, 2016
1 parent a701926 commit 47586e5
Show file tree
Hide file tree
Showing 4 changed files with 180 additions and 14 deletions.
3 changes: 2 additions & 1 deletion client/src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ module.exports = {
legacyComponent: 'The component can\'t be rendered because it was published with an older OC version',
registriesEmpty: 'registries must contain at least one endpoint',
registriesIsNotObject: 'registries must be an object',
serverSideRenderingFail: 'Server-side rendering failed: {0}'
serverSideRenderingFail: 'Server-side rendering failed: {0}',
warmupFailed: 'Error warming up oc-client: request {0} failed ({1})'
};
16 changes: 9 additions & 7 deletions client/src/utils/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@ var _ = {
}
},
eachAsync: function(obj, fn, cb){
var c = obj.length;
var callbacksLeft = obj.length;

var next = function(){
c--;
if(c === 0){
var a = cb;
var next = function(err){
callbacksLeft--;
if(callbacksLeft === 0 || !!err){

var cbCopy = cb;
cb = _.noop;
return a();

return cbCopy(err);
}
};

Expand Down Expand Up @@ -54,4 +56,4 @@ var _ = {
}
};

module.exports = _;
module.exports = _;
18 changes: 12 additions & 6 deletions client/src/warmup.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
var format = require('stringformat');
var request = require('minimal-request');

var packageInfo = require('../package');
var settings = require('./settings');
var _ = require('./utils/helpers');

module.exports = function(config, renderComponents){
Expand All @@ -29,18 +29,23 @@ module.exports = function(config, renderComponents){
toWarmup = [];

_.each(config.components, function(version, name){
urls.push(config.registries.serverRendering + name + '/' + version + '/~info');
var versionSegment = version ? (version + '/') : '';
urls.push(config.registries.serverRendering + name + '/' + versionSegment + '~info');
});

_.eachAsync(urls, function(url, next){
request({

var requestDetails = {
url: url,
json: true,
headers: options.headers,
method: 'GET',
timeout: options.timeout
}, function(err, componentInfo){
};

request(requestDetails, function(err, componentInfo){
if(err){
return cb(new Error(format('Error warming up oc-client: request to {0} failed ({1})', url, err)));
return next(new Error(format(settings.warmupFailed, JSON.stringify(requestDetails), err)));
}

var parameters = componentInfo.oc.parameters,
Expand All @@ -61,7 +66,8 @@ module.exports = function(config, renderComponents){
toWarmup.push(componentToWarmup);
next();
});
}, function(){
}, function(err){
if(err){ return cb(err); }
options.renderInfo = false;
options.container = false;

Expand Down
157 changes: 157 additions & 0 deletions test/unit/client-warmup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
'use strict';

var expect = require('chai').expect;
var injectr = require('injectr');
var sinon = require('sinon');

describe('client : warmup', function(){

var Warmup, requestStub;

var initialise = function(error, mockedResponse){

requestStub = sinon.stub().yields(error, mockedResponse || {
name: 'componentName',
version: '1.2.43',
oc: { parameters: {} }
});

Warmup = injectr('../../client/src/warmup.js', {
'minimal-request': requestStub
}, { console: console });
};

describe('when warming up the client for responsive components', function(){

var error, response, renderComponentStub;

beforeEach(function(done){
initialise();

renderComponentStub = sinon.stub().yields(null, ['hello', 'component', 'another']);

var warmup = new Warmup({
components: {
'component1': '',
'component2': '1.x.x'
},
registries: {
serverRendering: 'https://my-registry.com'
}
}, renderComponentStub);

warmup({}, function(err, res){
error = err;
response = res;
done();
});
});

it('should return no error', function(){
expect(error).to.be.null;
});

it('should make individual requests to ~info routes for each component + oc-client component', function(){
expect(requestStub.args.length).to.equal(3);
expect(requestStub.args[0][0].url).to.equal('https://my-registry.com/component1/~info');
expect(requestStub.args[1][0].url).to.equal('https://my-registry.com/component2/1.x.x/~info');
expect(requestStub.args[2][0].url).to.equal('https://my-registry.com/oc-client/~info');
});

it('should render the components', function(){
expect(renderComponentStub.args[0][0].length).to.equal(3);
});
});

describe('when warming up the client for component with parameters', function(){
var error, response, renderComponentStub;

beforeEach(function(done){
initialise(null, {
name: 'component-with-params',
version: '1.4.6',
oc: {
parameters: {
name: {
mandatory: true,
type: 'string',
example: 'John Doe',
description: 'Name'
}
}
}
});

renderComponentStub = sinon.stub().yields(null, ['hello']);

var warmup = new Warmup({
components: {
'component-with-params': ''
},
registries: {
serverRendering: 'https://my-registry.com'
}
}, renderComponentStub);

warmup({}, function(err, res){
error = err;
response = res;
done();
});
});

it('should return no error', function(){
expect(error).to.be.null;
});

it('should render the component with the mandatory parameters', function(){
expect(renderComponentStub.args[0][0][0]).to.eql({
name: 'component-with-params',
version: '1.4.6',
parameters: {
name: 'John Doe'
}
});
});
});

describe('when warming up the client for unresponsive components', function(){

var error;

beforeEach(function(done){
initialise('timeout');

var warmup = new Warmup({
components: { component1: '' },
registries: {
serverRendering: 'https://my-registry.com'
}
}, function(){});

warmup({
headers: {
'Accept-Language': 'en-US'
}
}, function(err){
error = err;
done();
});
});

it('should return an error with all the details', function(){

var expectedRequest = {
url: 'https://my-registry.com/component1/~info',
json: true,
headers: { 'Accept-Language': 'en-US' },
method: 'GET',
timeout: 5
};

var expectedError = 'Error: Error warming up oc-client: request ' + JSON.stringify(expectedRequest) + ' failed (timeout)';

expect(error.toString()).to.be.equal(expectedError);
});
});
});

0 comments on commit 47586e5

Please sign in to comment.