mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-02-03 23:00:14 -05:00
Improve channels router code (#9166)
refs #5091 - There is very little that changes here, just code readability - However I've expanded out the tests getting ready to be able to test more deeply as I refactor the routing
This commit is contained in:
Hannah Wolfe
GitHub
parent
eff331e99b
commit
c20a6aa7f7
2 changed files with 326 additions and 81 deletions
|
|
@ -8,7 +8,7 @@ var express = require('express'),
|
|||
channelConfig = require('./channel-config'),
|
||||
renderChannel = require('./render-channel'),
|
||||
rssRouter,
|
||||
channelRouter;
|
||||
channelsRouter;
|
||||
|
||||
function handlePageParam(req, res, next, page) {
|
||||
var pageRegex = new RegExp('/' + config.get('routeKeywords').page + '/(.*)?/'),
|
||||
|
|
@ -33,19 +33,28 @@ function handlePageParam(req, res, next, page) {
|
|||
}
|
||||
}
|
||||
|
||||
rssRouter = function rssRouter(channelConfig) {
|
||||
function rssConfigMiddleware(req, res, next) {
|
||||
res.locals.channel.isRSS = true;
|
||||
next();
|
||||
}
|
||||
function rssConfigMiddleware(req, res, next) {
|
||||
res.locals.channel.isRSS = true;
|
||||
next();
|
||||
}
|
||||
|
||||
function channelConfigMiddleware(channel) {
|
||||
return function doChannelConfig(req, res, next) {
|
||||
res.locals.channel = _.cloneDeep(channel);
|
||||
next();
|
||||
};
|
||||
}
|
||||
|
||||
rssRouter = function rssRouter(channelMiddleware) {
|
||||
// @TODO move this to an RSS module
|
||||
var router = express.Router({mergeParams: true}),
|
||||
stack = [channelConfig, rssConfigMiddleware, rss],
|
||||
baseRoute = '/rss/';
|
||||
baseRoute = '/rss/',
|
||||
pageRoute = utils.url.urlJoin(baseRoute, ':page(\\d+)/');
|
||||
|
||||
router.get(baseRoute, stack);
|
||||
router.get(utils.url.urlJoin(baseRoute, ':page(\\d+)/'), stack);
|
||||
// @TODO figure out how to collapse this into a single rule
|
||||
router.get(baseRoute, channelMiddleware, rssConfigMiddleware, rss);
|
||||
router.get(pageRoute, channelMiddleware, rssConfigMiddleware, rss);
|
||||
// Extra redirect rule
|
||||
router.get('/feed/', function redirectToRSS(req, res) {
|
||||
return utils.url.redirect301(res, utils.url.urlJoin(utils.url.getSubdir(), req.baseUrl, baseRoute));
|
||||
});
|
||||
|
|
@ -54,47 +63,44 @@ rssRouter = function rssRouter(channelConfig) {
|
|||
return router;
|
||||
};
|
||||
|
||||
channelRouter = function router() {
|
||||
function channelConfigMiddleware(channel) {
|
||||
return function doChannelConfig(req, res, next) {
|
||||
res.locals.channel = _.cloneDeep(channel);
|
||||
next();
|
||||
};
|
||||
function buildChannelRouter(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) {
|
||||
channelRouter.param('page', handlePageParam);
|
||||
channelRouter.get(pageRoute, middleware, renderChannel);
|
||||
}
|
||||
|
||||
var channelsRouter = express.Router({mergeParams: true}),
|
||||
baseRoute = '/',
|
||||
pageRoute = utils.url.urlJoin('/', config.get('routeKeywords').page, ':page(\\d+)/');
|
||||
// @TODO improve config and add defaults to make this simpler
|
||||
if (channel.rss !== false) {
|
||||
channelRouter.use(rssRouter(middleware));
|
||||
}
|
||||
|
||||
if (channel.editRedirect) {
|
||||
channelRouter.get('/edit/', function redirect(req, res) {
|
||||
utils.url.redirectToAdmin(302, res, channel.editRedirect.replace(':slug', req.params.slug));
|
||||
});
|
||||
}
|
||||
|
||||
return channelRouter;
|
||||
}
|
||||
|
||||
channelsRouter = function router() {
|
||||
var channelsRouter = express.Router({mergeParams: true});
|
||||
|
||||
_.each(channelConfig.list(), function (channel) {
|
||||
var channelRouter = express.Router({mergeParams: true}),
|
||||
configChannel = channelConfigMiddleware(channel);
|
||||
|
||||
// @TODO figure out how to collapse this into a single rule
|
||||
channelRouter.get(baseRoute, configChannel, renderChannel);
|
||||
|
||||
// @TODO improve config and add defaults to make this simpler
|
||||
if (channel.paged !== false) {
|
||||
channelRouter.param('page', handlePageParam);
|
||||
channelRouter.get(pageRoute, configChannel, renderChannel);
|
||||
}
|
||||
|
||||
// @TODO improve config and add defaults to make this simpler
|
||||
if (channel.rss !== false) {
|
||||
channelRouter.use(rssRouter(configChannel));
|
||||
}
|
||||
|
||||
if (channel.editRedirect) {
|
||||
channelRouter.get('/edit/', function redirect(req, res) {
|
||||
utils.url.redirectToAdmin(302, res, channel.editRedirect.replace(':slug', req.params.slug));
|
||||
});
|
||||
}
|
||||
|
||||
// Mount this channel router on the parent channels router
|
||||
channelsRouter.use(channel.route, channelRouter);
|
||||
channelsRouter.use(channel.route, buildChannelRouter(channel));
|
||||
});
|
||||
|
||||
return channelsRouter;
|
||||
};
|
||||
|
||||
module.exports.router = channelRouter;
|
||||
module.exports.router = channelsRouter;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
var should = require('should'), // jshint ignore:line
|
||||
sinon = require('sinon'),
|
||||
_ = require('lodash'),
|
||||
|
||||
// Stuff we are testing
|
||||
channelConfig = require('../../../../server/controllers/frontend/channel-config'),
|
||||
|
|
@ -7,6 +8,121 @@ var should = require('should'), // jshint ignore:line
|
|||
|
||||
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
|
||||
|
|
@ -26,23 +142,108 @@ describe('Custom Channels', function () {
|
|||
}
|
||||
});
|
||||
|
||||
var channelRouter = channels.router(),
|
||||
topRouter = channelRouter.stack[0],
|
||||
routeStack = topRouter.handle.stack,
|
||||
rssRouter = routeStack[2].handle.stack;
|
||||
var channelsRouter = channels.router(),
|
||||
firstChannel,
|
||||
routeStack;
|
||||
|
||||
topRouter.regexp.toString().should.match(/\\\/home\\\//);
|
||||
routeStack.should.have.lengthOf(3);
|
||||
channelsRouter.should.be.an.ExpressRouter({stackLength: 1});
|
||||
firstChannel = channelsRouter.stack[0];
|
||||
firstChannel.should.be.a.RouterLayer({
|
||||
stackLength: 3,
|
||||
regexp: /\\\/home\\\//,
|
||||
params: {
|
||||
key: 'page',
|
||||
value: 'handlePageParam'
|
||||
}
|
||||
});
|
||||
|
||||
// Route 1 should be /
|
||||
routeStack[0].route.path.should.eql('/');
|
||||
// Route 2 should be pagination
|
||||
routeStack[1].route.path.should.eql('/page/:page(\\d+)/');
|
||||
// Route 3 should be a whole new router for RSS
|
||||
rssRouter.should.have.lengthOf(3);
|
||||
rssRouter[0].route.path.should.eql('/rss/');
|
||||
rssRouter[1].route.path.should.eql('/rss/:page(\\d+)/');
|
||||
rssRouter[2].route.path.should.eql('/feed/');
|
||||
// The stack underneath our first channel router
|
||||
routeStack = firstChannel.handle.stack;
|
||||
|
||||
// Layer 1 should be the handler for /
|
||||
routeStack[0].should.be.a.DispatchLayer({
|
||||
route: {
|
||||
path: '/',
|
||||
stack: ['doChannelConfig', 'renderChannel']
|
||||
}
|
||||
});
|
||||
|
||||
// Layer 2 should be the handler for pagination
|
||||
routeStack[1].should.be.a.DispatchLayer({
|
||||
keys: ['page'],
|
||||
route: {
|
||||
path: '/page/:page(\\d+)/',
|
||||
stack: ['doChannelConfig', 'renderChannel']
|
||||
}
|
||||
});
|
||||
|
||||
// Layer 3 should be a whole new router for RSS
|
||||
routeStack[2].should.be.an.RSSRouter();
|
||||
});
|
||||
|
||||
it('allow multiple channels to be defined', function () {
|
||||
sandbox.stub(channelConfig, 'list').returns({
|
||||
home: {
|
||||
name: 'home',
|
||||
route: '/home/'
|
||||
},
|
||||
featured: {
|
||||
name: 'featured',
|
||||
route: '/featured/',
|
||||
postOptions: {
|
||||
filter: 'featured:true'
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
var channelsRouter = channels.router(),
|
||||
firstChannel,
|
||||
secondChannel,
|
||||
routeStack;
|
||||
|
||||
channelsRouter.should.be.an.ExpressRouter({stackLength: 2});
|
||||
firstChannel = channelsRouter.stack[0];
|
||||
secondChannel = channelsRouter.stack[1];
|
||||
firstChannel.should.be.a.RouterLayer({
|
||||
stackLength: 3,
|
||||
regexp: /\\\/home\\\//,
|
||||
params: {
|
||||
key: 'page',
|
||||
value: 'handlePageParam'
|
||||
}
|
||||
});
|
||||
|
||||
secondChannel.should.be.a.RouterLayer({
|
||||
stackLength: 3,
|
||||
regexp: /\\\/featured\\\//,
|
||||
params: {
|
||||
key: 'page',
|
||||
value: 'handlePageParam'
|
||||
}
|
||||
});
|
||||
|
||||
// The stack underneath our first channel router
|
||||
routeStack = secondChannel.handle.stack;
|
||||
|
||||
// Layer 1 should be the handler for /
|
||||
routeStack[0].should.be.a.DispatchLayer({
|
||||
route: {
|
||||
path: '/',
|
||||
stack: ['doChannelConfig', 'renderChannel']
|
||||
}
|
||||
});
|
||||
|
||||
// Layer 2 should be the handler for pagination
|
||||
routeStack[1].should.be.a.DispatchLayer({
|
||||
keys: ['page'],
|
||||
route: {
|
||||
path: '/page/:page(\\d+)/',
|
||||
stack: ['doChannelConfig', 'renderChannel']
|
||||
}
|
||||
});
|
||||
|
||||
// Layer 3 should be a whole new router for RSS
|
||||
routeStack[2].should.be.an.RSSRouter();
|
||||
});
|
||||
|
||||
it('allows rss to be disabled', function () {
|
||||
|
|
@ -54,17 +255,43 @@ describe('Custom Channels', function () {
|
|||
}
|
||||
});
|
||||
|
||||
var channelRouter = channels.router(),
|
||||
topRouter = channelRouter.stack[0],
|
||||
routeStack = topRouter.handle.stack;
|
||||
var channelsRouter = channels.router(),
|
||||
firstChannel,
|
||||
routeStack;
|
||||
|
||||
topRouter.regexp.toString().should.match(/\\\/home\\\//);
|
||||
routeStack.should.have.lengthOf(2);
|
||||
channelsRouter.should.be.an.ExpressRouter({stackLength: 1});
|
||||
firstChannel = channelsRouter.stack[0];
|
||||
firstChannel.should.be.a.RouterLayer({
|
||||
stackLength: 2,
|
||||
regexp: /\\\/home\\\//,
|
||||
params: {
|
||||
key: 'page',
|
||||
value: 'handlePageParam'
|
||||
}
|
||||
});
|
||||
|
||||
// Route 1 should be /
|
||||
routeStack[0].route.path.should.eql('/');
|
||||
// Route 2 should be pagination
|
||||
routeStack[1].route.path.should.eql('/page/:page(\\d+)/');
|
||||
// The stack underneath our first channel router
|
||||
routeStack = firstChannel.handle.stack;
|
||||
|
||||
// Layer 1 should be the handler for /
|
||||
routeStack[0].should.be.a.DispatchLayer({
|
||||
route: {
|
||||
path: '/',
|
||||
stack: ['doChannelConfig', 'renderChannel']
|
||||
}
|
||||
});
|
||||
|
||||
// Layer 2 should be the handler for pagination
|
||||
routeStack[1].should.be.a.DispatchLayer({
|
||||
keys: ['page'],
|
||||
route: {
|
||||
path: '/page/:page(\\d+)/',
|
||||
stack: ['doChannelConfig', 'renderChannel']
|
||||
}
|
||||
});
|
||||
|
||||
// Layer 3 does not exist
|
||||
should.not.exist(routeStack[2]);
|
||||
});
|
||||
|
||||
it('allows pagination to be disabled', function () {
|
||||
|
|
@ -76,20 +303,32 @@ describe('Custom Channels', function () {
|
|||
}
|
||||
});
|
||||
|
||||
var channelRouter = channels.router(),
|
||||
topRouter = channelRouter.stack[0],
|
||||
routeStack = topRouter.handle.stack,
|
||||
rssRouter = routeStack[1].handle.stack;
|
||||
var channelsRouter = channels.router(),
|
||||
firstChannel,
|
||||
routeStack;
|
||||
|
||||
topRouter.regexp.toString().should.match(/\\\/home\\\//);
|
||||
routeStack.should.have.lengthOf(2);
|
||||
channelsRouter.should.be.an.ExpressRouter({stackLength: 1});
|
||||
firstChannel = channelsRouter.stack[0];
|
||||
firstChannel.should.be.a.RouterLayer({
|
||||
stackLength: 2,
|
||||
regexp: /\\\/home\\\//
|
||||
});
|
||||
|
||||
// Route 1 should be /
|
||||
routeStack[0].route.path.should.eql('/');
|
||||
// Route 2 should be a whole new router for RSS
|
||||
rssRouter.should.have.lengthOf(3);
|
||||
rssRouter[0].route.path.should.eql('/rss/');
|
||||
rssRouter[1].route.path.should.eql('/rss/:page(\\d+)/');
|
||||
rssRouter[2].route.path.should.eql('/feed/');
|
||||
// The stack underneath our first channel router
|
||||
routeStack = firstChannel.handle.stack;
|
||||
|
||||
// Layer 1 should be the handler for /
|
||||
routeStack[0].should.be.a.DispatchLayer({
|
||||
route: {
|
||||
path: '/',
|
||||
stack: ['doChannelConfig', 'renderChannel']
|
||||
}
|
||||
});
|
||||
|
||||
// Layer 2 should be the handler for /rss/
|
||||
routeStack[1].should.be.an.RSSRouter();
|
||||
|
||||
// Layer 3 does not exist
|
||||
should.not.exist(routeStack[2]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue