0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00

Extracted an explicit "filter" parameter in UrlGenerator constructor

refs https://github.com/TryGhost/Toolbox/issues/127

- This is an effor t to define a precise set of data needed for the UrlGenerator to function, which should help with decoupling it from the frontend routes
This commit is contained in:
Naz 2021-11-15 13:22:56 +04:00
parent 176c369620
commit ca2809d432
8 changed files with 65 additions and 79 deletions

View file

@ -180,14 +180,6 @@ class ParentRouter {
return this.permalinks;
}
/**
* @description Get configured filter of this router.
* @returns {String}
*/
getFilter() {
return this.filter;
}
/**
* @description Get main route of this router.
*

View file

@ -43,7 +43,7 @@ class RouterManager {
return;
}
this.urlService.onRouterAddedType(router);
this.urlService.onRouterAddedType(router, router.filter);
}
/**

View file

@ -36,12 +36,13 @@ 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 {Object} options.queue instance of the backend Queue
* @param {Object} options.resources instance of the backend Resources
* @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, queue, resources, urls, position}) {
constructor({router, filter, queue, resources, urls, position}) {
this.router = router;
this.queue = queue;
this.urls = urls;
@ -51,8 +52,8 @@ class UrlGenerator {
debug('constructor', this.toString());
// CASE: routers can define custom filters, but not required.
if (this.router.getFilter()) {
this.filter = this.router.getFilter();
if (filter) {
this.filter = filter;
this.nql = nql(this.filter, {
expansions: EXPANSIONS,
transformer: nql.utils.mapKeyValues({

View file

@ -82,12 +82,14 @@ class UrlService {
/**
* @description Router was created, connect it with a url generator.
* @param {ExpressRouter} router
* @param {String} filter NQL filter
*/
onRouterAddedType(router) {
onRouterAddedType(router, filter) {
debug('Registering route: ', router.name);
let urlGenerator = new UrlGenerator({
router,
filter,
queue: this.queue,
resources: this.resources,
urls: this.urls,

View file

@ -39,7 +39,6 @@ describe('Integration: services/url/UrlService', function () {
urlService = new UrlService();
router1 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -49,7 +48,6 @@ describe('Integration: services/url/UrlService', function () {
};
router2 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -59,7 +57,6 @@ describe('Integration: services/url/UrlService', function () {
};
router3 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -69,7 +66,6 @@ describe('Integration: services/url/UrlService', function () {
};
router4 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -78,7 +74,7 @@ describe('Integration: services/url/UrlService', function () {
}
};
router1.getFilter.returns('featured:false');
router1.filter = 'featured:false';
router1.getResourceType.returns('posts');
router1.getPermalinks.returns({
getValue: function () {
@ -86,7 +82,6 @@ describe('Integration: services/url/UrlService', function () {
}
});
router2.getFilter.returns(false);
router2.getResourceType.returns('authors');
router2.getPermalinks.returns({
getValue: function () {
@ -94,7 +89,6 @@ describe('Integration: services/url/UrlService', function () {
}
});
router3.getFilter.returns(false);
router3.getResourceType.returns('tags');
router3.getPermalinks.returns({
getValue: function () {
@ -102,7 +96,6 @@ describe('Integration: services/url/UrlService', function () {
}
});
router4.getFilter.returns(false);
router4.getResourceType.returns('pages');
router4.getPermalinks.returns({
getValue: function () {
@ -110,10 +103,10 @@ describe('Integration: services/url/UrlService', function () {
}
});
urlService.onRouterAddedType(router1);
urlService.onRouterAddedType(router2);
urlService.onRouterAddedType(router3);
urlService.onRouterAddedType(router4);
urlService.onRouterAddedType(router1, router1.filter);
urlService.onRouterAddedType(router2, router2.filter);
urlService.onRouterAddedType(router3, router3.filter);
urlService.onRouterAddedType(router4, router4.filter);
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
urlService.init();
@ -225,7 +218,6 @@ describe('Integration: services/url/UrlService', function () {
urlService = new UrlService();
router1 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -235,7 +227,6 @@ describe('Integration: services/url/UrlService', function () {
};
router2 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -245,7 +236,6 @@ describe('Integration: services/url/UrlService', function () {
};
router3 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -255,7 +245,6 @@ describe('Integration: services/url/UrlService', function () {
};
router4 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -265,7 +254,6 @@ describe('Integration: services/url/UrlService', function () {
};
router5 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -274,7 +262,7 @@ describe('Integration: services/url/UrlService', function () {
}
};
router1.getFilter.returns('featured:true');
router1.filter = 'featured:true';
router1.getResourceType.returns('posts');
router1.getPermalinks.returns({
getValue: function () {
@ -282,7 +270,7 @@ describe('Integration: services/url/UrlService', function () {
}
});
router2.getFilter.returns('page:false');
router2.filter = 'page:false';
router2.getResourceType.returns('posts');
router2.getPermalinks.returns({
getValue: function () {
@ -290,7 +278,6 @@ describe('Integration: services/url/UrlService', function () {
}
});
router3.getFilter.returns(false);
router3.getResourceType.returns('authors');
router3.getPermalinks.returns({
getValue: function () {
@ -298,7 +285,6 @@ describe('Integration: services/url/UrlService', function () {
}
});
router4.getFilter.returns(false);
router4.getResourceType.returns('tags');
router4.getPermalinks.returns({
getValue: function () {
@ -306,19 +292,17 @@ describe('Integration: services/url/UrlService', function () {
}
});
router5.getFilter.returns(false);
router5.getResourceType.returns('pages');
router5.getPermalinks.returns({
getValue: function () {
return '/:slug/';
}
});
urlService.onRouterAddedType(router1);
urlService.onRouterAddedType(router2);
urlService.onRouterAddedType(router3);
urlService.onRouterAddedType(router4);
urlService.onRouterAddedType(router5);
urlService.onRouterAddedType(router1, router1.filter);
urlService.onRouterAddedType(router2, router2.filter);
urlService.onRouterAddedType(router3, router3.filter);
urlService.onRouterAddedType(router4, router4.filter);
urlService.onRouterAddedType(router5, router5.filter);
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
urlService.init();
@ -350,11 +334,11 @@ describe('Integration: services/url/UrlService', function () {
it('getUrl', function () {
urlService.urlGenerators.forEach(function (generator) {
if (generator.router.getResourceType() === 'posts' && generator.router.getFilter() === 'page:false') {
if (generator.router.getResourceType() === 'posts' && generator.filter === 'page:false') {
generator.getUrls().length.should.eql(2);
}
if (generator.router.getResourceType() === 'posts' && generator.router.getFilter() === 'featured:true') {
if (generator.router.getResourceType() === 'posts' && generator.filter === 'featured:true') {
generator.getUrls().length.should.eql(2);
}
@ -423,7 +407,6 @@ describe('Integration: services/url/UrlService', function () {
urlService = new UrlService();
router1 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -433,7 +416,6 @@ describe('Integration: services/url/UrlService', function () {
};
router2 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -443,7 +425,6 @@ describe('Integration: services/url/UrlService', function () {
};
router3 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -453,7 +434,6 @@ describe('Integration: services/url/UrlService', function () {
};
router4 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -463,7 +443,6 @@ describe('Integration: services/url/UrlService', function () {
};
router5 = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub(),
@ -472,7 +451,7 @@ describe('Integration: services/url/UrlService', function () {
}
};
router1.getFilter.returns('featured:false');
router1.filter = 'featured:false';
router1.getResourceType.returns('posts');
router1.getPermalinks.returns({
getValue: function () {
@ -480,7 +459,7 @@ describe('Integration: services/url/UrlService', function () {
}
});
router2.getFilter.returns('featured:true');
router2.filter = 'featured:true';
router2.getResourceType.returns('posts');
router2.getPermalinks.returns({
getValue: function () {
@ -488,7 +467,6 @@ describe('Integration: services/url/UrlService', function () {
}
});
router3.getFilter.returns(false);
router3.getResourceType.returns('authors');
router3.getPermalinks.returns({
getValue: function () {
@ -496,7 +474,6 @@ describe('Integration: services/url/UrlService', function () {
}
});
router4.getFilter.returns(false);
router4.getResourceType.returns('tags');
router4.getPermalinks.returns({
getValue: function () {
@ -504,7 +481,6 @@ describe('Integration: services/url/UrlService', function () {
}
});
router5.getFilter.returns(false);
router5.getResourceType.returns('pages');
router5.getPermalinks.returns({
getValue: function () {
@ -512,11 +488,11 @@ describe('Integration: services/url/UrlService', function () {
}
});
urlService.onRouterAddedType(router1);
urlService.onRouterAddedType(router2);
urlService.onRouterAddedType(router3);
urlService.onRouterAddedType(router4);
urlService.onRouterAddedType(router5);
urlService.onRouterAddedType(router1, router1.filter);
urlService.onRouterAddedType(router2, router2.filter);
urlService.onRouterAddedType(router3, router3.filter);
urlService.onRouterAddedType(router4, router4.filter);
urlService.onRouterAddedType(router5, router5.filter);
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
urlService.init();
@ -549,11 +525,11 @@ describe('Integration: services/url/UrlService', function () {
it('getUrl', function () {
urlService.urlGenerators.forEach(function (generator) {
if (generator.router.getResourceType() === 'posts' && generator.router.getFilter() === 'featured:false') {
if (generator.router.getResourceType() === 'posts' && generator.filter === 'featured:false') {
generator.getUrls().length.should.eql(2);
}
if (generator.router.getResourceType() === 'posts' && generator.router.getFilter() === 'featured:true') {
if (generator.router.getResourceType() === 'posts' && generator.filter === 'featured:true') {
generator.getUrls().length.should.eql(2);
}

View file

@ -39,7 +39,7 @@ describe('UNIT - services/routing/CollectionRouter', function () {
should.exist(collectionRouter.router);
should.not.exist(collectionRouter.getFilter());
should.not.exist(collectionRouter.filter);
collectionRouter.getResourceType().should.eql('posts');
collectionRouter.templates.should.eql([]);
collectionRouter.getPermalinks().getValue().should.eql('/:slug/');
@ -86,7 +86,7 @@ describe('UNIT - services/routing/CollectionRouter', function () {
should.exist(collectionRouter.router);
should.not.exist(collectionRouter.getFilter());
should.not.exist(collectionRouter.filter);
collectionRouter.getResourceType().should.eql('posts');
collectionRouter.templates.should.eql([]);
collectionRouter.getPermalinks().getValue().should.eql('/blog/:year/:slug/');
@ -116,7 +116,7 @@ describe('UNIT - services/routing/CollectionRouter', function () {
it('with custom filter', function () {
const collectionRouter = new CollectionRouter('/', {permalink: '/:slug/', filter: 'featured:true'}, RESOURCE_CONFIG, routerCreatedSpy);
collectionRouter.getFilter().should.eql('featured:true');
collectionRouter.filter.should.eql('featured:true');
});
it('with templates', function () {

View file

@ -36,7 +36,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
const staticRoutesRouter = new StaticRoutesRouter('/about/', {templates: ['test']}, routerCreatedSpy);
should.exist(staticRoutesRouter.router);
should.not.exist(staticRoutesRouter.getFilter());
should.not.exist(staticRoutesRouter.filter);
should.not.exist(staticRoutesRouter.getPermalinks());
staticRoutesRouter.templates.should.eql(['test']);
@ -60,7 +60,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
should.exist(staticRoutesRouter.router);
should.not.exist(staticRoutesRouter.getPermalinks());
should.not.exist(staticRoutesRouter.getFilter());
should.not.exist(staticRoutesRouter.filter);
staticRoutesRouter.templates.should.eql([]);
routerCreatedSpy.calledOnce.should.be.true();
@ -119,7 +119,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
should.exist(staticRoutesRouter.router);
should.not.exist(staticRoutesRouter.getPermalinks());
staticRoutesRouter.getFilter().should.eql('tag:test');
staticRoutesRouter.filter.should.eql('tag:test');
staticRoutesRouter.templates.should.eql([]);
should.exist(staticRoutesRouter.data);
@ -146,7 +146,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
should.exist(staticRoutesRouter.router);
should.not.exist(staticRoutesRouter.getPermalinks());
staticRoutesRouter.getFilter().should.eql('tag:test');
staticRoutesRouter.filter.should.eql('tag:test');
staticRoutesRouter.templates.should.eql([]);
@ -170,7 +170,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
data: {query: {}, router: {}}
}, routerCreatedSpy);
should.not.exist(staticRoutesRouter.getFilter());
should.not.exist(staticRoutesRouter.filter);
});
it('initialize on subdirectory with controller+data+filter', function () {

View file

@ -18,7 +18,6 @@ describe('Unit: services/url/UrlGenerator', function () {
};
router = {
getFilter: sinon.stub(),
addListener: sinon.stub(),
getResourceType: sinon.stub(),
getPermalinks: sinon.stub()
@ -65,8 +64,11 @@ describe('Unit: services/url/UrlGenerator', function () {
});
it('routing type has filter', function () {
router.getFilter.returns('featured:true');
const urlGenerator = new UrlGenerator({router, queue});
const urlGenerator = new UrlGenerator({
router,
filter: 'featured:true',
queue
});
urlGenerator.filter.should.eql('featured:true');
});
@ -151,7 +153,6 @@ describe('Unit: services/url/UrlGenerator', function () {
describe('fn: _try', function () {
describe('no filter', function () {
it('resource is not taken', function () {
router.getFilter.returns(false);
router.getResourceType.returns('posts');
resource.isReserved.returns(false);
@ -170,7 +171,6 @@ describe('Unit: services/url/UrlGenerator', function () {
});
it('resource is taken', function () {
router.getFilter.returns(false);
router.getResourceType.returns('posts');
resource.isReserved.returns(true);
@ -191,11 +191,16 @@ describe('Unit: services/url/UrlGenerator', function () {
describe('custom filter', function () {
it('matches', function () {
router.getFilter.returns('featured:true');
router.getResourceType.returns('posts');
resource.isReserved.returns(false);
const urlGenerator = new UrlGenerator({router, queue, resources, urls});
const urlGenerator = new UrlGenerator({
router,
filter: 'featured:true',
queue,
resources,
urls
});
sinon.stub(urlGenerator.nql, 'queryJSON').returns(true);
sinon.stub(urlGenerator, '_generateUrl').returns('something');
@ -211,11 +216,16 @@ describe('Unit: services/url/UrlGenerator', function () {
});
it('no match', function () {
router.getFilter.returns('featured:true');
router.getResourceType.returns('posts');
resource.isReserved.returns(false);
const urlGenerator = new UrlGenerator({router, queue, resources, urls});
const urlGenerator = new UrlGenerator({
router,
filter: 'featured:true',
queue,
resources,
urls
});
sinon.stub(urlGenerator.nql, 'queryJSON').returns(false);
sinon.stub(urlGenerator, '_generateUrl').returns('something');
@ -231,11 +241,16 @@ describe('Unit: services/url/UrlGenerator', function () {
});
it('resource is taken', function () {
router.getFilter.returns('featured:true');
router.getResourceType.returns('posts');
resource.isReserved.returns(true);
const urlGenerator = new UrlGenerator({router, queue, resources, urls});
const urlGenerator = new UrlGenerator({
router,
filter: 'featured:true',
queue,
resources,
urls
});
sinon.stub(urlGenerator.nql, 'queryJSON').returns(true);
sinon.stub(urlGenerator, '_generateUrl').returns('something');