From 9b54ed06899d2f000c8022b607537c6941b31600 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 31 Oct 2017 09:46:59 +0000 Subject: [PATCH] Refactored apps to have access to a router refs #9192 - Instead of `setupRoutes` function in apps that gets passed a router, there is now a registerRouter function as part of the proxy - Moved towards a route service, which will know about all routes - Using classes to abstract away shared behaviour Notes: - changing the app proxy didn't result in a test failure! - structure of route service is totally new and may change a lot yet --- core/server/apps/amp/index.js | 4 +--- core/server/apps/private-blogging/index.js | 6 ++---- core/server/apps/subscribers/index.js | 5 ++--- core/server/services/apps/proxy.js | 7 +++++++ core/server/services/route/AppRouter.js | 15 +++++++++++++++ core/server/services/route/base/Router.js | 9 +++++++++ core/server/services/route/index.js | 9 +++++++++ core/server/site/routes.js | 12 +++--------- 8 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 core/server/services/route/AppRouter.js create mode 100644 core/server/services/route/base/Router.js create mode 100644 core/server/services/route/index.js diff --git a/core/server/apps/amp/index.js b/core/server/apps/amp/index.js index 9dad7c578708..ffec7b14121c 100644 --- a/core/server/apps/amp/index.js +++ b/core/server/apps/amp/index.js @@ -5,9 +5,7 @@ var router = require('./lib/router'), module.exports = { activate: function activate(ghost) { registerHelpers(ghost); - }, - setupRoutes: function setupRoutes(siteRouter) { - siteRouter.use('*/' + config.get('routeKeywords').amp + '/', router); + ghost.routeService.registerRouter('*/' + config.get('routeKeywords').amp + '/', router); } }; diff --git a/core/server/apps/private-blogging/index.js b/core/server/apps/private-blogging/index.js index dac5ba57cf99..f9bbfeafbaa4 100644 --- a/core/server/apps/private-blogging/index.js +++ b/core/server/apps/private-blogging/index.js @@ -26,15 +26,13 @@ module.exports = { } } + ghost.routeService.registerRouter('/' + config.get('routeKeywords').private + '/', router); + registerHelpers(ghost); }, setupMiddleware: function setupMiddleware(siteApp) { siteApp.use(middleware.checkIsPrivate); siteApp.use(middleware.filterPrivateRoutes); - }, - - setupRoutes: function setupRoutes(siteRouter) { - siteRouter.use('/' + config.get('routeKeywords').private + '/', router); } }; diff --git a/core/server/apps/subscribers/index.js b/core/server/apps/subscribers/index.js index 453e3b749daa..68da358f02b4 100644 --- a/core/server/apps/subscribers/index.js +++ b/core/server/apps/subscribers/index.js @@ -7,11 +7,10 @@ var router = require('./lib/router'), module.exports = { activate: function activate(ghost) { + // TODO, how to do all this only if the Subscribers flag is set?! registerHelpers(ghost); - }, - setupRoutes: function setupRoutes(siteRouter) { - siteRouter.use('/' + config.get('routeKeywords').subscribe + '/', function labsEnabledRouter(req, res, next) { + ghost.routeService.registerRouter('/' + config.get('routeKeywords').subscribe + '/', function labsEnabledRouter(req, res, next) { if (labs.isSet('subscribers') === true) { return router.apply(this, arguments); } diff --git a/core/server/services/apps/proxy.js b/core/server/services/apps/proxy.js index a508389fc4fd..c5024908137e 100644 --- a/core/server/services/apps/proxy.js +++ b/core/server/services/apps/proxy.js @@ -3,6 +3,7 @@ var _ = require('lodash'), helpers = require('../../helpers/register'), filters = require('../../filters'), i18n = require('../../i18n'), + router = require('../route').appRouter, generateProxyFunctions; generateProxyFunctions = function (name, permissions, isInternal) { @@ -69,6 +70,12 @@ generateProxyFunctions = function (name, permissions, isInternal) { register: checkRegisterPermissions('helpers', helpers.registerThemeHelper.bind(helpers)), registerAsync: checkRegisterPermissions('helpers', helpers.registerAsyncThemeHelper.bind(helpers)) }, + // Expose the route service... + routeService: { + // This allows for mounting an entirely new Router at a path... + registerRouter: checkRegisterPermissions('routes', router.registerRouter.bind(router)) + }, + // Mini proxy to the API - needs review api: { posts: passThruAppContextToApi('posts', _.pick(api.posts, 'browse', 'read', 'edit', 'add', 'destroy') diff --git a/core/server/services/route/AppRouter.js b/core/server/services/route/AppRouter.js new file mode 100644 index 000000000000..915309a17132 --- /dev/null +++ b/core/server/services/route/AppRouter.js @@ -0,0 +1,15 @@ +/** + * An instance of router that is provided to Apps, to mount routes into. + */ + +var debug = require('ghost-ignition').debug('services:routes:app'), + Router = require('./base/Router'); + +class AppRouter extends Router { + registerRouter(path, router) { + debug('registerRouter for', path); + this.router.use(path, router); + } +} + +module.exports = AppRouter; diff --git a/core/server/services/route/base/Router.js b/core/server/services/route/base/Router.js new file mode 100644 index 000000000000..ab2a80afccb1 --- /dev/null +++ b/core/server/services/route/base/Router.js @@ -0,0 +1,9 @@ +var expressRouter = require('express').Router; + +class Router { + constructor() { + this.router = expressRouter({mergeParams: true}); + } +} + +module.exports = Router; diff --git a/core/server/services/route/index.js b/core/server/services/route/index.js new file mode 100644 index 000000000000..493429d57557 --- /dev/null +++ b/core/server/services/route/index.js @@ -0,0 +1,9 @@ +/** + * # Route Service + * + * Everything in here is 100% experimental + */ + +var AppRouter = require('./AppRouter'); + +module.exports.appRouter = new AppRouter(); diff --git a/core/server/site/routes.js b/core/server/site/routes.js index 22721a677f47..73981f1902bb 100644 --- a/core/server/site/routes.js +++ b/core/server/site/routes.js @@ -1,8 +1,8 @@ var express = require('express'), - path = require('path'), config = require('../config'), controllers = require('../controllers'), channels = require('../controllers/channels'), + apps = require('../services/route').appRouter, utils = require('../utils'); module.exports = function siteRouter() { @@ -21,14 +21,8 @@ module.exports = function siteRouter() { // Channels router.use(channels.router()); - // setup routes for internal apps - // @TODO: refactor this to be a proper app route hook for internal & external apps - config.get('apps:internal').forEach(function (appName) { - var app = require(path.join(config.get('paths').internalAppPath, appName)); - if (app.hasOwnProperty('setupRoutes')) { - app.setupRoutes(router); - } - }); + // setup routes for apps + router.use(apps.router); // Default router.get('*', controllers.single);