From 677ea1073d0399976bcb65de46fa9b71c01dcf93 Mon Sep 17 00:00:00 2001 From: Naz Date: Mon, 15 Nov 2021 17:08:22 +0400 Subject: [PATCH] Extracted an explicit "resourceType" 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 --- .../services/routing/router-manager.js | 2 +- core/server/services/url/UrlGenerator.js | 12 ++-- core/server/services/url/UrlService.js | 3 +- test/integration/url_service.test.js | 42 +++++-------- .../services/url/UrlGenerator.test.js | 60 ++++++++++++++----- 5 files changed, 69 insertions(+), 50 deletions(-) diff --git a/core/frontend/services/routing/router-manager.js b/core/frontend/services/routing/router-manager.js index 62598d073e..298b80db89 100644 --- a/core/frontend/services/routing/router-manager.js +++ b/core/frontend/services/routing/router-manager.js @@ -43,7 +43,7 @@ class RouterManager { return; } - this.urlService.onRouterAddedType(router, router.filter); + this.urlService.onRouterAddedType(router, router.filter, router.getResourceType()); } /** diff --git a/core/server/services/url/UrlGenerator.js b/core/server/services/url/UrlGenerator.js index 96e22f7b09..1fef4c50f6 100644 --- a/core/server/services/url/UrlGenerator.js +++ b/core/server/services/url/UrlGenerator.js @@ -37,13 +37,15 @@ 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 {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, filter, queue, resources, urls, position}) { + constructor({router, filter, resourceType, queue, resources, urls, position}) { this.router = router; + this.resourceType = resourceType; this.queue = queue; this.urls = urls; this.resources = resources; @@ -119,10 +121,10 @@ class UrlGenerator { * @private */ _onInit() { - debug('_onInit', this.router.getResourceType()); + debug('_onInit', this.resourceType); // @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); @@ -140,7 +142,7 @@ class UrlGenerator { debug('onAdded', this.toString()); // CASE: you are type "pages", but the incoming type is "users" - if (event.type !== this.router.getResourceType()) { + if (event.type !== this.resourceType) { return; } @@ -223,7 +225,7 @@ class UrlGenerator { action: 'added:' + resource.data.id, eventData: { id: resource.data.id, - type: this.router.getResourceType() + type: this.resourceType } }); }; diff --git a/core/server/services/url/UrlService.js b/core/server/services/url/UrlService.js index e3b3419306..50a3456889 100644 --- a/core/server/services/url/UrlService.js +++ b/core/server/services/url/UrlService.js @@ -84,12 +84,13 @@ class UrlService { * @param {ExpressRouter} router * @param {String} filter NQL filter */ - onRouterAddedType(router, filter) { + onRouterAddedType(router, filter, resourceType) { debug('Registering route: ', router.name); let urlGenerator = new UrlGenerator({ router, filter, + resourceType, queue: this.queue, resources: this.resources, urls: this.urls, diff --git a/test/integration/url_service.test.js b/test/integration/url_service.test.js index 1af8f21ad1..c5de0f0335 100644 --- a/test/integration/url_service.test.js +++ b/test/integration/url_service.test.js @@ -75,38 +75,34 @@ describe('Integration: services/url/UrlService', function () { }; router1.filter = 'featured:false'; - router1.getResourceType.returns('posts'); router1.getPermalinks.returns({ getValue: function () { return '/:slug/'; } }); - router2.getResourceType.returns('authors'); router2.getPermalinks.returns({ getValue: function () { return '/author/:slug/'; } }); - router3.getResourceType.returns('tags'); router3.getPermalinks.returns({ getValue: function () { return '/tag/:slug/'; } }); - router4.getResourceType.returns('pages'); router4.getPermalinks.returns({ getValue: function () { return '/:slug/'; } }); - urlService.onRouterAddedType(router1, router1.filter); - urlService.onRouterAddedType(router2, router2.filter); - urlService.onRouterAddedType(router3, router3.filter); - urlService.onRouterAddedType(router4, router4.filter); + urlService.onRouterAddedType(router1, router1.filter, 'posts'); + urlService.onRouterAddedType(router2, router2.filter, 'authors'); + urlService.onRouterAddedType(router3, router3.filter, 'tags'); + 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 urlService.init(); @@ -263,7 +259,6 @@ describe('Integration: services/url/UrlService', function () { }; router1.filter = 'featured:true'; - router1.getResourceType.returns('posts'); router1.getPermalinks.returns({ getValue: function () { return '/podcast/:slug/'; @@ -271,38 +266,34 @@ describe('Integration: services/url/UrlService', function () { }); router2.filter = 'page:false'; - router2.getResourceType.returns('posts'); router2.getPermalinks.returns({ getValue: function () { return '/collection/:year/:slug/'; } }); - router3.getResourceType.returns('authors'); router3.getPermalinks.returns({ getValue: function () { return '/persons/:slug/'; } }); - router4.getResourceType.returns('tags'); router4.getPermalinks.returns({ getValue: function () { return '/category/:slug/'; } }); - router5.getResourceType.returns('pages'); router5.getPermalinks.returns({ getValue: function () { return '/:slug/'; } }); - 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); + urlService.onRouterAddedType(router1, router1.filter, 'posts'); + urlService.onRouterAddedType(router2, router2.filter, 'posts'); + urlService.onRouterAddedType(router3, router3.filter, 'authors'); + urlService.onRouterAddedType(router4, router4.filter, 'tags'); + 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 urlService.init(); @@ -452,7 +443,6 @@ describe('Integration: services/url/UrlService', function () { }; router1.filter = 'featured:false'; - router1.getResourceType.returns('posts'); router1.getPermalinks.returns({ getValue: function () { return '/collection/:year/:slug/'; @@ -460,39 +450,35 @@ describe('Integration: services/url/UrlService', function () { }); router2.filter = 'featured:true'; - router2.getResourceType.returns('posts'); router2.getPermalinks.returns({ getValue: function () { return '/podcast/:slug/'; } }); - router3.getResourceType.returns('authors'); router3.getPermalinks.returns({ getValue: function () { return '/persons/:slug/'; } }); - router4.getResourceType.returns('tags'); router4.getPermalinks.returns({ getValue: function () { return '/category/:slug/'; } }); - router5.getResourceType.returns('pages'); router5.getPermalinks.returns({ getValue: function () { return '/:slug/'; } }); - 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); + urlService.onRouterAddedType(router1, router1.filter, 'posts'); + urlService.onRouterAddedType(router2, router2.filter, 'posts'); + urlService.onRouterAddedType(router3, router3.filter, 'authors'); + urlService.onRouterAddedType(router4, router4.filter, 'tags'); + 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 urlService.init(); diff --git a/test/unit/frontend/services/url/UrlGenerator.test.js b/test/unit/frontend/services/url/UrlGenerator.test.js index 49f7b60d2f..8fed324076 100644 --- a/test/unit/frontend/services/url/UrlGenerator.test.js +++ b/test/unit/frontend/services/url/UrlGenerator.test.js @@ -105,10 +105,15 @@ describe('Unit: services/url/UrlGenerator', function () { describe('fn: _onInit', function () { it('1 resource', function () { - router.getResourceType.returns('posts'); 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'); urlGenerator._onInit(); @@ -116,10 +121,15 @@ describe('Unit: services/url/UrlGenerator', function () { }); it('no resource', function () { - router.getResourceType.returns('posts'); 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'); urlGenerator._onInit(); @@ -129,10 +139,15 @@ describe('Unit: services/url/UrlGenerator', function () { describe('fn: _onAdded', function () { it('type is equal', function () { - router.getResourceType.returns('posts'); 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'); urlGenerator._onAdded({id: 1, type: 'posts'}); @@ -140,9 +155,13 @@ describe('Unit: services/url/UrlGenerator', function () { }); it('type is not equal', function () { - router.getResourceType.returns('pages'); - - const urlGenerator = new UrlGenerator({router, queue, resources, urls}); + const urlGenerator = new UrlGenerator({ + router, + resourceType: 'pages', + queue, + resources, + urls + }); sinon.stub(urlGenerator, '_try'); urlGenerator._onAdded({id: 1, type: 'posts'}); @@ -153,10 +172,15 @@ describe('Unit: services/url/UrlGenerator', function () { describe('fn: _try', function () { describe('no filter', function () { it('resource is not taken', function () { - router.getResourceType.returns('posts'); 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); sinon.stub(urlGenerator, '_generateUrl').returns('something'); @@ -174,7 +198,13 @@ describe('Unit: services/url/UrlGenerator', function () { router.getResourceType.returns('posts'); 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); sinon.stub(urlGenerator, '_generateUrl').returns('something'); @@ -191,12 +221,12 @@ describe('Unit: services/url/UrlGenerator', function () { describe('custom filter', function () { it('matches', function () { - router.getResourceType.returns('posts'); resource.isReserved.returns(false); const urlGenerator = new UrlGenerator({ router, filter: 'featured:true', + resourceType: 'posts', queue, resources, urls @@ -216,12 +246,12 @@ describe('Unit: services/url/UrlGenerator', function () { }); it('no match', function () { - router.getResourceType.returns('posts'); resource.isReserved.returns(false); const urlGenerator = new UrlGenerator({ router, filter: 'featured:true', + resourceType: 'posts', queue, resources, urls @@ -241,12 +271,12 @@ describe('Unit: services/url/UrlGenerator', function () { }); it('resource is taken', function () { - router.getResourceType.returns('posts'); resource.isReserved.returns(true); const urlGenerator = new UrlGenerator({ router, filter: 'featured:true', + resourceType: 'posts', queue, resources, urls