Added Router etc to shared/express + use everywhere

- Added a wrapper around express.Router to our shared/express util
- Also export static and _express
- Use this shared util everywhre, meaning express is only used directly in this one file
- ATM this file is mostly an experiment / debug helper, it might be removed again later
- The aim is to have a minimal framework wrapping express that allows us to:
     - reduce our usage of express() in favour of Router()
     - unify some of our duplicated logic
     - fix some structural issues e.g. Sentry
     - make it easier to understand the codebase

core/frontend/apps/amp/lib/router.js

@@ -1,6 +1,6 @@
 const path = require('path');
-const express = require('express');
+const express = require('../../../../shared/express');
-const ampRouter = express.Router();
+const ampRouter = express.Router('amp');
 
 // Dirty requires
 const common = require('../../../../server/lib/common');

core/frontend/apps/private-blogging/lib/router.js

@@ -1,11 +1,11 @@
 const path = require('path');
-const express = require('express');
+const express = require('../../../../shared/express');
 const middleware = require('./middleware');
 const bodyParser = require('body-parser');
 const routing = require('../../../services/routing');
 const web = require('../../../../server/web');
 const templateName = 'private';
-const privateRouter = express.Router();
+const privateRouter = express.Router(templateName);
 
 function _renderer(req, res) {
     res.routerOptions = {

core/frontend/services/routing/ParentRouter.js

@@ -10,7 +10,7 @@
 const debug = require('ghost-ignition').debug('services:routing:ParentRouter');
 
 const EventEmitter = require('events').EventEmitter;
-const express = require('express');
+const express = require('../../../shared/express');
 const _ = require('lodash');
 const url = require('url');
 const security = require('../../../server/lib/security');
@@ -29,7 +29,7 @@ const registry = require('./registry');
  * @constructor
  */
 function GhostRouter(options) {
-    const router = express.Router(options);
+    const router = express.Router('Parent', options);
 
     function innerRouter(req, res, next) {
         return innerRouter.handle(req, res, next);

core/server/adapters/storage/LocalFileStorage.js

@@ -1,7 +1,6 @@
 // # Local File System Image Storage module
 // The (default) module for storing images, using the local file system
-
+const serveStatic = require('../../../shared/express').static;
-const serveStatic = require('express').static;
 
 const fs = require('fs-extra');
 const path = require('path');

core/server/web/admin/app.js

@@ -1,6 +1,6 @@
 const debug = require('ghost-ignition').debug('web:admin:app');
 const express = require('../../../shared/express');
-const serveStatic = require('express').static;
+const serveStatic = express.static;
 const config = require('../../config');
 const constants = require('../../lib/constants');
 const urlUtils = require('../../lib/url-utils');
@@ -9,7 +9,7 @@ const adminMiddleware = require('./middleware');
 
 module.exports = function setupAdminApp() {
     debug('Admin setup start');
-    const adminApp = express();
+    const adminApp = express('admin');
 
     // Admin assets
     // @TODO ensure this gets a local 404 error handler

core/server/web/api/app.js

@@ -5,7 +5,7 @@ const errorHandler = require('../shared/middlewares/error-handler');
 
 module.exports = function setupApiApp() {
     debug('Parent API setup start');
-    const apiApp = express();
+    const apiApp = express('api');
 
     // Mount different API versions
     apiApp.use(urlUtils.getVersionPath({version: 'v2', type: 'content'}), require('./v2/content/app')());

core/server/web/api/canary/admin/app.js

@@ -8,7 +8,7 @@ const routes = require('./routes');
 
 module.exports = function setupApiApp() {
     debug('Admin API canary setup start');
-    const apiApp = express();
+    const apiApp = express('canary admin');
 
     // API middleware
 

core/server/web/api/canary/admin/routes.js

@@ -1,4 +1,4 @@
-const express = require('express');
+const express = require('../../../../../shared/express');
 const apiCanary = require('../../../../api/canary');
 const apiMw = require('../../middleware');
 const mw = require('./middleware');
@@ -6,7 +6,7 @@ const mw = require('./middleware');
 const shared = require('../../../shared');
 
 module.exports = function apiRoutes() {
-    const router = express.Router();
+    const router = express.Router('canary admin');
 
     // alias delete with del
     router.del = router.delete;

core/server/web/api/canary/content/app.js

@@ -7,7 +7,7 @@ const routes = require('./routes');
 
 module.exports = function setupApiApp() {
     debug('Content API canary setup start');
-    const apiApp = express();
+    const apiApp = express('canary content');
 
     // API middleware
 

core/server/web/api/canary/content/routes.js

@@ -1,10 +1,10 @@
-const express = require('express');
+const express = require('../../../../../shared/express');
 const cors = require('cors');
 const apiCanary = require('../../../../api/canary');
 const mw = require('./middleware');
 
 module.exports = function apiRoutes() {
-    const router = express.Router();
+    const router = express.Router('canary content');
 
     router.use(cors());
 

core/server/web/api/v2/admin/app.js

@@ -8,7 +8,7 @@ const routes = require('./routes');
 
 module.exports = function setupApiApp() {
     debug('Admin API v2 setup start');
-    const apiApp = express();
+    const apiApp = express('v2 admin');
 
     // API middleware
 

core/server/web/api/v2/admin/routes.js

@@ -1,4 +1,4 @@
-const express = require('express');
+const express = require('../../../../../shared/express');
 const apiv2 = require('../../../../api/v2');
 const mw = require('./middleware');
 const apiMw = require('../../middleware');
@@ -6,7 +6,7 @@ const apiMw = require('../../middleware');
 const shared = require('../../../shared');
 
 module.exports = function apiRoutes() {
-    const router = express.Router();
+    const router = express.Router('v2 admin');
 
     // alias delete with del
     router.del = router.delete;

core/server/web/api/v2/content/app.js

@@ -7,7 +7,7 @@ const routes = require('./routes');
 
 module.exports = function setupApiApp() {
     debug('Content API v2 setup start');
-    const apiApp = express();
+    const apiApp = express('v2 content');
 
     // API middleware
 

core/server/web/api/v2/content/routes.js

@@ -1,10 +1,10 @@
-const express = require('express');
+const express = require('../../../../../shared/express');
 const cors = require('cors');
 const apiv2 = require('../../../../api/v2');
 const mw = require('./middleware');
 
 module.exports = function apiRoutes() {
-    const router = express.Router();
+    const router = express.Router('v2 content');
 
     router.use(cors());
 

core/server/web/members/app.js

@@ -9,7 +9,7 @@ const shared = require('../shared');
 
 module.exports = function setupMembersApp() {
     debug('Members App setup start');
-    const membersApp = express();
+    const membersApp = express('members');
 
     // Entire app is behind labs flag
     membersApp.use(shared.middlewares.labs.members);

core/server/web/parent/app.js

@@ -11,7 +11,7 @@ const shared = require('../shared');
 
 module.exports = function setupParentApp(options = {}) {
     debug('ParentApp setup start');
-    const parentApp = express();
+    const parentApp = express('parent');
 
     parentApp.use(mw.requestId);
     parentApp.use(mw.logRequest);
@@ -44,7 +44,7 @@ module.exports = function setupParentApp(options = {}) {
 
     // BACKEND
     // Wrap the admin and API apps into a single express app for use with vhost
-    const backendApp = express();
+    const backendApp = express('backend');
     backendApp.use('/ghost/api', require('../api')());
     backendApp.use('/ghost/.well-known', require('../well-known')());
     backendApp.use('/ghost', require('../../services/auth/session').createSessionFromToken, require('../admin')());
@@ -55,7 +55,7 @@ module.exports = function setupParentApp(options = {}) {
     parentApp.use(vhost(backendVhostArg, backendApp));
 
     // FRONTEND
-    const frontendApp = express();
+    const frontendApp = express('frontend');
 
     // Force SSL if blog url is set to https. The redirects handling must happen before asset and page routing,
     // otherwise we serve assets/pages with http. This can cause mixed content warnings in the admin client.

core/server/web/shared/middlewares/custom-redirects.js

@@ -1,5 +1,5 @@
 const fs = require('fs-extra');
-const express = require('express');
+const express = require('../../../../shared/express');
 const url = require('url');
 const path = require('path');
 const debug = require('ghost-ignition').debug('web:shared:mw:custom-redirects');
@@ -16,7 +16,7 @@ let customRedirectsRouter;
 _private.registerRoutes = () => {
     debug('redirects loading');
 
-    customRedirectsRouter = express.Router();
+    customRedirectsRouter = express.Router('redirects');
 
     try {
         let redirects = fs.readFileSync(path.join(config.getContentPath('data'), 'redirects.json'), 'utf-8');

core/server/web/site/app.js

@@ -75,7 +75,7 @@ function SiteRouter(req, res, next) {
 module.exports = function setupSiteApp(options = {}) {
     debug('Site setup start');
 
-    const siteApp = express();
+    const siteApp = express('site');
 
     // ## App - specific code
     // set the view engine

core/server/web/site/middleware/redirect-ghost-to-admin.js

@@ -1,4 +1,4 @@
-const express = require('express');
+const express = require('../../../../shared/express');
 const config = require('../../../config');
 const urlUtils = require('../../../lib/url-utils');
 
@@ -10,7 +10,7 @@ const adminRedirect = (path) => {
 
 // redirect to /ghost to the admin
 module.exports = function redirectGhostToAdmin() {
-    const router = express.Router();
+    const router = express.Router('redirect-ghost-to-admin');
 
     if (config.get('admin:redirects')) {
         router.get(/^\/ghost\/?$/, adminRedirect('/'));

core/server/web/site/middleware/static-theme.js

@@ -1,8 +1,8 @@
-const express = require('express');
 const path = require('path');
 const config = require('../../../config');
 const constants = require('../../../lib/constants');
 const themeUtils = require('../../../../frontend/services/themes');
+const express = require('../../../../shared/express');
 
 function isBlackListedFileType(file) {
     const blackListedFileTypes = ['.hbs', '.md', '.json'];

core/server/web/well-known.js

@@ -1,4 +1,4 @@
-const express = require('express');
+const express = require('../../shared/express');
 const settings = require('../services/settings/cache');
 const jose = require('node-jose');
 
@@ -12,7 +12,7 @@ const getSafePublicJWKS = async () => {
 };
 
 module.exports = function setupWellKnownApp() {
-    const wellKnownApp = express();
+    const wellKnownApp = express('well-known');
 
     wellKnownApp.get('/jwks.json', async (req, res) => {
         const jwks = await getSafePublicJWKS();

core/shared/express.js

@@ -1,16 +1,36 @@
+const debug = require('ghost-ignition').debug('shared:express');
 const express = require('express');
 const sentry = require('./sentry');
 
-module.exports = () => {
+module.exports = (name) => {
+    debug('new app start', name);
     const app = express();
+    app.set('name', name);
 
     // Make sure 'req.secure' is valid for proxied requests
     // (X-Forwarded-Proto header will be checked, if present)
-    // NB: required here because it's not passed down via vhost
     app.enable('trust proxy');
 
     // Sentry must be our first error handler. Mounting it here means all custom error handlers will come after
     app.use(sentry.errorHandler);
 
+    debug('new app end', name);
     return app;
 };
+
+// Wrap the main express router call
+// This is mostly an experiement, and can likely be removed soon
+module.exports.Router = (name, options) => {
+    debug('new Router start', name);
+    const router = express.Router(options);
+
+    router.use(sentry.errorHandler);
+
+    debug('new Router end', name);
+    return router;
+};
+
+module.exports.static = express.static;
+
+// Export the OG module for testing based on the internals
+module.exports._express = express;

index.js

@@ -15,7 +15,8 @@ debug('Required ghost');
 const express = require('./core/shared/express');
 const {logging} = require('./core/server/lib/common');
 const urlService = require('./core/frontend/services/url');
-const ghostApp = express();
+// This is what listen gets called on, it needs to be a full Express App
+const ghostApp = express('ghost');
 
 // Use the request handler at the top level
 // @TODO: decide if this should be here or in parent App - should it come after request id mw?

test/unit/adapters/scheduling/SchedulingDefault_spec.js

@@ -2,9 +2,9 @@ const should = require('should');
 const sinon = require('sinon');
 const moment = require('moment');
 const _ = require('lodash');
-const express = require('express');
 const bodyParser = require('body-parser');
 const http = require('http');
+const express = require('../../../../core/shared/express');
 const SchedulingDefault = require('../../../../core/server/adapters/scheduling/SchedulingDefault');
 
 describe('Scheduling Default Adapter', function () {

test/unit/services/routing/CollectionRouter_spec.js

@@ -1,6 +1,6 @@
 const should = require('should');
 const sinon = require('sinon');
-const express = require('express');
+const express = require('../../../../core/shared/express')._express;
 const settingsCache = require('../../../../core/server/services/settings/cache');
 const common = require('../../../../core/server/lib/common');
 const controllers = require('../../../../core/frontend/services/routing/controllers');

test/unit/web/site/middleware/serve-favicon_spec.js

@@ -1,6 +1,6 @@
 const should = require('should');
 const sinon = require('sinon');
-const express = require('express');
+const express = require('../../../../../core/shared/express');
 const serveFavicon = require('../../../../../core/server/web/site/middleware/serve-favicon');
 const settingsCache = require('../../../../../core/server/services/settings/cache');
 const storage = require('../../../../../core/server/adapters/storage');
@@ -19,7 +19,7 @@ describe('Serve Favicon', function () {
         req = sinon.spy();
         res = sinon.spy();
         next = sinon.spy();
-        blogApp = express();
+        blogApp = express('test');
         req.app = blogApp;
 
         sinon.stub(settingsCache, 'get').callsFake(function (key) {

test/unit/web/site/middleware/static-theme_spec.js

@@ -1,7 +1,7 @@
 const should = require('should');
 const sinon = require('sinon');
 
-const express = require('express');
+const express = require('../../../../../core/shared/express');
 const themeUtils = require('../../../../../core/frontend/services/themes');
 const staticTheme = require('../../../../../core/server/web/site/middleware/static-theme');
 

test/utils/index.js

@@ -3,7 +3,7 @@ const _ = require('lodash');
 const fs = require('fs-extra');
 const path = require('path');
 const os = require('os');
-const express = require('express');
+const express = require('../../core/shared/express');
 const debug = require('ghost-ignition').debug('test');
 const ObjectId = require('bson-objectid');
 const uuid = require('uuid');
@@ -907,7 +907,7 @@ startGhost = function startGhost(options) {
             ghostServer = _ghostServer;
 
             if (options.subdir) {
-                parentApp = express();
+                parentApp = express('test parent');
                 parentApp.use(urlUtils.getSubdir(), ghostServer.rootApp);
                 return ghostServer.start(parentApp);
             }