Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-17 23:44:39 -05:00
Code Issues Activity

Extracted an explicit "resourceType" parameter in UrlGenerator constructor

Browse source 
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 17:08:22 +04:00
parent ca2809d432
commit 677ea1073d
5 changed files with 69 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
core/frontend/services/routing/router-manager.js
View file

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

12
core/server/services/url/UrlGenerator.js
View file

@ -37,13 +37,15 @@ class UrlGenerator {
* @param {Object} options * @param {Object} options
* @param {Object} options.router instance of a frontend Routes (e.g. CollectionRouter, PreviewRouter) * @param {Object} options.router instance of a frontend Routes (e.g. CollectionRouter, PreviewRouter)
* @param {String} options.filter NQL filter string * @param {String} options.filter NQL filter string
* @param {String} options.resourceType resource type (e.g. 'posts', 'tags')
* @param {Object} options.queue instance of the backend Queue * @param {Object} options.queue instance of the backend Queue
* @param {Object} options.resources instance of the backend Resources * @param {Object} options.resources instance of the backend Resources
* @param {Object} options.urls instance of the backend URLs (used to store the urls) * @param {Object} options.urls instance of the backend URLs (used to store the urls)
* @param {Number} options.position an ID of the generator * @param {Number} options.position an ID of the generator
*/ */
constructor({router, filter, queue, resources, urls, position}) { constructor({router, filter, resourceType, queue, resources, urls, position}) {
this.router = router; this.router = router;
this.resourceType = resourceType;
this.queue = queue; this.queue = queue;
this.urls = urls; this.urls = urls;
this.resources = resources; this.resources = resources;
@ -119,10 +121,10 @@ class UrlGenerator {
* @private * @private
*/ */
_onInit() { _onInit() {
debug('_onInit', this.router.getResourceType()); debug('_onInit', this.resourceType);
// @NOTE: get the resources of my type e.g. posts. // @NOTE: get the resources of my type e.g. posts.
const resources = this.resources.getAllByType(this.router.getResourceType()); const resources = this.resources.getAllByType(this.resourceType);
debug(resources.length); debug(resources.length);
@ -140,7 +142,7 @@ class UrlGenerator {
debug('onAdded', this.toString()); debug('onAdded', this.toString());
// CASE: you are type "pages", but the incoming type is "users" // CASE: you are type "pages", but the incoming type is "users"
if (event.type !== this.router.getResourceType()) { if (event.type !== this.resourceType) {
return; return;
} }
@ -223,7 +225,7 @@ class UrlGenerator {
action: 'added:' + resource.data.id, action: 'added:' + resource.data.id,
eventData: { eventData: {
id: resource.data.id, id: resource.data.id,
type: this.router.getResourceType() type: this.resourceType
} }
}); });
}; };

3
core/server/services/url/UrlService.js
View file

@ -84,12 +84,13 @@ class UrlService {
* @param {ExpressRouter} router * @param {ExpressRouter} router
* @param {String} filter NQL filter * @param {String} filter NQL filter
*/ */
onRouterAddedType(router, filter) { onRouterAddedType(router, filter, resourceType) {
debug('Registering route: ', router.name); debug('Registering route: ', router.name);
let urlGenerator = new UrlGenerator({ let urlGenerator = new UrlGenerator({
router, router,
filter, filter,
resourceType,
queue: this.queue, queue: this.queue,
resources: this.resources, resources: this.resources,
urls: this.urls, urls: this.urls,

42
test/integration/url_service.test.js
View file

@ -75,38 +75,34 @@ describe('Integration: services/url/UrlService', function () {
}; };
router1.filter = 'featured:false'; router1.filter = 'featured:false';
router1.getResourceType.returns('posts');
router1.getPermalinks.returns({ router1.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/:slug/'; return '/:slug/';
} }
}); });
router2.getResourceType.returns('authors');
router2.getPermalinks.returns({ router2.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/author/:slug/'; return '/author/:slug/';
} }
}); });
router3.getResourceType.returns('tags');
router3.getPermalinks.returns({ router3.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/tag/:slug/'; return '/tag/:slug/';
} }
}); });
router4.getResourceType.returns('pages');
router4.getPermalinks.returns({ router4.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/:slug/'; return '/:slug/';
} }
}); });
urlService.onRouterAddedType(router1, router1.filter); urlService.onRouterAddedType(router1, router1.filter, 'posts');
urlService.onRouterAddedType(router2, router2.filter); urlService.onRouterAddedType(router2, router2.filter, 'authors');
urlService.onRouterAddedType(router3, router3.filter); urlService.onRouterAddedType(router3, router3.filter, 'tags');
urlService.onRouterAddedType(router4, router4.filter); urlService.onRouterAddedType(router4, router4.filter, 'pages');
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone // We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
urlService.init(); urlService.init();
@ -263,7 +259,6 @@ describe('Integration: services/url/UrlService', function () {
}; };
router1.filter = 'featured:true'; router1.filter = 'featured:true';
router1.getResourceType.returns('posts');
router1.getPermalinks.returns({ router1.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/podcast/:slug/'; return '/podcast/:slug/';
@ -271,38 +266,34 @@ describe('Integration: services/url/UrlService', function () {
}); });
router2.filter = 'page:false'; router2.filter = 'page:false';
router2.getResourceType.returns('posts');
router2.getPermalinks.returns({ router2.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/collection/:year/:slug/'; return '/collection/:year/:slug/';
} }
}); });
router3.getResourceType.returns('authors');
router3.getPermalinks.returns({ router3.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/persons/:slug/'; return '/persons/:slug/';
} }
}); });
router4.getResourceType.returns('tags');
router4.getPermalinks.returns({ router4.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/category/:slug/'; return '/category/:slug/';
} }
}); });
router5.getResourceType.returns('pages');
router5.getPermalinks.returns({ router5.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/:slug/'; return '/:slug/';
} }
}); });
urlService.onRouterAddedType(router1, router1.filter); urlService.onRouterAddedType(router1, router1.filter, 'posts');
urlService.onRouterAddedType(router2, router2.filter); urlService.onRouterAddedType(router2, router2.filter, 'posts');
urlService.onRouterAddedType(router3, router3.filter); urlService.onRouterAddedType(router3, router3.filter, 'authors');
urlService.onRouterAddedType(router4, router4.filter); urlService.onRouterAddedType(router4, router4.filter, 'tags');
urlService.onRouterAddedType(router5, router5.filter); urlService.onRouterAddedType(router5, router5.filter, 'pages');
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone // We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
urlService.init(); urlService.init();
@ -452,7 +443,6 @@ describe('Integration: services/url/UrlService', function () {
}; };
router1.filter = 'featured:false'; router1.filter = 'featured:false';
router1.getResourceType.returns('posts');
router1.getPermalinks.returns({ router1.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/collection/:year/:slug/'; return '/collection/:year/:slug/';
@ -460,39 +450,35 @@ describe('Integration: services/url/UrlService', function () {
}); });
router2.filter = 'featured:true'; router2.filter = 'featured:true';
router2.getResourceType.returns('posts');
router2.getPermalinks.returns({ router2.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/podcast/:slug/'; return '/podcast/:slug/';
} }
}); });
router3.getResourceType.returns('authors');
router3.getPermalinks.returns({ router3.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/persons/:slug/'; return '/persons/:slug/';
} }
}); });
router4.getResourceType.returns('tags');
router4.getPermalinks.returns({ router4.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/category/:slug/'; return '/category/:slug/';
} }
}); });
router5.getResourceType.returns('pages');
router5.getPermalinks.returns({ router5.getPermalinks.returns({
getValue: function () { getValue: function () {
return '/:slug/'; return '/:slug/';
} }
}); });
urlService.onRouterAddedType(router1, router1.filter); urlService.onRouterAddedType(router1, router1.filter, 'posts');
urlService.onRouterAddedType(router2, router2.filter); urlService.onRouterAddedType(router2, router2.filter, 'posts');
urlService.onRouterAddedType(router3, router3.filter); urlService.onRouterAddedType(router3, router3.filter, 'authors');
urlService.onRouterAddedType(router4, router4.filter); urlService.onRouterAddedType(router4, router4.filter, 'tags');
urlService.onRouterAddedType(router5, router5.filter); urlService.onRouterAddedType(router5, router5.filter, 'pages');
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone // We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
urlService.init(); urlService.init();

60
test/unit/frontend/services/url/UrlGenerator.test.js
View file

@ -105,10 +105,15 @@ describe('Unit: services/url/UrlGenerator', function () {
describe('fn: _onInit', function () { describe('fn: _onInit', function () {
it('1 resource', function () { it('1 resource', function () {
router.getResourceType.returns('posts');
resources.getAllByType.withArgs('posts').returns([resource]); resources.getAllByType.withArgs('posts').returns([resource]);
const urlGenerator = new UrlGenerator({router, queue, resources, urls}); const urlGenerator = new UrlGenerator({
router,
resourceType: 'posts',
queue,
resources,
urls
});
sinon.stub(urlGenerator, '_try'); sinon.stub(urlGenerator, '_try');
urlGenerator._onInit(); urlGenerator._onInit();
@ -116,10 +121,15 @@ describe('Unit: services/url/UrlGenerator', function () {
}); });
it('no resource', function () { it('no resource', function () {
router.getResourceType.returns('posts');
resources.getAllByType.withArgs('posts').returns([]); resources.getAllByType.withArgs('posts').returns([]);
const urlGenerator = new UrlGenerator({router, queue, resources, urls}); const urlGenerator = new UrlGenerator({
router,
resourceType: 'posts',
queue,
resources,
urls
});
sinon.stub(urlGenerator, '_try'); sinon.stub(urlGenerator, '_try');
urlGenerator._onInit(); urlGenerator._onInit();
@ -129,10 +139,15 @@ describe('Unit: services/url/UrlGenerator', function () {
describe('fn: _onAdded', function () { describe('fn: _onAdded', function () {
it('type is equal', function () { it('type is equal', function () {
router.getResourceType.returns('posts');
resources.getByIdAndType.withArgs('posts', 1).returns(resource); resources.getByIdAndType.withArgs('posts', 1).returns(resource);
const urlGenerator = new UrlGenerator({router, queue, resources, urls}); const urlGenerator = new UrlGenerator({
router,
resourceType: 'posts',
queue,
resources,
urls
});
sinon.stub(urlGenerator, '_try'); sinon.stub(urlGenerator, '_try');
urlGenerator._onAdded({id: 1, type: 'posts'}); urlGenerator._onAdded({id: 1, type: 'posts'});
@ -140,9 +155,13 @@ describe('Unit: services/url/UrlGenerator', function () {
}); });
it('type is not equal', function () { it('type is not equal', function () {
router.getResourceType.returns('pages'); const urlGenerator = new UrlGenerator({
router,
const urlGenerator = new UrlGenerator({router, queue, resources, urls}); resourceType: 'pages',
queue,
resources,
urls
});
sinon.stub(urlGenerator, '_try'); sinon.stub(urlGenerator, '_try');
urlGenerator._onAdded({id: 1, type: 'posts'}); urlGenerator._onAdded({id: 1, type: 'posts'});
@ -153,10 +172,15 @@ describe('Unit: services/url/UrlGenerator', function () {
describe('fn: _try', function () { describe('fn: _try', function () {
describe('no filter', function () { describe('no filter', function () {
it('resource is not taken', function () { it('resource is not taken', function () {
router.getResourceType.returns('posts');
resource.isReserved.returns(false); resource.isReserved.returns(false);
const urlGenerator = new UrlGenerator({router, queue, resources, urls}); const urlGenerator = new UrlGenerator({
router,
resourceType: 'posts',
queue,
resources,
urls
});
should.not.exist(urlGenerator.nql); should.not.exist(urlGenerator.nql);
sinon.stub(urlGenerator, '_generateUrl').returns('something'); sinon.stub(urlGenerator, '_generateUrl').returns('something');
@ -174,7 +198,13 @@ describe('Unit: services/url/UrlGenerator', function () {
router.getResourceType.returns('posts'); router.getResourceType.returns('posts');
resource.isReserved.returns(true); resource.isReserved.returns(true);
const urlGenerator = new UrlGenerator({router, queue, resources, urls}); const urlGenerator = new UrlGenerator({
router,
resourceType: 'posts',
queue,
resources,
urls
});
should.not.exist(urlGenerator.nql); should.not.exist(urlGenerator.nql);
sinon.stub(urlGenerator, '_generateUrl').returns('something'); sinon.stub(urlGenerator, '_generateUrl').returns('something');
@ -191,12 +221,12 @@ describe('Unit: services/url/UrlGenerator', function () {
describe('custom filter', function () { describe('custom filter', function () {
it('matches', function () { it('matches', function () {
router.getResourceType.returns('posts');
resource.isReserved.returns(false); resource.isReserved.returns(false);
const urlGenerator = new UrlGenerator({ const urlGenerator = new UrlGenerator({
router, router,
filter: 'featured:true', filter: 'featured:true',
resourceType: 'posts',
queue, queue,
resources, resources,
urls urls
@ -216,12 +246,12 @@ describe('Unit: services/url/UrlGenerator', function () {
}); });
it('no match', function () { it('no match', function () {
router.getResourceType.returns('posts');
resource.isReserved.returns(false); resource.isReserved.returns(false);
const urlGenerator = new UrlGenerator({ const urlGenerator = new UrlGenerator({
router, router,
filter: 'featured:true', filter: 'featured:true',
resourceType: 'posts',
queue, queue,
resources, resources,
urls urls
@ -241,12 +271,12 @@ describe('Unit: services/url/UrlGenerator', function () {
}); });
it('resource is taken', function () { it('resource is taken', function () {
router.getResourceType.returns('posts');
resource.isReserved.returns(true); resource.isReserved.returns(true);
const urlGenerator = new UrlGenerator({ const urlGenerator = new UrlGenerator({
router, router,
filter: 'featured:true', filter: 'featured:true',
resourceType: 'posts',
queue, queue,
resources, resources,
urls urls