Changed channelsRouter to use new base class

refs #9192, #5091

- changed channels to use our new base class
- keep the flexible structure, so that channels can be reloaded
- I had to move the router into the route service otherwise we get circular dependencies
- Don't _really_ want to keep it like this - need a way to define base classes as shared

core/server/services/channels/index.js

@@ -1 +1,2 @@
-module.exports.router = require('./parent-router');
+module.exports.router = require('./router');
+module.exports.loader = require('./loader');

core/server/services/channels/parent-router.js

@@ -1,14 +0,0 @@
-var express = require('express'),
-    channelLoader = require('./loader'),
-    channelRouter = require('./router');
-
-module.exports = function channelsRouter() {
-    var channelsRouter = express.Router({mergeParams: true});
-
-    channelLoader.list().forEach(function (channel) {
-        // Mount this channel router on the parent channels router
-        channelsRouter.use(channel.route, channelRouter(channel));
-    });
-
-    return channelsRouter;
-};

core/server/services/route/base/Router.js

@@ -15,7 +15,7 @@ var debug = require('ghost-ignition').debug('services:routes:ParentRouter'),
 /**
  * We expose a very limited amount of express.Router via specialist methods
  */
-class Router {
+class ParentRouter {
     constructor(name) {
         this.name = name;
         this._router = express.Router({mergeParams: true});
@@ -46,4 +46,4 @@ class Router {
     }
 }
 
-module.exports = Router;
+module.exports = ParentRouter;

core/server/services/route/channels-router.js

@@ -0,0 +1,18 @@
+var Router = require('./base/Router'),
+    channelService = require('../channels');
+
+/**
+ * Channels Router
+ * Parent channels router will load & mount all routes when
+ * .router() is called. This allows for reloading.
+ */
+module.exports.router = function channelsRouter() {
+    var channelsRouter = new Router('channels');
+
+    channelService.loader.list().forEach(function (channel) {
+        // Mount this channel router on the parent channels router
+        channelsRouter.mountRouter(channel.route, channelService.router(channel));
+    });
+
+    return channelsRouter.router();
+};

core/server/services/route/site-router.js

@@ -3,7 +3,7 @@ var Router = require('./base/Router'),
 
     // Sub Routers
     appRouter = require('./app-router'),
-    channelService = require('../channels'),
+    channelsRouter = require('./channels-router'),
 
     // Controllers
     controllers = require('../../controllers'),
@@ -30,7 +30,7 @@ _private.mountDefaultRoutes = function mountDefaultRoutes() {
     // Channels - register sub-router
     // The purpose of having a parentRouter for channels, is so that we can load channels from wherever we want:
     // config, settings, apps, etc, and that it will be possible for the router to be reloaded.
-    siteRouter.mountRouter(channelService.router());
+    siteRouter.mountRouter(channelsRouter.router());
 
     // Apps - register sub-router
     // The purpose of having a parentRouter for apps, is that Apps can register a route whenever they want.

core/test/unit/services/channels/custom_channels_spec.js

@@ -4,7 +4,7 @@ var should = require('should'),  // jshint ignore:line
 
     // Stuff we are testing
     channelLoader = require('../../../../server/services/channels/loader'),
-    channels = require('../../../../server/services/channels'),
+    channelsParentRouter = require('../../../../server/services/route/channels-router'),
     channelUtils = require('../../../utils/channelUtils'),
     Channel = channelUtils.Channel,
 
@@ -18,7 +18,7 @@ var should = require('should'),  // jshint ignore:line
  * And we need to differentiate more between testing the default channels, and channels in general
  */
 describe('Custom Channels', function () {
-    var channelLoaderStub;
+    var channelLoaderStub, channelsRouter, firstChannel, secondChannel, routeStack;
 
     afterEach(function () {
         sandbox.restore();
@@ -28,12 +28,15 @@ describe('Custom Channels', function () {
         channelLoaderStub = sandbox.stub(channelLoader, 'list');
     });
 
+    function getNewChannelsRouter() {
+        return channelsParentRouter.router();
+    }
+
     it('allows basic custom config', function () {
         channelLoaderStub.returns([new Channel('home', {route: '/home/'})]);
 
-        var channelsRouter = channels.router(),
-            firstChannel,
-            routeStack;
+        // Load the router
+        channelsRouter = getNewChannelsRouter();
 
         channelsRouter.should.be.an.ExpressRouter({stackLength: 1});
         firstChannel = channelsRouter.stack[0];
@@ -82,10 +85,8 @@ describe('Custom Channels', function () {
                 })
             ]);
 
-        var channelsRouter = channels.router(),
-            firstChannel,
-            secondChannel,
-            routeStack;
+        // Load the router
+        channelsRouter = getNewChannelsRouter();
 
         channelsRouter.should.be.an.ExpressRouter({stackLength: 2});
         firstChannel = channelsRouter.stack[0];
@@ -138,9 +139,8 @@ describe('Custom Channels', function () {
             rss: false
         })]);
 
-        var channelsRouter = channels.router(),
-            firstChannel,
-            routeStack;
+        // Load the router
+        channelsRouter = getNewChannelsRouter();
 
         channelsRouter.should.be.an.ExpressRouter({stackLength: 1});
         firstChannel = channelsRouter.stack[0];
@@ -183,9 +183,8 @@ describe('Custom Channels', function () {
             paged: false
         })]);
 
-        var channelsRouter = channels.router(),
-            firstChannel,
-            routeStack;
+        // Load the router
+        channelsRouter = getNewChannelsRouter();
 
         channelsRouter.should.be.an.ExpressRouter({stackLength: 1});
         firstChannel = channelsRouter.stack[0];

core/test/unit/services/channels/parent-router_spec.js

@@ -4,7 +4,7 @@ var should = require('should'),
     _ = require('lodash'),
 
     // Stuff we are testing
-    channelService = require('../../../../server/services/channels'),
+    channelsParentRouter = require('../../../../server/services/route/channels-router'),
     api = require('../../../../server/api'),
     themes = require('../../../../server/themes'),
     sandbox = sinon.sandbox.create();
@@ -142,7 +142,7 @@ describe('Channels', function () {
 
     before(function () {
         // We don't overwrite this, so only do it once
-        channelsRouter = channelService.router();
+        channelsRouter = channelsParentRouter.router();
     });
 
     afterEach(function () {

core/test/utils/channelUtils.js

@@ -38,10 +38,25 @@ should.Assertion.add('Channel', function (options) {
 /**
  * Assertions on the express API
  */
+should.Assertion.add('Stack', function (length) {
+    this.params = {
+        operator: 'to be a valid Express Stack',
+        actual: _.map(this.obj, 'name')
+    };
+
+    this.obj.should.be.an.Array();
+    if (length) {
+        this.obj.length.should.eql(length);
+    }
+});
+
 should.Assertion.add('ExpressRouter', function (options) {
     options = options || {};
 
-    this.params = {operator: 'to be a valid Express Router'};
+    this.params = {
+        operator: 'to be a valid Express Router',
+        actual: require('util').inspect(this.obj, {depth: 1})
+    };
 
     this.obj.should.be.a.Function();
     this.obj.name.should.eql('router');
@@ -55,11 +70,7 @@ should.Assertion.add('ExpressRouter', function (options) {
         this.obj.params[options.params.key][0].name.should.eql(options.params.value);
     }
 
-    this.obj.stack.should.be.an.Array();
-
-    if (options.stackLength) {
-        this.obj.stack.should.have.lengthOf(options.stackLength);
-    }
+    this.obj.stack.should.be.a.Stack(options.stackLength);
 });
 
 should.Assertion.add('Layer', function () {