From 6e205a3f0594414ba4cbcad79d2d3a4e0cf063ae Mon Sep 17 00:00:00 2001 From: Naz Date: Mon, 15 Nov 2021 17:59:40 +0400 Subject: [PATCH] Extracted an explicit "permalink" 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 is almost the last piece to free us up from the massive "router" object that has been passed around --- .../services/routing/router-manager.js | 2 +- core/server/services/url/UrlGenerator.js | 7 +- core/server/services/url/UrlService.js | 5 +- test/integration/url_service.test.js | 116 +++--------------- .../services/url/UrlGenerator.test.js | 12 +- 5 files changed, 29 insertions(+), 113 deletions(-) diff --git a/core/frontend/services/routing/router-manager.js b/core/frontend/services/routing/router-manager.js index 298b80db89..8353226e97 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, router.getResourceType()); + this.urlService.onRouterAddedType(router, router.filter, router.getResourceType(), router.getPermalinks().getValue()); } /** diff --git a/core/server/services/url/UrlGenerator.js b/core/server/services/url/UrlGenerator.js index 1fef4c50f6..0554f58f80 100644 --- a/core/server/services/url/UrlGenerator.js +++ b/core/server/services/url/UrlGenerator.js @@ -38,14 +38,16 @@ class UrlGenerator { * @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 * @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, resourceType, queue, resources, urls, position}) { + constructor({router, filter, resourceType, permalink, queue, resources, urls, position}) { this.router = router; this.resourceType = resourceType; + this.permalink = permalink; this.queue = queue; this.urls = urls; this.resources = resources; @@ -193,8 +195,7 @@ class UrlGenerator { * @NOTE We currently generate relative urls (https://github.com/TryGhost/Ghost/commit/7b0d5d465ba41073db0c3c72006da625fa11df32). */ _generateUrl(resource) { - const permalink = this.router.getPermalinks().getValue(); - return localUtils.replacePermalink(permalink, resource.data); + return localUtils.replacePermalink(this.permalink, resource.data); } /** diff --git a/core/server/services/url/UrlService.js b/core/server/services/url/UrlService.js index 50a3456889..18abd0957d 100644 --- a/core/server/services/url/UrlService.js +++ b/core/server/services/url/UrlService.js @@ -83,14 +83,17 @@ 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) { + onRouterAddedType(router, filter, resourceType, permalink) { debug('Registering route: ', router.name); let urlGenerator = new UrlGenerator({ router, filter, resourceType, + permalink, 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 c5de0f0335..a58871d3f9 100644 --- a/test/integration/url_service.test.js +++ b/test/integration/url_service.test.js @@ -74,35 +74,10 @@ describe('Integration: services/url/UrlService', function () { } }; - router1.filter = 'featured:false'; - router1.getPermalinks.returns({ - getValue: function () { - return '/:slug/'; - } - }); - - router2.getPermalinks.returns({ - getValue: function () { - return '/author/:slug/'; - } - }); - - router3.getPermalinks.returns({ - getValue: function () { - return '/tag/:slug/'; - } - }); - - router4.getPermalinks.returns({ - getValue: function () { - return '/:slug/'; - } - }); - - urlService.onRouterAddedType(router1, router1.filter, 'posts'); - urlService.onRouterAddedType(router2, router2.filter, 'authors'); - urlService.onRouterAddedType(router3, router3.filter, 'tags'); - urlService.onRouterAddedType(router4, router4.filter, '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/'); // We can't use our url service utils here, because this is a local copy of the urlService, not the singletone urlService.init(); @@ -258,42 +233,11 @@ describe('Integration: services/url/UrlService', function () { } }; - router1.filter = 'featured:true'; - router1.getPermalinks.returns({ - getValue: function () { - return '/podcast/:slug/'; - } - }); - - router2.filter = 'page:false'; - router2.getPermalinks.returns({ - getValue: function () { - return '/collection/:year/:slug/'; - } - }); - - router3.getPermalinks.returns({ - getValue: function () { - return '/persons/:slug/'; - } - }); - - router4.getPermalinks.returns({ - getValue: function () { - return '/category/:slug/'; - } - }); - - router5.getPermalinks.returns({ - getValue: function () { - return '/:slug/'; - } - }); - 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'); + 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/'); // We can't use our url service utils here, because this is a local copy of the urlService, not the singletone urlService.init(); @@ -442,43 +386,11 @@ describe('Integration: services/url/UrlService', function () { } }; - router1.filter = 'featured:false'; - router1.getPermalinks.returns({ - getValue: function () { - return '/collection/:year/:slug/'; - } - }); - - router2.filter = 'featured:true'; - router2.getPermalinks.returns({ - getValue: function () { - return '/podcast/:slug/'; - } - }); - - router3.getPermalinks.returns({ - getValue: function () { - return '/persons/:slug/'; - } - }); - - router4.getPermalinks.returns({ - getValue: function () { - return '/category/:slug/'; - } - }); - - router5.getPermalinks.returns({ - getValue: function () { - return '/:slug/'; - } - }); - - 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'); + 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/'); // 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 8fed324076..0237c9f7ce 100644 --- a/test/unit/frontend/services/url/UrlGenerator.test.js +++ b/test/unit/frontend/services/url/UrlGenerator.test.js @@ -299,13 +299,13 @@ describe('Unit: services/url/UrlGenerator', function () { describe('fn: _generateUrl', function () { it('returns url', function () { - router.getPermalinks.returns({ - getValue: function () { - return '/:slug/'; - } + const urlGenerator = new UrlGenerator({ + router, + permalink: '/:slug/', + queue, + resources, + urls }); - - const urlGenerator = new UrlGenerator({router, queue, resources, urls}); const replacePermalink = sinon.stub().returns('/url/'); sinon.stub(urlUtils, 'replacePermalink').get(() => replacePermalink);