mirror of
https://github.com/TryGhost/Ghost.git
synced 2024-12-25 03:44:29 +03:00
Removed "router" dependency from UrlGenerator
refs https://github.com/TryGhost/Toolbox/issues/127 - Passing around whole instance of a frontend router was an overkill when there are only 3 static pieces of information that needed to be loaded. Extracting the router out makes the UrlGenerator way more readable, tests slimer, and the memory footpring of the process should be slightly lighter - The toString overloading didn't make sense at the time of this refactor, maybe if there's a concrete usecase we could resurect it in a form of passing in a router's name or something.
This commit is contained in:
Naz
parent
6e205a3f05
commit
5a62253466
@ -43,7 +43,7 @@ class RouterManager {
|
||||
return;
|
||||
}
|
||||
|
||||
this.urlService.onRouterAddedType(router, router.filter, router.getResourceType(), router.getPermalinks().getValue());
|
||||
this.urlService.onRouterAddedType(router.filter, router.getResourceType(), router.getPermalinks().getValue());
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -35,7 +35,6 @@ const EXPANSIONS = [{
|
||||
class UrlGenerator {
|
||||
/**
|
||||
* @param {Object} options
|
||||
* @param {Object} options.router instance of a frontend Routes (e.g. CollectionRouter, PreviewRouter)
|
||||
* @param {String} options.filter NQL filter string
|
||||
* @param {String} options.resourceType resource type (e.g. 'posts', 'tags')
|
||||
* @param {String} options.permalink permalink string
|
||||
@ -44,8 +43,7 @@ class UrlGenerator {
|
||||
* @param {Object} options.urls instance of the backend URLs (used to store the urls)
|
||||
* @param {Number} options.position an ID of the generator
|
||||
*/
|
||||
constructor({router, filter, resourceType, permalink, queue, resources, urls, position}) {
|
||||
this.router = router;
|
||||
constructor({filter, resourceType, permalink, queue, resources, urls, position}) {
|
||||
this.resourceType = resourceType;
|
||||
this.permalink = permalink;
|
||||
this.queue = queue;
|
||||
@ -53,8 +51,6 @@ class UrlGenerator {
|
||||
this.resources = resources;
|
||||
this.uid = position;
|
||||
|
||||
debug('constructor', this.toString());
|
||||
|
||||
// CASE: routers can define custom filters, but not required.
|
||||
if (filter) {
|
||||
this.filter = filter;
|
||||
@ -264,14 +260,6 @@ class UrlGenerator {
|
||||
getUrls() {
|
||||
return this.urls.getByGeneratorId(this.uid);
|
||||
}
|
||||
|
||||
/**
|
||||
* @description Override of `toString`
|
||||
* @returns {string}
|
||||
*/
|
||||
toString() {
|
||||
return this.router.toString();
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = UrlGenerator;
|
||||
|
||||
@ -81,16 +81,14 @@ class UrlService {
|
||||
|
||||
/**
|
||||
* @description Router was created, connect it with a url generator.
|
||||
* @param {ExpressRouter} router
|
||||
* @param {String} filter NQL filter
|
||||
* @param {String} resourceType
|
||||
* @param {String} permalink
|
||||
*/
|
||||
onRouterAddedType(router, filter, resourceType, permalink) {
|
||||
debug('Registering route: ', router.name);
|
||||
onRouterAddedType(filter, resourceType, permalink) {
|
||||
debug('Registering route: ', filter, resourceType, permalink);
|
||||
|
||||
let urlGenerator = new UrlGenerator({
|
||||
router,
|
||||
filter,
|
||||
resourceType,
|
||||
permalink,
|
||||
|
||||
@ -30,54 +30,13 @@ describe('Integration: services/url/UrlService', function () {
|
||||
});
|
||||
|
||||
describe('functional: default routing set', function () {
|
||||
let router1;
|
||||
let router2;
|
||||
let router3;
|
||||
let router4;
|
||||
|
||||
before(function (done) {
|
||||
urlService = new UrlService();
|
||||
|
||||
router1 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'post collection';
|
||||
}
|
||||
};
|
||||
|
||||
router2 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'authors';
|
||||
}
|
||||
};
|
||||
|
||||
router3 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'tags';
|
||||
}
|
||||
};
|
||||
|
||||
router4 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'static pages';
|
||||
}
|
||||
};
|
||||
|
||||
urlService.onRouterAddedType(router1, 'featured:false', 'posts', '/:slug/');
|
||||
urlService.onRouterAddedType(router2, null, 'authors', '/author/:slug/');
|
||||
urlService.onRouterAddedType(router3, null, 'tags', '/tag/:slug/');
|
||||
urlService.onRouterAddedType(router4, null, 'pages', '/:slug/');
|
||||
urlService.onRouterAddedType('featured:false', 'posts', '/:slug/');
|
||||
urlService.onRouterAddedType(null, 'authors', '/author/:slug/');
|
||||
urlService.onRouterAddedType(null, 'tags', '/tag/:slug/');
|
||||
urlService.onRouterAddedType(null, 'pages', '/:slug/');
|
||||
|
||||
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
|
||||
urlService.init();
|
||||
@ -98,29 +57,21 @@ describe('Integration: services/url/UrlService', function () {
|
||||
urlService.reset();
|
||||
});
|
||||
|
||||
it('check url generators', function () {
|
||||
urlService.urlGenerators.length.should.eql(4);
|
||||
urlService.urlGenerators[0].router.should.eql(router1);
|
||||
urlService.urlGenerators[1].router.should.eql(router2);
|
||||
urlService.urlGenerators[2].router.should.eql(router3);
|
||||
urlService.urlGenerators[3].router.should.eql(router4);
|
||||
});
|
||||
|
||||
it('getUrl', function () {
|
||||
urlService.urlGenerators.forEach(function (generator) {
|
||||
if (generator.router.getResourceType() === 'posts') {
|
||||
if (generator.resourceType === 'posts') {
|
||||
generator.getUrls().length.should.eql(2);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'pages') {
|
||||
if (generator.resourceType === 'pages') {
|
||||
generator.getUrls().length.should.eql(1);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'tags') {
|
||||
if (generator.resourceType === 'tags') {
|
||||
generator.getUrls().length.should.eql(3);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'authors') {
|
||||
if (generator.resourceType === 'authors') {
|
||||
generator.getUrls().length.should.eql(2);
|
||||
}
|
||||
});
|
||||
@ -172,12 +123,6 @@ describe('Integration: services/url/UrlService', function () {
|
||||
});
|
||||
|
||||
describe('functional: extended/modified routing set', function () {
|
||||
let router1;
|
||||
let router2;
|
||||
let router3;
|
||||
let router4;
|
||||
let router5;
|
||||
|
||||
before(testUtils.teardownDb);
|
||||
before(testUtils.setup('users:roles', 'posts'));
|
||||
|
||||
@ -188,56 +133,11 @@ describe('Integration: services/url/UrlService', function () {
|
||||
before(function (done) {
|
||||
urlService = new UrlService();
|
||||
|
||||
router1 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'post collection 1';
|
||||
}
|
||||
};
|
||||
|
||||
router2 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'post collection 2';
|
||||
}
|
||||
};
|
||||
|
||||
router3 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'authors';
|
||||
}
|
||||
};
|
||||
|
||||
router4 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'tags';
|
||||
}
|
||||
};
|
||||
|
||||
router5 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'static pages';
|
||||
}
|
||||
};
|
||||
|
||||
urlService.onRouterAddedType(router1, 'featured:true', 'posts', '/podcast/:slug/');
|
||||
urlService.onRouterAddedType(router2, 'page:false', 'posts', '/collection/:year/:slug/');
|
||||
urlService.onRouterAddedType(router3, null, 'authors', '/persons/:slug/');
|
||||
urlService.onRouterAddedType(router4, null, 'tags', '/category/:slug/');
|
||||
urlService.onRouterAddedType(router5, null, 'pages', '/:slug/');
|
||||
urlService.onRouterAddedType('featured:true', 'posts', '/podcast/:slug/');
|
||||
urlService.onRouterAddedType('page:false', 'posts', '/collection/:year/:slug/');
|
||||
urlService.onRouterAddedType(null, 'authors', '/persons/:slug/');
|
||||
urlService.onRouterAddedType(null, 'tags', '/category/:slug/');
|
||||
urlService.onRouterAddedType(null, 'pages', '/:slug/');
|
||||
|
||||
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
|
||||
urlService.init();
|
||||
@ -258,34 +158,25 @@ describe('Integration: services/url/UrlService', function () {
|
||||
urlService.resetGenerators();
|
||||
});
|
||||
|
||||
it('check url generators', function () {
|
||||
urlService.urlGenerators.length.should.eql(5);
|
||||
urlService.urlGenerators[0].router.should.eql(router1);
|
||||
urlService.urlGenerators[1].router.should.eql(router2);
|
||||
urlService.urlGenerators[2].router.should.eql(router3);
|
||||
urlService.urlGenerators[3].router.should.eql(router4);
|
||||
urlService.urlGenerators[4].router.should.eql(router5);
|
||||
});
|
||||
|
||||
it('getUrl', function () {
|
||||
urlService.urlGenerators.forEach(function (generator) {
|
||||
if (generator.router.getResourceType() === 'posts' && generator.filter === 'page:false') {
|
||||
if (generator.resourceType === 'posts' && generator.filter === 'page:false') {
|
||||
generator.getUrls().length.should.eql(2);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'posts' && generator.filter === 'featured:true') {
|
||||
if (generator.resourceType === 'posts' && generator.filter === 'featured:true') {
|
||||
generator.getUrls().length.should.eql(2);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'pages') {
|
||||
if (generator.resourceType === 'pages') {
|
||||
generator.getUrls().length.should.eql(1);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'tags') {
|
||||
if (generator.resourceType === 'tags') {
|
||||
generator.getUrls().length.should.eql(3);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'authors') {
|
||||
if (generator.resourceType === 'authors') {
|
||||
generator.getUrls().length.should.eql(2);
|
||||
}
|
||||
});
|
||||
@ -330,67 +221,16 @@ describe('Integration: services/url/UrlService', function () {
|
||||
});
|
||||
|
||||
describe('functional: subdirectory', function () {
|
||||
let router1;
|
||||
let router2;
|
||||
let router3;
|
||||
let router4;
|
||||
let router5;
|
||||
|
||||
beforeEach(function (done) {
|
||||
configUtils.set('url', 'http://localhost:2388/blog/');
|
||||
|
||||
urlService = new UrlService();
|
||||
|
||||
router1 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'post collection 1';
|
||||
}
|
||||
};
|
||||
|
||||
router2 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'post collection 2';
|
||||
}
|
||||
};
|
||||
|
||||
router3 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'authors';
|
||||
}
|
||||
};
|
||||
|
||||
router4 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'tags';
|
||||
}
|
||||
};
|
||||
|
||||
router5 = {
|
||||
addListener: sinon.stub(),
|
||||
getResourceType: sinon.stub(),
|
||||
getPermalinks: sinon.stub(),
|
||||
toString: function () {
|
||||
return 'static pages';
|
||||
}
|
||||
};
|
||||
|
||||
urlService.onRouterAddedType(router1, 'featured:false', 'posts', '/collection/:year/:slug/');
|
||||
urlService.onRouterAddedType(router2, 'featured:true', 'posts', '/podcast/:slug/');
|
||||
urlService.onRouterAddedType(router3, null, 'authors', '/persons/:slug/');
|
||||
urlService.onRouterAddedType(router4, null, 'tags', '/category/:slug/');
|
||||
urlService.onRouterAddedType(router5, null, 'pages', '/:slug/');
|
||||
urlService.onRouterAddedType('featured:false', 'posts', '/collection/:year/:slug/');
|
||||
urlService.onRouterAddedType('featured:true', 'posts', '/podcast/:slug/');
|
||||
urlService.onRouterAddedType(null, 'authors', '/persons/:slug/');
|
||||
urlService.onRouterAddedType(null, 'tags', '/category/:slug/');
|
||||
urlService.onRouterAddedType(null, 'pages', '/:slug/');
|
||||
|
||||
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
|
||||
urlService.init();
|
||||
@ -412,34 +252,25 @@ describe('Integration: services/url/UrlService', function () {
|
||||
configUtils.restore();
|
||||
});
|
||||
|
||||
it('check url generators', function () {
|
||||
urlService.urlGenerators.length.should.eql(5);
|
||||
urlService.urlGenerators[0].router.should.eql(router1);
|
||||
urlService.urlGenerators[1].router.should.eql(router2);
|
||||
urlService.urlGenerators[2].router.should.eql(router3);
|
||||
urlService.urlGenerators[3].router.should.eql(router4);
|
||||
urlService.urlGenerators[4].router.should.eql(router5);
|
||||
});
|
||||
|
||||
it('getUrl', function () {
|
||||
urlService.urlGenerators.forEach(function (generator) {
|
||||
if (generator.router.getResourceType() === 'posts' && generator.filter === 'featured:false') {
|
||||
if (generator.resourceType === 'posts' && generator.filter === 'featured:false') {
|
||||
generator.getUrls().length.should.eql(2);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'posts' && generator.filter === 'featured:true') {
|
||||
if (generator.resourceType === 'posts' && generator.filter === 'featured:true') {
|
||||
generator.getUrls().length.should.eql(2);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'pages') {
|
||||
if (generator.resourceType === 'pages') {
|
||||
generator.getUrls().length.should.eql(1);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'tags') {
|
||||
if (generator.resourceType === 'tags') {
|
||||
generator.getUrls().length.should.eql(3);
|
||||
}
|
||||
|
||||
if (generator.router.getResourceType() === 'authors') {
|
||||
if (generator.resourceType === 'authors') {
|
||||
generator.getUrls().length.should.eql(2);
|
||||
}
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user