Represent channels as class instances (#9209)

refs #9192, refs #5091

- Using a class allows for easy shared logic
- Loading is designed to work from config right now, but could be DB driven, etc
- Provided configuration can be simplified and extended in the constructor / class methods
- Update tests, move custom assertions to utils

core/server/controllers/channels/Channel.js

@@ -0,0 +1,61 @@
+'use strict';
+
+var _ = require('lodash'),
+    config  = require('../../config'),
+    defaultPostOptions = {};
+
+class Channel {
+    constructor(name, options) {
+        // Set the name
+        this.name = name;
+
+        // Store the originally passed in options
+        this._origOptions = _.cloneDeep(options) || {};
+
+        // Setup our route
+        // @TODO should a channel have a route as part of the object? Or should this live elsewhere?
+        this.route = this._origOptions.route ? this.translateRoute(this._origOptions.route) : '/';
+
+        // Define context as name, plus any additional contexts, and don't allow duplicates
+        this.context = _.union([this.name], this._origOptions.context);
+
+        // DATA options
+        // Options for fetching related posts
+        this.postOptions = _.defaults({}, defaultPostOptions, this._origOptions.postOptions);
+
+        // RESOURCE!!!
+        // @TODO figure out a better way to declare relation to resource
+        if (this._origOptions.data) {
+            this.data = this._origOptions.data;
+        }
+
+        // Template options
+        // @TODO fix these HORRIBLE names
+        this.slugTemplate = !!this._origOptions.slugTemplate;
+        if (this._origOptions.frontPageTemplate) {
+            this.frontPageTemplate = this._origOptions.frontPageTemplate;
+        }
+
+        if (this._origOptions.editRedirect) {
+            this.editRedirect = this._origOptions.editRedirect;
+        }
+    }
+
+    get isPaged() {
+        return _.has(this._origOptions, 'paged') ? this._origOptions.paged : true;
+    }
+
+    get hasRSS() {
+        return _.has(this._origOptions, 'rss') ? this._origOptions.rss : true;
+    }
+
+    translateRoute(route) {
+        // @TODO find this a more general / global home, as part of the Router system,
+        // so that ALL routes that get registered WITH Ghost can do this
+        return route.replace(/:t_([a-zA-Z]+)/, function (fullMatch, keyword) {
+            return config.get('routeKeywords')[keyword];
+        });
+    }
+}
+
+module.exports = Channel;

core/server/controllers/channels/loader.js

@@ -1,4 +1,6 @@
-var _ = require('lodash'),
+var debug = require('ghost-ignition').debug('channels:loader'),
+    _ = require('lodash'),
+    Channel = require('./Channel'),
     channels = [];
 
 function loadConfig() {
@@ -17,11 +19,11 @@ function loadConfig() {
 }
 
 module.exports.list = function list() {
+    debug('Load channels start');
     _.each(loadConfig(), function (channelConfig, channelName) {
-        var channel = _.cloneDeep(channelConfig);
-        channel.name = channelName;
-        channels.push(channel);
+        channels.push(new Channel(channelName, channelConfig));
     });
 
+    debug('Load channels end');
     return channels;
 };

core/server/controllers/channels/router.js

@@ -8,6 +8,7 @@ var express = require('express'),
     channelLoader = require('./loader'),
     renderChannel = require('../frontend/render-channel'),
     rssRouter,
+    channelRouter,
     channelsRouter;
 
 function handlePageParam(req, res, next, page) {
@@ -63,23 +64,20 @@ rssRouter = function rssRouter(channelMiddleware) {
     return router;
 };
 
-function buildChannelRouter(channel) {
+channelRouter = function channelRouter(channel) {
     var channelRouter = express.Router({mergeParams: true}),
         baseRoute = '/',
         pageRoute = utils.url.urlJoin('/', config.get('routeKeywords').page, ':page(\\d+)/'),
         middleware = [channelConfigMiddleware(channel)];
 
-    // @TODO figure out how to collapse this into a single rule
     channelRouter.get(baseRoute, middleware, renderChannel);
 
-    // @TODO improve config and add defaults to make this simpler
-    if (channel.paged !== false) {
+    if (channel.isPaged) {
         channelRouter.param('page', handlePageParam);
         channelRouter.get(pageRoute, middleware, renderChannel);
     }
 
-    // @TODO improve config and add defaults to make this simpler
-    if (channel.rss !== false) {
+    if (channel.hasRSS) {
         channelRouter.use(rssRouter(middleware));
     }
 
@@ -90,18 +88,14 @@ function buildChannelRouter(channel) {
     }
 
     return channelRouter;
-}
+};
 
-channelsRouter = function router() {
+channelsRouter = function channelsRouter() {
     var channelsRouter = express.Router({mergeParams: true});
 
     _.each(channelLoader.list(), function (channel) {
-        var channelRoute = channel.route.replace(/:t_([a-zA-Z]+)/, function (fullMatch, keyword) {
-            return config.get('routeKeywords')[keyword];
-        });
-
         // Mount this channel router on the parent channels router
-        channelsRouter.use(channelRoute, buildChannelRouter(channel));
+        channelsRouter.use(channel.route, channelRouter(channel));
     });
 
     return channelsRouter;

core/server/controllers/frontend/context.js

@@ -54,11 +54,7 @@ function setResponseContext(req, res, data) {
 
     // Each page can only have at most one of these
     if (res.locals.channel) {
-        if (res.locals.channel.context) {
-            res.locals.context = res.locals.context.concat(res.locals.channel.context);
-        } else {
-            res.locals.context.push(res.locals.channel.name);
-        }
+        res.locals.context = res.locals.context.concat(res.locals.channel.context);
     } else if (privatePattern.test(res.locals.relativeUrl)) {
         res.locals.context.push('private');
     } else if (subscribePattern.test(res.locals.relativeUrl) && labs.isSet('subscribers') === true) {

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

@@ -5,142 +5,30 @@ var should = require('should'),  // jshint ignore:line
     // Stuff we are testing
     channelLoader = require('../../../../server/controllers/channels/loader'),
     channels = require('../../../../server/controllers/channels'),
+    channelUtils = require('../../../utils/channelUtils'),
+    Channel = channelUtils.Channel,
 
     sandbox = sinon.sandbox.create();
 
-/**
- * Assertions on the express API
- */
-should.Assertion.add('ExpressRouter', function (options) {
-    options = options || {};
-
-    this.params = {operator: 'to be a valid Express Router'};
-    this.obj.should.be.a.Function();
-    this.obj.name.should.eql('router');
-    this.obj.should.have.property('mergeParams', true);
-    this.obj.should.have.property('strict', undefined);
-    this.obj.should.have.property('stack');
-
-    if (options.params) {
-        // Verify the params function!
-        this.obj.params.should.have.property(options.params.key);
-        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);
-    }
-});
-
-should.Assertion.add('Layer', function () {
-    this.params = {operator: 'to be a valid Express Layer'};
-
-    this.obj.should.be.an.Object().with.properties(['handle', 'name', 'params', 'path', 'keys', 'regexp', 'route']);
-});
-
-should.Assertion.add('RouterLayer', function (options) {
-    options = options || {};
-
-    this.params = {operator: 'to be a valid Express Layer, with Router as handle'};
-
-    this.obj.should.be.a.Layer();
-    this.obj.name.should.eql('router');
-    this.obj.handle.should.be.an.ExpressRouter(options);
-
-    if (options.regexp) {
-        this.obj.regexp.toString().should.match(options.regexp);
-    }
-});
-
-should.Assertion.add('DispatchLayer', function (options) {
-    options = options || {};
-
-    this.params = {operator: 'to be a valid Express Layer, with Dispatch as handle'};
-    this.obj.should.be.a.Layer();
-    this.obj.name.should.eql('bound dispatch');
-
-    if (options.regexp) {
-        this.obj.regexp.toString().should.match(options.regexp);
-    }
-
-    if (options.keys) {
-        this.obj.keys.should.be.an.Array().with.lengthOf(options.keys.length);
-        _.map(this.obj.keys, 'name').should.eql(options.keys);
-    } else {
-        this.obj.keys.should.be.an.Array().with.lengthOf(0);
-    }
-
-    this.obj.route.should.be.an.Object().with.properties(['path', 'stack', 'methods']);
-    if (options.route && options.route.path) {
-        this.obj.route.path.should.eql(options.route.path);
-    }
-
-    if (options.route.stack) {
-        this.obj.route.stack.should.be.an.Array().with.lengthOf(options.route.stack.length);
-        _.map(this.obj.route.stack, 'name').should.eql(options.route.stack);
-    } else {
-        this.obj.route.stack.should.be.an.Array();
-    }
-});
-
-should.Assertion.add('RSSRouter', function () {
-    this.params = {operator: 'to be a valid RSS Router'};
-
-    this.obj.should.be.a.RouterLayer({
-        stackLength: 3,
-        params: {
-            key: 'page',
-            value: 'handlePageParam'
-        }
-    });
-
-    var routeStack = this.obj.handle.stack;
-
-    // Layer 1 should be the handler for /rss/
-    routeStack[0].should.be.a.DispatchLayer({
-        route: {
-            path: '/rss/',
-            stack: ['doChannelConfig', 'rssConfigMiddleware', 'generate']
-        }
-    });
-
-    // Layer 2 should be the handler for pagination
-    routeStack[1].should.be.a.DispatchLayer({
-        keys: ['page'],
-        route: {
-            path: '/rss/:page(\\d+)/',
-            stack: ['doChannelConfig', 'rssConfigMiddleware', 'generate']
-        }
-    });
-
-    // // Layer 3 should be a handler for the extra /feed/ url
-    routeStack[2].should.be.a.DispatchLayer({
-        route: {
-            path: '/feed/',
-            stack: ['redirectToRSS']
-        }
-    });
-});
-
 /**
  * These tests are a bit weird,
  * need to test express private API
  * Would be better to be testing our own internal API
  * E.g. setupRSS.calledOnce, rather than router stack!
  */
 describe('Custom Channels', function () {
+    var channelLoaderStub;
+
     afterEach(function () {
         sandbox.restore();
     });
 
+    beforeEach(function () {
+        channelLoaderStub = sandbox.stub(channelLoader, 'list');
+    });
+
     it('allows basic custom config', function () {
-        sandbox.stub(channelLoader, 'list').returns({
-            home: {
-                name: 'home',
-                route: '/home/'
-            }
-        });
+        channelLoaderStub.returns([new Channel('home', {route: '/home/'})]);
 
         var channelsRouter = channels.router(),
             firstChannel,
@@ -182,19 +70,16 @@ describe('Custom Channels', function () {
     });
 
     it('allow multiple channels to be defined', function () {
-        sandbox.stub(channelLoader, 'list').returns({
-            home: {
-                name: 'home',
-                route: '/home/'
-            },
-            featured: {
-                name: 'featured',
-                route: '/featured/',
-                postOptions: {
-                    filter: 'featured:true'
-                }
-            }
-        });
+        channelLoaderStub.returns(
+            [
+                new Channel('home', {route: '/home/'}),
+                new Channel('featured', {
+                    route: '/featured/',
+                    postOptions: {
+                        filter: 'featured:true'
+                    }
+                })
+            ]);
 
         var channelsRouter = channels.router(),
             firstChannel,
@@ -247,13 +132,10 @@ describe('Custom Channels', function () {
     });
 
     it('allows rss to be disabled', function () {
-        sandbox.stub(channelLoader, 'list').returns({
-            home: {
-                name: 'home',
-                route: '/home/',
-                rss: false
-            }
-        });
+        channelLoaderStub.returns([new Channel('home', {
+            route: '/home/',
+            rss: false
+        })]);
 
         var channelsRouter = channels.router(),
             firstChannel,
@@ -295,13 +177,10 @@ describe('Custom Channels', function () {
     });
 
     it('allows pagination to be disabled', function () {
-        sandbox.stub(channelLoader, 'list').returns({
-            home: {
-                name: 'home',
-                route: '/home/',
-                paged: false
-            }
-        });
+        channelLoaderStub.returns([new Channel('home', {
+            route: '/home/',
+            paged: false
+        })]);
 
         var channelsRouter = channels.router(),
             firstChannel,

core/test/unit/controllers/channels/loader_spec.js

@@ -2,7 +2,8 @@ var should = require('should'),  // jshint ignore:line
     _ = require('lodash'),
     rewire = require('rewire'),
     channelUtils = require('../../../utils/channelUtils'),
-    channelLoader = rewire('../../../../server/controllers/channels/loader');
+    channelLoader = rewire('../../../../server/controllers/channels/loader'),
+    Channel = channelUtils.Channel;
 
 describe('Channel Config', function () {
     var channelReset;
@@ -19,8 +20,12 @@ describe('Channel Config', function () {
 
     it('should build a list of channels', function () {
         var channels = channelLoader.list();
-        channels.should.be.an.Object();
+        channels.should.be.an.Array().with.lengthOf(3);
 
         _.map(channels, 'name').should.eql(['index', 'tag', 'author']);
+
+        _.each(channels, function (channel) {
+            channel.should.be.a.Channel();
+        });
     });
 });