Refactored bootstrap module into RouterManager class

refs https://linear.app/tryghost/issue/CORE-104/decouple-frontend-routing-events-from-urlserver-events

- The "bootstrap" didn't give enough credit to everything this module was doing - it's responsible for managing correct initialization and reinitialization of the frontend Routes as well as passing router creation information back to the frontend's URL service
- The refactor is done in two steps - the "bootstrap.js" will be renamed in the follow-up commit to have a clean history of how the file evolved

core/bridge.js

@@ -16,7 +16,7 @@ const config = require('./shared/config');
 const logging = require('@tryghost/logging');
 const tpl = require('@tryghost/tpl');
 const themeEngine = require('./frontend/services/theme-engine');
-const frontendRouting = require('./frontend/services/routing/bootstrap');
+const frontendRouting = require('./frontend/services/routing').bootstrap;
 const settingsCache = require('./shared/settings-cache');
 
 // Listen to settings.lang.edited, similar to the member service and models/base/listeners

core/frontend/services/routing/bootstrap.js

@@ -12,13 +12,39 @@ const UnsubscribeRouter = require('./UnsubscribeRouter');
 // This emits its own routing events
 const events = require('../../../server/lib/common/events');
 
-const defaultApiVersion = 'v4';
+class RouterManager {
+    constructor({registry, defaultApiVersion = 'v4'}) {
+        this.registry = registry;
+        this.defaultApiVersion = defaultApiVersion;
+        this.siteRouter = null;
+        this.urlService = null;
+    }
 
-// This registry thing should be passed into here as a dependency once the module
+    owns(routerId, id) {
-// is rewritten into the class + DI pattern
+        return this.urlService.owns(routerId, id);
-const registry = require('./registry');
+    }
-let siteRouter;
+
-let _urlService;
+    getUrlByResourceId(id, options) {
+        return this.urlService.getUrlByResourceId(id, options);
+    }
+
+    getResourceById(resourceId) {
+        return this.urlService.getResourceById(resourceId);
+    }
+
+    routerCreated(router) {
+        // NOTE: this event should be become an "internal frontend even"
+        //       and should not be consumed by the modules outside the frontend
+        events.emit('router.created', this);
+
+        // CASE: there are router types which do not generate resource urls
+        //       e.g. static route router, in this case we don't want ot notify the URL service
+        if (!router || !router.getPermalinks()) {
+            return;
+        }
+
+        this.urlService.onRouterAddedType(router);
+    }
 
     /**
      * @description The `init` function will return the wrapped parent express router and will start creating all
@@ -36,27 +62,27 @@ let _urlService;
      * @param {Object} options.urlService - service providing resource URL utility functions such as owns, getUrlByResourceId, and getResourceById
      * @returns {ExpressRouter}
      */
-const init = ({start = false, routerSettings, apiVersion, urlService}) => {
+    init({start = false, routerSettings, apiVersion, urlService}) {
-    _urlService = urlService;
+        this.urlService = urlService;
         debug('bootstrap init', start, apiVersion, routerSettings);
 
-    registry.resetAllRouters();
+        this.registry.resetAllRouters();
-    registry.resetAllRoutes();
+        this.registry.resetAllRoutes();
 
         // NOTE: this event could become an "internal frontend" in the future, it's used has been kept to prevent
         //       from tying up this module with sitemaps
         events.emit('routers.reset');
 
-    siteRouter = new ParentRouter('SiteRouter');
+        this.siteRouter = new ParentRouter('SiteRouter');
-    registry.setRouter('siteRouter', siteRouter);
+        this.registry.setRouter('siteRouter', this.siteRouter);
 
         if (start) {
-        apiVersion = apiVersion || defaultApiVersion;
+            apiVersion = apiVersion || this.defaultApiVersion;
             this.start(apiVersion, routerSettings);
         }
 
-    return siteRouter.router();
+        return this.siteRouter.router();
-};
+    }
 
     /**
      * @description This function will create the routers based on the route settings
@@ -74,54 +100,54 @@ const init = ({start = false, routerSettings, apiVersion, urlService}) => {
      * @param {string} apiVersion
      * @param {object} routerSettings
      */
-const start = (apiVersion, routerSettings) => {
+    start(apiVersion, routerSettings) {
         debug('bootstrap start', apiVersion, routerSettings);
         const RESOURCE_CONFIG = require(`./config/${apiVersion}`);
 
         const unsubscribeRouter = new UnsubscribeRouter();
-    siteRouter.mountRouter(unsubscribeRouter.router());
+        this.siteRouter.mountRouter(unsubscribeRouter.router());
-    registry.setRouter('unsubscribeRouter', unsubscribeRouter);
+        this.registry.setRouter('unsubscribeRouter', unsubscribeRouter);
 
         const emailRouter = new EmailRouter(RESOURCE_CONFIG);
-    siteRouter.mountRouter(emailRouter.router());
+        this.siteRouter.mountRouter(emailRouter.router());
-    registry.setRouter('emailRouter', emailRouter);
+        this.registry.setRouter('emailRouter', emailRouter);
 
         const previewRouter = new PreviewRouter(RESOURCE_CONFIG);
-    siteRouter.mountRouter(previewRouter.router());
+        this.siteRouter.mountRouter(previewRouter.router());
-    registry.setRouter('previewRouter', previewRouter);
+        this.registry.setRouter('previewRouter', previewRouter);
 
         _.each(routerSettings.routes, (value, key) => {
-        const staticRoutesRouter = new StaticRoutesRouter(key, value, this.internal.routerCreated);
+            const staticRoutesRouter = new StaticRoutesRouter(key, value, this.routerCreated.bind(this));
-        siteRouter.mountRouter(staticRoutesRouter.router());
+            this.siteRouter.mountRouter(staticRoutesRouter.router());
 
-        registry.setRouter(staticRoutesRouter.identifier, staticRoutesRouter);
+            this.registry.setRouter(staticRoutesRouter.identifier, staticRoutesRouter);
         });
 
         _.each(routerSettings.collections, (value, key) => {
-        const collectionRouter = new CollectionRouter(key, value, RESOURCE_CONFIG, this.internal.routerCreated);
+            const collectionRouter = new CollectionRouter(key, value, RESOURCE_CONFIG, this.routerCreated.bind(this));
-        siteRouter.mountRouter(collectionRouter.router());
+            this.siteRouter.mountRouter(collectionRouter.router());
-        registry.setRouter(collectionRouter.identifier, collectionRouter);
+            this.registry.setRouter(collectionRouter.identifier, collectionRouter);
         });
 
-    const staticPagesRouter = new StaticPagesRouter(RESOURCE_CONFIG, this.internal.routerCreated);
+        const staticPagesRouter = new StaticPagesRouter(RESOURCE_CONFIG, this.routerCreated.bind(this));
-    siteRouter.mountRouter(staticPagesRouter.router());
+        this.siteRouter.mountRouter(staticPagesRouter.router());
 
-    registry.setRouter('staticPagesRouter', staticPagesRouter);
+        this.registry.setRouter('staticPagesRouter', staticPagesRouter);
 
         _.each(routerSettings.taxonomies, (value, key) => {
-        const taxonomyRouter = new TaxonomyRouter(key, value, RESOURCE_CONFIG, this.internal.routerCreated);
+            const taxonomyRouter = new TaxonomyRouter(key, value, RESOURCE_CONFIG, this.routerCreated.bind(this));
-        siteRouter.mountRouter(taxonomyRouter.router());
+            this.siteRouter.mountRouter(taxonomyRouter.router());
 
-        registry.setRouter(taxonomyRouter.identifier, taxonomyRouter);
+            this.registry.setRouter(taxonomyRouter.identifier, taxonomyRouter);
         });
 
         const appRouter = new ParentRouter('AppsRouter');
-    siteRouter.mountRouter(appRouter.router());
+        this.siteRouter.mountRouter(appRouter.router());
 
-    registry.setRouter('appRouter', appRouter);
+        this.registry.setRouter('appRouter', appRouter);
 
-    debug('Routes:', registry.getAllRoutes());
+        debug('Routes:', this.registry.getAllRoutes());
-};
+    }
 
     /**
      * This is a glue code to keep the implementation of routers away from
@@ -130,7 +156,7 @@ const start = (apiVersion, routerSettings) => {
      * @param {Object} settingModel instance of the settings model
      * @returns {void}
      */
-const handleTimezoneEdit = (settingModel) => {
+    handleTimezoneEdit(settingModel) {
         const newTimezone = settingModel.attributes.value;
         const previousTimezone = settingModel._previousAttributes.value;
 
@@ -147,43 +173,13 @@ const handleTimezoneEdit = (settingModel) => {
          */
 
         // NOTE: timezone change only affects the collection router with dated permalinks
-    const collectionRouter = registry.getRouterByName('CollectionRouter');
+        const collectionRouter = this.registry.getRouterByName('CollectionRouter');
         if (collectionRouter.getPermalinks().getValue().match(/:year|:month|:day/)) {
             debug('handleTimezoneEdit: trigger regeneration');
 
-        this.internal.routerUpdated(collectionRouter);
+            this.urlService.onRouterUpdated(collectionRouter);
+        }
     }
-};
-
-module.exports.internal = {
-    owns: (routerId, id) => {
-        return _urlService.owns(routerId, id);
-    },
-    getUrlByResourceId: (id, options) => {
-        return _urlService.getUrlByResourceId(id, options);
-    },
-    getResourceById: (resourceId) => {
-        return _urlService.getResourceById(resourceId);
-    },
-    routerCreated: (router) => {
-        // NOTE: this event should be become an "internal frontend even"
-        //       and should not be consumed by the modules outside the frontend
-        events.emit('router.created', this);
-
-        // CASE: there are router types which do not generate resource urls
-        //       e.g. static route router, in this case we don't want ot notify the URL service
-        if (!router || !router.getPermalinks()) {
-            return;
 }
 
-        _urlService.onRouterAddedType(router);
+module.exports = RouterManager;
-    },
-    routerUpdated: (router) => {
-        _urlService.onRouterUpdated(router);
-    }
-};
-
-// Public API methods called out by the backend
-module.exports.init = init;
-module.exports.start = start;
-module.exports.handleTimezoneEdit = handleTimezoneEdit;

core/frontend/services/routing/controllers/collection.js

@@ -3,7 +3,7 @@ const debug = require('@tryghost/debug')('services:routing:controllers:collectio
 const tpl = require('@tryghost/tpl');
 const errors = require('@tryghost/errors');
 const security = require('@tryghost/security');
-const bootstrap = require('../bootstrap');
+const {bootstrap} = require('../');
 const themeEngine = require('../../theme-engine');
 const helpers = require('../helpers');
 
@@ -72,7 +72,7 @@ module.exports = function collectionController(req, res, next) {
              * People should always invert their filters to ensure that the database query loads unique posts per collection.
              */
             result.posts = _.filter(result.posts, (post) => {
-                if (bootstrap.internal.owns(res.routerOptions.identifier, post.id)) {
+                if (bootstrap.owns(res.routerOptions.identifier, post.id)) {
                     return post;
                 }
 

core/frontend/services/routing/controllers/email-post.js

@@ -1,6 +1,6 @@
 const debug = require('@tryghost/debug')('services:routing:controllers:emailpost');
 const config = require('../../../../shared/config');
-const bootstrap = require('../bootstrap');
+const {bootstrap} = require('../');
 const urlUtils = require('../../../../shared/url-utils');
 const helpers = require('../helpers');
 
@@ -48,7 +48,7 @@ module.exports = function emailPostController(req, res, next) {
             }
 
             if (post.status === 'published') {
-                return urlUtils.redirect301(res, bootstrap.internal.getUrlByResourceId(post.id, {withSubdirectory: true}));
+                return urlUtils.redirect301(res, bootstrap.getUrlByResourceId(post.id, {withSubdirectory: true}));
             }
 
             if (res.locals.apiVersion !== 'v0.1' && res.locals.apiVersion !== 'v2') {

core/frontend/services/routing/controllers/entry.js

@@ -1,7 +1,7 @@
 const debug = require('@tryghost/debug')('services:routing:controllers:entry');
 const url = require('url');
 const config = require('../../../../shared/config');
-const bootstrap = require('../bootstrap');
+const {bootstrap} = require('../');
 const urlUtils = require('../../../../shared/url-utils');
 const helpers = require('../helpers');
 
@@ -60,7 +60,7 @@ module.exports = function entryController(req, res, next) {
              *
              * That's why we have to check against the router type.
              */
-            if (bootstrap.internal.getResourceById(entry.id).config.type !== res.routerOptions.resourceType) {
+            if (bootstrap.getResourceById(entry.id).config.type !== res.routerOptions.resourceType) {
                 debug('not my resource type');
                 return next();
             }

core/frontend/services/routing/controllers/preview.js

@@ -1,6 +1,6 @@
 const debug = require('@tryghost/debug')('services:routing:controllers:preview');
 const config = require('../../../../shared/config');
-const bootstrap = require('../bootstrap');
+const {bootstrap} = require('../');
 const urlUtils = require('../../../../shared/url-utils');
 const helpers = require('../helpers');
 
@@ -50,7 +50,7 @@ module.exports = function previewController(req, res, next) {
             }
 
             if (post.status === 'published') {
-                return urlUtils.redirect301(res, bootstrap.internal.getUrlByResourceId(post.id, {withSubdirectory: true}));
+                return urlUtils.redirect301(res, bootstrap.getUrlByResourceId(post.id, {withSubdirectory: true}));
             }
 
             if (res.locals.apiVersion !== 'v0.1' && res.locals.apiVersion !== 'v2') {

core/frontend/services/routing/index.js

@@ -1,7 +1,9 @@
+const registry = require('./registry');
+const RouterManager = require('./bootstrap');
+const routerManager = new RouterManager({registry});
+
 module.exports = {
-    get bootstrap() {
+    bootstrap: routerManager,
-        return require('./bootstrap');
-    },
 
     get registry() {
         return require('./registry');

core/frontend/services/rss/generate-feed.js

@@ -3,7 +3,7 @@ const Promise = require('bluebird');
 const cheerio = require('cheerio');
 const RSS = require('rss');
 const urlUtils = require('../../../shared/url-utils');
-const bootstrap = require('../routing/bootstrap');
+const {bootstrap} = require('../routing');
 
 const generateTags = function generateTags(data) {
     if (data.tags) {
@@ -19,7 +19,7 @@ const generateTags = function generateTags(data) {
 };
 
 const generateItem = function generateItem(post, secure) {
-    const itemUrl = bootstrap.internal.getUrlByResourceId(post.id, {secure, absolute: true});
+    const itemUrl = bootstrap.getUrlByResourceId(post.id, {secure, absolute: true});
     const htmlContent = cheerio.load(post.html || '');
     const item = {
         title: post.title,

test/unit/frontend/services/routing/bootstrap.test.js

@@ -1,18 +1,18 @@
 const should = require('should');
 const sinon = require('sinon');
 const CollectionRouter = require('../../../../../core/frontend/services/routing/CollectionRouter');
-const bootstrap = require('../../../../../core/frontend/services/routing/bootstrap');
+const RouterManager = require('../../../../../core/frontend/services/routing/bootstrap');
 const registry = require('../../../../../core/frontend/services/routing/registry');
 
 const RESOURCE_CONFIG = {QUERY: {post: {controller: 'posts', resource: 'posts'}}};
 
 describe('UNIT: services/routing/bootstrap', function () {
-    let routesUpdatedStub;
+    let routerUpdatedSpy;
     let routerCreatedSpy;
 
     beforeEach(function () {
         routerCreatedSpy = sinon.spy();
-        routesUpdatedStub = sinon.stub(bootstrap.internal, 'routerUpdated').returns();
+        routerUpdatedSpy = sinon.spy();
     });
 
     afterEach(function () {
@@ -25,24 +25,38 @@ describe('UNIT: services/routing/bootstrap', function () {
                 const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/'}, RESOURCE_CONFIG, routerCreatedSpy);
                 sinon.stub(registry, 'getRouterByName').withArgs('CollectionRouter').returns(collectionRouter);
 
-                bootstrap.handleTimezoneEdit({
+                const routerManager = new RouterManager({registry});
+                routerManager.init({
+                    urlService: {
+                        onRouterUpdated: routerUpdatedSpy
+                    }
+                });
+
+                routerManager.handleTimezoneEdit({
                     attributes: {value: 'America/Los_Angeles'},
                     _previousAttributes: {value: 'Europe/London'}
                 });
 
-                routesUpdatedStub.called.should.be.false();
+                routerUpdatedSpy.called.should.be.false();
             });
 
             it('tz has not changed', function () {
                 const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/'}, RESOURCE_CONFIG, routerCreatedSpy);
                 sinon.stub(registry, 'getRouterByName').withArgs('CollectionRouter').returns(collectionRouter);
 
-                bootstrap.handleTimezoneEdit({
+                const routerManager = new RouterManager({registry});
+                routerManager.init({
+                    urlService: {
+                        onRouterUpdated: routerUpdatedSpy
+                    }
+                });
+
+                routerManager.handleTimezoneEdit({
                     attributes: {value: 'America/Los_Angeles'},
                     _previousAttributes: {value: 'America/Los_Angeles'}
                 });
 
-                routesUpdatedStub.called.should.be.false();
+                routerUpdatedSpy.called.should.be.false();
             });
         });
 
@@ -51,24 +65,38 @@ describe('UNIT: services/routing/bootstrap', function () {
                 const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:year/:slug/'}, RESOURCE_CONFIG, routerCreatedSpy);
                 sinon.stub(registry, 'getRouterByName').withArgs('CollectionRouter').returns(collectionRouter);
 
-                bootstrap.handleTimezoneEdit({
+                const routerManager = new RouterManager({registry});
+                routerManager.init({
+                    urlService: {
+                        onRouterUpdated: routerUpdatedSpy
+                    }
+                });
+
+                routerManager.handleTimezoneEdit({
                     attributes: {value: 'America/Los_Angeles'},
                     _previousAttributes: {value: 'Europe/London'}
                 });
 
-                routesUpdatedStub.called.should.be.true();
+                routerUpdatedSpy.called.should.be.true();
             });
 
             it('tz has not changed', function () {
                 const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:year/:slug/'}, RESOURCE_CONFIG, routerCreatedSpy);
                 sinon.stub(registry, 'getRouterByName').withArgs('CollectionRouter').returns(collectionRouter);
 
-                bootstrap.handleTimezoneEdit({
+                const routerManager = new RouterManager({registry});
+                routerManager.init({
+                    urlService: {
+                        onRouterUpdated: routerUpdatedSpy
+                    }
+                });
+
+                routerManager.handleTimezoneEdit({
                     attributes: {value: 'America/Los_Angeles'},
                     _previousAttributes: {value: 'America/Los_Angeles'}
                 });
 
-                routesUpdatedStub.called.should.be.false();
+                routerUpdatedSpy.called.should.be.false();
             });
         });
     });

test/unit/frontend/services/routing/controllers/collection.test.js

@@ -4,7 +4,7 @@ const sinon = require('sinon');
 const testUtils = require('../../../../../utils');
 const security = require('@tryghost/security');
 const themeEngine = require('../../../../../../core/frontend/services/theme-engine');
-const bootstrap = require('../../../../../../core/frontend/services/routing/bootstrap');
+const bootstrap = require('../../../../../../core/frontend/services/routing/').bootstrap;
 const controllers = require('../../../../../../core/frontend/services/routing/controllers');
 const helpers = require('../../../../../../core/frontend/services/routing/helpers');
 
@@ -56,7 +56,7 @@ describe('Unit - services/routing/controllers/collection', function () {
 
         sinon.stub(helpers, 'renderEntries').returns(renderStub);
 
-        ownsStub = sinon.stub(bootstrap.internal, 'owns');
+        ownsStub = sinon.stub(bootstrap, 'owns');
         ownsStub.withArgs('identifier', posts[0].id).returns(true);
 
         req = {

test/unit/frontend/services/routing/controllers/entry.test.js

@@ -3,7 +3,7 @@ const sinon = require('sinon');
 const testUtils = require('../../../../../utils');
 const configUtils = require('../../../../../utils/configUtils');
 const urlUtils = require('../../../../../../core/shared/url-utils');
-const bootstrap = require('../../../../../../core/frontend/services/routing/bootstrap');
+const bootstrap = require('../../../../../../core/frontend/services/routing/').bootstrap;
 const controllers = require('../../../../../../core/frontend/services/routing/controllers');
 const helpers = require('../../../../../../core/frontend/services/routing/helpers');
 const EDITOR_URL = `/#/editor/post/`;
@@ -41,7 +41,7 @@ describe('Unit - services/routing/controllers/entry', function () {
 
         sinon.stub(urlUtils, 'redirectToAdmin');
         sinon.stub(urlUtils, 'redirect301');
-        sinon.stub(bootstrap.internal, 'getResourceById');
+        sinon.stub(bootstrap, 'getResourceById');
 
         req = {
             path: '/',
@@ -81,7 +81,7 @@ describe('Unit - services/routing/controllers/entry', function () {
 
         res.routerOptions.resourceType = 'posts';
 
-        bootstrap.internal.getResourceById.withArgs(post.id).returns({
+        bootstrap.getResourceById.withArgs(post.id).returns({
             config: {
                 type: 'posts'
             }
@@ -162,7 +162,7 @@ describe('Unit - services/routing/controllers/entry', function () {
             req.path = post.url;
             res.routerOptions.resourceType = 'posts';
 
-            bootstrap.internal.getResourceById.withArgs(post.id).returns({
+            bootstrap.getResourceById.withArgs(post.id).returns({
                 config: {
                     type: 'pages'
                 }
@@ -186,7 +186,7 @@ describe('Unit - services/routing/controllers/entry', function () {
 
             res.routerOptions.resourceType = 'posts';
 
-            bootstrap.internal.getResourceById.withArgs(post.id).returns({
+            bootstrap.getResourceById.withArgs(post.id).returns({
                 config: {
                     type: 'posts'
                 }
@@ -215,7 +215,7 @@ describe('Unit - services/routing/controllers/entry', function () {
 
             res.routerOptions.resourceType = 'posts';
 
-            bootstrap.internal.getResourceById.withArgs(post.id).returns({
+            bootstrap.getResourceById.withArgs(post.id).returns({
                 config: {
                     type: 'posts'
                 }

test/unit/frontend/services/rss/generate-feed.test.js

@@ -3,7 +3,7 @@ const sinon = require('sinon');
 const _ = require('lodash');
 const testUtils = require('../../../../utils');
 const configUtils = require('../../../../utils/configUtils');
-const bootstrap = require('../../../../../core/frontend/services/routing/bootstrap');
+const bootstrap = require('../../../../../core/frontend/services/routing').bootstrap;
 const generateFeed = require('../../../../../core/frontend/services/rss/generate-feed');
 
 describe('RSS: Generate Feed', function () {
@@ -43,7 +43,7 @@ describe('RSS: Generate Feed', function () {
     });
 
     beforeEach(function () {
-        sinon.stub(bootstrap.internal, 'getUrlByResourceId');
+        sinon.stub(bootstrap, 'getUrlByResourceId');
 
         baseUrl = '/rss/';
 
@@ -92,7 +92,7 @@ describe('RSS: Generate Feed', function () {
             data.posts = posts;
 
             _.each(data.posts, function (post) {
-                bootstrap.internal.getUrlByResourceId.withArgs(post.id, {secure: undefined, absolute: true}).returns('http://my-ghost-blog.com/' + post.slug + '/');
+                bootstrap.getUrlByResourceId.withArgs(post.id, {secure: undefined, absolute: true}).returns('http://my-ghost-blog.com/' + post.slug + '/');
             });
 
             generateFeed(baseUrl, data).then(function (xmlData) {
@@ -204,7 +204,7 @@ describe('RSS: Generate Feed', function () {
             data.posts = [posts[0]];
 
             _.each(data.posts, function (post) {
-                bootstrap.internal.getUrlByResourceId.withArgs(post.id, {secure: undefined, absolute: true}).returns('http://my-ghost-blog.com/' + post.slug + '/');
+                bootstrap.getUrlByResourceId.withArgs(post.id, {secure: undefined, absolute: true}).returns('http://my-ghost-blog.com/' + post.slug + '/');
             });
 
             generateFeed(baseUrl, data).then(function (xmlData) {