Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle fallbackRegistryUrl for ~info and ~preview #380

Merged
merged 10 commits into from
Mar 10, 2017
Merged

Handle fallbackRegistryUrl for ~info and ~preview #380

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1f23561
Initial fallback for info and preview
pbazydlo Feb 9, 2017
eeedd9d
Registry is hiding fallback registry behind itself
pbazydlo Feb 9, 2017
67d90aa
Clean up component info code
pbazydlo Feb 9, 2017
a0cb5cd
Move all of fallback logic to get-component-fallback
pbazydlo Feb 9, 2017
f46695f
Add tests for info and preview endpoint fallback
pbazydlo Feb 9, 2017
34f7b44
Fallback naming cleanup
pbazydlo Feb 9, 2017
dffbbce
Cleanup info and preview fallback
Mar 9, 2017
b2e61e6
Don't pick specific funciton in require
pbazydlo Mar 9, 2017
5f6b306
Fix tests after rebase
pbazydlo Mar 9, 2017
5041069
Change naming in fallbacks for more clarity
pbazydlo Mar 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions src/registry/domain/url-builder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,48 +4,62 @@ var querystring = require('querystring');
var url = require('url');
var _ = require('underscore');

function componentForType(component, baseUrl, type) {
if (_.isString(component)) {
component = {name: component};
}

var href = url.resolve(baseUrl, component.name) + '/';

if (!!component.version) {
href += component.version + '/';
}

href += '~' + type;

return href;
}

var build = {
component: function(component, baseUrl){
if(_.isString(component)){
component = { name: component };
component: function (component, baseUrl) {
if (_.isString(component)) {
component = {name: component};
}

var componentUrl = url.resolve(baseUrl, component.name);

if(component.version){
if (component.version) {
componentUrl += '/' + component.version;
}

componentUrl += build.queryString(component.parameters);

return componentUrl;
},
componentPreview: function(component, baseUrl){
var href = baseUrl + component.name + '/';

if(!!component.version){
href += component.version + '/';
}

href += '~preview/';

if(!!component.parameters && !_.isEmpty(component.parameters)){
href += '?' + querystring.stringify(component.parameters);
componentInfo: function (component, baseUrl) {
return componentForType(component, baseUrl, 'info');
},
componentPreview: function (component, baseUrl) {
var href = componentForType(component, baseUrl, 'preview');
if (!!component.parameters && !_.isEmpty(component.parameters)) {
href += '/?' + querystring.stringify(component.parameters);
} else {
href += '/';
}

return href;
},
queryString: function(parameters){
queryString: function (parameters) {
var qs = '';

if(_.keys(parameters).length > 0){
if (_.keys(parameters).length > 0) {
qs += '?';

_.forEach(parameters, function(parameter, key){
_.forEach(parameters, function (parameter, key) {
qs += key + '=' + encodeURIComponent(parameter) + '&';
});

if(_.keys(parameters).length > 0){
if (_.keys(parameters).length > 0) {
qs = qs.slice(0, -1);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/registry/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ module.exports.create = function(app, conf, repository){
var routes = {
component: new ComponentRoute(conf, repository),
components: new ComponentsRoute(conf, repository),
componentInfo: new ComponentInfoRoute(repository),
componentPreview: new ComponentPreviewRoute(repository),
componentInfo: new ComponentInfoRoute(conf, repository),
componentPreview: new ComponentPreviewRoute(conf, repository),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those two endpoints now need configuration for fallbackRegistryUrl

listComponents: new ListComponentsRoute(repository),
publish: new PublishRoute(repository),
staticRedirector: new StaticRedirectorRoute(repository)
Expand Down
105 changes: 63 additions & 42 deletions src/registry/routes/component-info.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,61 +4,82 @@ var parseAuthor = require('parse-author');
var _ = require('underscore');

var urlBuilder = require('../domain/url-builder');
var getComponentFallback = require('./helpers/get-component-fallback');

module.exports = function(repository){
return function(req, res){
function getParams(component) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've split this function from the main callback, because it seemed little bit hard to read.

This kind of refactor probably shouldn't be in this PR, so if it is prepared to revert this change I can make a commit for that.

var params = {};
if(!!component.oc.parameters){
var mandatoryParams = _.filter(_.keys(component.oc.parameters), function(paramName){
var param = component.oc.parameters[paramName];
return !!param.mandatory && !!param.example;
});

params = _.mapObject(_.pick(component.oc.parameters, mandatoryParams), function(param){
return param.example;
});
}

return params;
}

repository.getComponent(req.params.componentName, req.params.componentVersion, function(err, component){
function getParsedAuthor(component) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've split this function from the main callback, because it seemed little bit hard to read.

This kind of refactor probably shouldn't be in this PR, so if it is prepared to revert this change I can make a commit for that.

var author = component.author || {};
return _.isString(author) ? parseAuthor(author) : author;
}

if(err){
res.errorDetails = err;
return res.status(404).json({ err: err });
function addGetRepositoryUrlFunction(component) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've split this function from the main callback, because it seemed little bit hard to read.

This kind of refactor probably shouldn't be in this PR, so if it is prepared to revert this change I can make a commit for that.

component.getRepositoryUrl = function() {
if (_.isObject(this.repository)) {
if (this.repository.url) {
return this.repository.url;
}
}
if (_.isString(this.repository)) {
return this.repository;
}
return null;
};
}

var isHtmlRequest = !!req.headers.accept && req.headers.accept.indexOf('text/html') >= 0;
function componentInfo(err, req, res, component) {
if(err) {
res.errorDetails = err.localError;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local reference

return res.status(404).json(err);
}

if(isHtmlRequest && !!res.conf.discovery){
var isHtmlRequest = !!req.headers.accept && req.headers.accept.indexOf('text/html') >= 0;

var params = {},
author = component.author || {},
parsedAuthor = _.isString(author) ? parseAuthor(author) : author;
if(isHtmlRequest && !!res.conf.discovery){

if(!!component.oc.parameters){
var mandatoryParams = _.filter(_.keys(component.oc.parameters), function(paramName){
var param = component.oc.parameters[paramName];
return !!param.mandatory && !!param.example;
});
var params = getParams(component);
var parsedAuthor = getParsedAuthor(component);
addGetRepositoryUrlFunction(component);

params = _.mapObject(_.pick(component.oc.parameters, mandatoryParams), function(param){
return param.example;
});
}
return res.render('component-info', {
component: component,
dependencies: _.keys(component.dependencies),
href: res.conf.baseUrl,
parsedAuthor: parsedAuthor,
sandBoxDefaultQs: urlBuilder.queryString(params)
});

component.getRepositoryUrl = function() {
if (_.isObject(this.repository)) {
if (this.repository.url) {
return this.repository.url;
}
}
if (_.isString(this.repository)) {
return this.repository;
}
return null;
};
} else {
res.status(200).json(_.extend(component, {
requestVersion: req.params.componentVersion || ''
}));
}
}

return res.render('component-info', {
component: component,
dependencies: _.keys(component.dependencies),
href: res.conf.baseUrl,
parsedAuthor: parsedAuthor,
sandBoxDefaultQs: urlBuilder.queryString(params)
module.exports = function(conf, repository){
return function(req, res){
repository.getComponent(req.params.componentName, req.params.componentVersion, function(localRegistryError, localComponent){
if(localRegistryError && conf.fallbackRegistryUrl) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One little thing here in regards of naming. In many other places in the code, we refer "local" registry as registry running as oc dev, as way to differentiate it from the "default" registry running as real instance.

I think what you are calling local here can be confused with the other "local" so I think I would prefer us to change the naming. What about making local => default (so registryError, component) and the other default => fallback (so fallbackError, fallbackComponent)?

Another idea is to call the "local" "current"? Or maybe you have other ideas?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense for me

return getComponentFallback.getComponentInfo(conf, req, res, localRegistryError, function(error, component){
componentInfo(error, req, res, component);
});

} else {
res.status(200).json(_.extend(component, {
requestVersion: req.params.componentVersion || ''
}));
}

componentInfo(localRegistryError, req, res, localComponent);
});
};
};
48 changes: 30 additions & 18 deletions src/registry/routes/component-preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,46 @@
var _ = require('underscore');

var urlBuilder = require('../domain/url-builder');
var getComponentFallback = require('./helpers/get-component-fallback');

module.exports = function(repository){
return function(req, res){

repository.getComponent(req.params.componentName, req.params.componentVersion, function(err, component){

if(err){
res.errorDetails = err;
function componentPreview(err, req, res, component) {
if(err) {
res.errorDetails = err.localError;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local reference

res.errorCode = 'NOT_FOUND';
return res.status(404).json({ err: err });
}
return res.status(404).json(err);
}

var isHtmlRequest = !!req.headers.accept && req.headers.accept.indexOf('text/html') >= 0;

var isHtmlRequest = !!req.headers.accept && req.headers.accept.indexOf('text/html') >= 0;
if(isHtmlRequest && !!res.conf.discovery){

if(isHtmlRequest && !!res.conf.discovery){

return res.render('component-preview', {
component: component,
dependencies: _.keys(component.dependencies),
href: res.conf.baseUrl,
qs: urlBuilder.queryString(req.query)
component: component,
dependencies: _.keys(component.dependencies),
href: res.conf.baseUrl,
qs: urlBuilder.queryString(req.query)
});

} else {
} else {
res.status(200).json(_.extend(component, {
requestVersion: req.params.componentVersion || ''
requestVersion: req.params.componentVersion || ''
}));
}
}

module.exports = function(conf, repository){
return function(req, res){

repository.getComponent(req.params.componentName, req.params.componentVersion, function(localRegistryError, localComponent){

if(localRegistryError && conf.fallbackRegistryUrl) {
return getComponentFallback.getComponentPreview(conf, req, res, localRegistryError, function(error, component){
componentPreview(error, req, res, component);
});
}

componentPreview(localRegistryError, req, res, localComponent);

});
};
};
79 changes: 59 additions & 20 deletions src/registry/routes/helpers/get-component-fallback.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,69 @@

var request = require('minimal-request');
var url = require('url');
var urlBuilder = require('../../domain/url-builder');
var _ = require('underscore');

module.exports = function(fallbackRegistryUrl, headers, component, callback) {
function getComponentFallbackForViewType(buildUrl, conf, req, res, localRegistryError, callback) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local reference

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

damn, I should have checked.

var path = buildUrl({
name: req.params.componentName,
version: req.params.componentVersion
}, conf.fallbackRegistryUrl);

return request({
method: 'post',
url: fallbackRegistryUrl,
headers: _.extend({}, headers, {'host': url.parse(fallbackRegistryUrl).host}),
json: true,
body: {
components: [
component
]
method: 'get',
url: path,
headers: _.extend({}, req.headers, {
'host': url.parse(conf.fallbackRegistryUrl).host,
'accept': 'application/json'
})
}, function (fallbackErr, fallbackResponse) {
if (fallbackErr === 304) {
return res.status(304).send('');
}
}, function(err, res) {
if(err || !res || res.length === 0) {
return callback({
status: 404,
response: {
code: 'NOT_FOUND',
error: err
}
});

if (fallbackErr) {
return callback({localError: localRegistryError, fallbackError: fallbackErr});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another local reference

}

return callback(res[0]);
try {
return callback(null, JSON.parse(fallbackResponse));
} catch (parseError) {
return callback({localError: localRegistryError, fallbackError: 'Could not parse fallback response: ' + fallbackResponse});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local reference

}
});
};
}

module.exports = {
getComponent: function (fallbackRegistryUrl, headers, component, callback) {
return request({
method: 'post',
url: fallbackRegistryUrl,
headers: _.extend({}, headers, {'host': url.parse(fallbackRegistryUrl).host}),
json: true,
body: {
components: [
component
]
}
}, function (err, res) {
if (err || !res || res.length === 0) {
return callback({
status: 404,
response: {
code: 'NOT_FOUND',
error: err
}
});
}

return callback(res[0]);
});
},
getComponentPreview: function (conf, req, res, localRegistryError, callback) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local reference

getComponentFallbackForViewType(urlBuilder.componentPreview, conf, req, res, localRegistryError, callback);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local reference

},
getComponentInfo: function (conf, req, res, localRegistryError, callback) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local reference

getComponentFallbackForViewType(urlBuilder.componentInfo, conf, req, res, localRegistryError, callback);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local reference

}
};
2 changes: 1 addition & 1 deletion src/registry/routes/helpers/get-component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ module.exports = function(conf, repository){
// check route exist for component and version
if(err){
if(conf.fallbackRegistryUrl) {
return getComponentFallback(conf.fallbackRegistryUrl, options.headers, requestedComponent, callback);
return getComponentFallback.getComponent(conf.fallbackRegistryUrl, options.headers, requestedComponent, callback);
}

return callback({
Expand Down
Loading