From 13c1742eb9f28ce49ccd46487499534a26b3db73 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 9 Feb 2016 14:14:24 +0000 Subject: [PATCH] Make frontend routing dynamic & driven by channels refs #5091 - Move renderChannel to own file - Update channel config to have get/list methods - Move main routes to be generated based on the list of channels - Move RSS routes to be subroutes of channels - Move redirect301 to be a shared util - Add full test coverage - Split frontend route tests into frontend & channels --- .../controllers/frontend/channel-config.js | 20 +- core/server/controllers/frontend/channels.js | 93 +++ core/server/controllers/frontend/index.js | 70 +-- .../controllers/frontend/render-channel.js | 51 ++ core/server/routes/frontend.js | 84 +-- core/server/utils/index.js | 5 + core/test/functional/routes/channel_spec.js | 554 ++++++++++++++++++ core/test/functional/routes/frontend_spec.js | 491 ---------------- .../controllers/frontend/channels_spec.js | 421 +++++++++++++ .../unit/controllers/frontend/index_spec.js | 171 ------ core/test/unit/rss_spec.js | 22 +- core/test/unit/server_utils_spec.js | 19 + 12 files changed, 1180 insertions(+), 821 deletions(-) create mode 100644 core/server/controllers/frontend/channels.js create mode 100644 core/server/controllers/frontend/render-channel.js create mode 100644 core/test/functional/routes/channel_spec.js create mode 100644 core/test/unit/controllers/frontend/channels_spec.js diff --git a/core/server/controllers/frontend/channel-config.js b/core/server/controllers/frontend/channel-config.js index abed7a98cd..ac0f98e3bd 100644 --- a/core/server/controllers/frontend/channel-config.js +++ b/core/server/controllers/frontend/channel-config.js @@ -1,8 +1,8 @@ var _ = require('lodash'), config = require('../../config'), - getConfig; + channelConfig; -getConfig = function getConfig(name) { +channelConfig = function channelConfig() { var defaults = { index: { name: 'index', @@ -22,7 +22,8 @@ getConfig = function getConfig(name) { options: {slug: '%s'} } }, - slugTemplate: true + slugTemplate: true, + editRedirect: '/ghost/settings/tags/:slug/' }, author: { name: 'author', @@ -37,11 +38,18 @@ getConfig = function getConfig(name) { options: {slug: '%s'} } }, - slugTemplate: true + slugTemplate: true, + editRedirect: '/ghost/team/:slug/' } }; - return _.cloneDeep(defaults[name]); + return defaults; }; -module.exports = getConfig; +module.exports.list = function list() { + return channelConfig(); +}; + +module.exports.get = function get(name) { + return _.cloneDeep(channelConfig()[name]); +}; diff --git a/core/server/controllers/frontend/channels.js b/core/server/controllers/frontend/channels.js new file mode 100644 index 0000000000..44244d7a31 --- /dev/null +++ b/core/server/controllers/frontend/channels.js @@ -0,0 +1,93 @@ +var express = require('express'), + _ = require('lodash'), + config = require('../../config'), + errors = require('../../errors'), + rss = require('../../data/xml/rss'), + utils = require('../../utils'), + + channelConfig = require('./channel-config'), + renderChannel = require('./render-channel'), + + rssRouter, + channelRouter; + +function handlePageParam(req, res, next, page) { + var pageRegex = new RegExp('/' + config.routeKeywords.page + '/(.*)?/'), + rssRegex = new RegExp('/rss/(.*)?/'); + + page = parseInt(page, 10); + + if (page === 1) { + // Page 1 is an alias, do a permanent 301 redirect + if (rssRegex.test(req.url)) { + return utils.redirect301(res, req.originalUrl.replace(rssRegex, '/rss/')); + } else { + return utils.redirect301(res, req.originalUrl.replace(pageRegex, '/')); + } + } else if (page < 1 || isNaN(page)) { + // Nothing less than 1 is a valid page number, go straight to a 404 + return next(new errors.NotFoundError()); + } else { + // Set req.params.page to the already parsed number, and continue + req.params.page = page; + return next(); + } +} + +rssRouter = function rssRouter(channelConfig) { + function rssConfigMiddleware(req, res, next) { + req.channelConfig.isRSS = true; + next(); + } + + // @TODO move this to an RSS module + var router = express.Router({mergeParams: true}), + stack = [channelConfig, rssConfigMiddleware, rss], + baseRoute = '/rss/'; + + router.get(baseRoute, stack); + router.get(baseRoute + ':page/', stack); + router.get('/feed/', function redirectToRSS(req, res) { + return utils.redirect301(res, config.paths.subdir + req.baseUrl + baseRoute); + }); + router.param('page', handlePageParam); + + return router; +}; + +channelRouter = function router() { + function channelConfigMiddleware(channel) { + return function doChannelConfig(req, res, next) { + req.channelConfig = _.cloneDeep(channel); + next(); + }; + } + + var channelsRouter = express.Router({mergeParams: true}), + baseRoute = '/', + pageRoute = '/' + config.routeKeywords.page + '/:page/'; + + _.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); + channelRouter.get(pageRoute, configChannel, renderChannel); + channelRouter.param('page', handlePageParam); + channelRouter.use(rssRouter(configChannel)); + + if (channel.editRedirect) { + channelRouter.get('/edit/', function redirect(req, res) { + res.redirect(config.paths.subdir + channel.editRedirect.replace(':slug', req.params.slug)); + }); + } + + // Mount this channel router on the parent channels router + channelsRouter.use(channel.route, channelRouter); + }); + + return channelsRouter; +}; + +module.exports.router = channelRouter; diff --git a/core/server/controllers/frontend/index.js b/core/server/controllers/frontend/index.js index bd482178e4..3f6a21d345 100644 --- a/core/server/controllers/frontend/index.js +++ b/core/server/controllers/frontend/index.js @@ -6,21 +6,17 @@ var _ = require('lodash'), api = require('../../api'), - rss = require('../../data/xml/rss'), path = require('path'), config = require('../../config'), errors = require('../../errors'), filters = require('../../filters'), Promise = require('bluebird'), - templates = require('./templates'), + templates = require('./templates'), routeMatch = require('path-match')(), - safeString = require('../../utils/index').safeString, handleError = require('./error'), - fetchData = require('./fetch-data'), formatResponse = require('./format-response'), - channelConfig = require('./channel-config'), setResponseContext = require('./context'), - setRequestIsSecure = require('./secure'), + setRequestIsSecure = require('./secure'), frontendControllers, staticPostPermalink = routeMatch('/:slug/:edit?'); @@ -41,69 +37,7 @@ function renderPost(req, res) { }; } -function renderChannel(name) { - return function renderChannel(req, res, next) { - // Parse the parameters we need from the URL - var channelOpts = channelConfig(name), - pageParam = req.params.page !== undefined ? req.params.page : 1, - slugParam = req.params.slug ? safeString(req.params.slug) : undefined; - - // Ensure we at least have an empty object for postOptions - channelOpts.postOptions = channelOpts.postOptions || {}; - // Set page on postOptions for the query made later - channelOpts.postOptions.page = pageParam; - channelOpts.slugParam = slugParam; - - // Call fetchData to get everything we need from the API - return fetchData(channelOpts).then(function handleResult(result) { - // If page is greater than number of pages we have, go straight to 404 - if (pageParam > result.meta.pagination.pages) { - return next(new errors.NotFoundError()); - } - - // @TODO: figure out if this can be removed, it's supposed to ensure that absolutely URLs get generated - // correctly for the various objects, but I believe it doesn't work and a different approach is needed. - setRequestIsSecure(req, result.posts); - _.each(result.data, function (data) { - setRequestIsSecure(req, data); - }); - - // @TODO: properly design these filters - filters.doFilter('prePostsRender', result.posts, res.locals).then(function then(posts) { - var view = templates.channel(req.app.get('activeTheme'), channelOpts); - - // Do final data formatting and then render - result.posts = posts; - result = formatResponse.channel(result); - setResponseContext(req, res); - res.render(view, result); - }); - }).catch(handleError(next)); - }; -} - frontendControllers = { - index: renderChannel('index'), - tag: renderChannel('tag'), - author: renderChannel('author'), - rss: function (req, res, next) { - // Temporary hack, channels will allow us to resolve this better eventually - var tagPattern = new RegExp('^\\/' + config.routeKeywords.tag + '\\/.+'), - authorPattern = new RegExp('^\\/' + config.routeKeywords.author + '\\/.+'); - - if (tagPattern.test(res.locals.relativeUrl)) { - req.channelConfig = channelConfig('tag'); - } else if (authorPattern.test(res.locals.relativeUrl)) { - req.channelConfig = channelConfig('author'); - } else { - req.channelConfig = channelConfig('index'); - } - - req.channelConfig.isRSS = true; - - return rss(req, res, next); - }, - preview: function preview(req, res, next) { var params = { uuid: req.params.uuid, diff --git a/core/server/controllers/frontend/render-channel.js b/core/server/controllers/frontend/render-channel.js new file mode 100644 index 0000000000..10cc660b86 --- /dev/null +++ b/core/server/controllers/frontend/render-channel.js @@ -0,0 +1,51 @@ +var _ = require('lodash'), + errors = require('../../errors'), + filters = require('../../filters'), + safeString = require('../../utils/index').safeString, + handleError = require('./error'), + fetchData = require('./fetch-data'), + formatResponse = require('./format-response'), + setResponseContext = require('./context'), + setRequestIsSecure = require('./secure'), + templates = require('./templates'); + +function renderChannel(req, res, next) { + // Parse the parameters we need from the URL + var channelOpts = req.channelConfig, + pageParam = req.params.page !== undefined ? req.params.page : 1, + slugParam = req.params.slug ? safeString(req.params.slug) : undefined; + + // Ensure we at least have an empty object for postOptions + channelOpts.postOptions = channelOpts.postOptions || {}; + // Set page on postOptions for the query made later + channelOpts.postOptions.page = pageParam; + channelOpts.slugParam = slugParam; + + // Call fetchData to get everything we need from the API + return fetchData(channelOpts).then(function handleResult(result) { + // If page is greater than number of pages we have, go straight to 404 + if (pageParam > result.meta.pagination.pages) { + return next(new errors.NotFoundError()); + } + + // @TODO: figure out if this can be removed, it's supposed to ensure that absolutely URLs get generated + // correctly for the various objects, but I believe it doesn't work and a different approach is needed. + setRequestIsSecure(req, result.posts); + _.each(result.data, function (data) { + setRequestIsSecure(req, data); + }); + + // @TODO: properly design these filters + filters.doFilter('prePostsRender', result.posts, res.locals).then(function then(posts) { + var view = templates.channel(req.app.get('activeTheme'), channelOpts); + + // Do final data formatting and then render + result.posts = posts; + result = formatResponse.channel(result); + setResponseContext(req, res); + res.render(view, result); + }); + }).catch(handleError(next)); +} + +module.exports = renderChannel; diff --git a/core/server/routes/frontend.js b/core/server/routes/frontend.js index 352321cd43..5c9cc7f216 100644 --- a/core/server/routes/frontend.js +++ b/core/server/routes/frontend.js @@ -1,6 +1,6 @@ var frontend = require('../controllers/frontend'), + channels = require('../controllers/frontend/channels'), config = require('../config'), - errors = require('../errors'), express = require('express'), utils = require('../utils'), @@ -10,52 +10,19 @@ frontendRoutes = function frontendRoutes(middleware) { var router = express.Router(), subdir = config.paths.subdir, routeKeywords = config.routeKeywords, - indexRouter = express.Router(), - tagRouter = express.Router({mergeParams: true}), - authorRouter = express.Router({mergeParams: true}), - rssRouter = express.Router({mergeParams: true}), privateRouter = express.Router(); - function redirect301(res, path) { - /*jslint unparam:true*/ - res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}); - res.redirect(301, path); - } - - function handlePageParam(req, res, next, page) { - var pageRegex = new RegExp('/' + routeKeywords.page + '/(.*)?/'), - rssRegex = new RegExp('/rss/(.*)?/'); - - page = parseInt(page, 10); - - if (page === 1) { - // Page 1 is an alias, do a permanent 301 redirect - if (rssRegex.test(req.url)) { - return redirect301(res, req.originalUrl.replace(rssRegex, '/rss/')); - } else { - return redirect301(res, req.originalUrl.replace(pageRegex, '/')); - } - } else if (page < 1 || isNaN(page)) { - // Nothing less than 1 is a valid page number, go straight to a 404 - return next(new errors.NotFoundError()); - } else { - // Set req.params.page to the already parsed number, and continue - req.params.page = page; - return next(); - } - } - // ### Admin routes router.get(/^\/(logout|signout)\/$/, function redirectToSignout(req, res) { - redirect301(res, subdir + '/ghost/signout/'); + utils.redirect301(res, subdir + '/ghost/signout/'); }); router.get(/^\/signup\/$/, function redirectToSignup(req, res) { - redirect301(res, subdir + '/ghost/signup/'); + utils.redirect301(res, subdir + '/ghost/signup/'); }); // redirect to /ghost and let that do the authentication to prevent redirects to /ghost//admin etc. router.get(/^\/((ghost-admin|admin|wp-admin|dashboard|signin|login)\/?)$/, function redirectToAdmin(req, res) { - redirect301(res, subdir + '/ghost/'); + utils.redirect301(res, subdir + '/ghost/'); }); // password-protected frontend route @@ -71,46 +38,15 @@ frontendRoutes = function frontendRoutes(middleware) { frontend.private ); - rssRouter.route('/rss/').get(frontend.rss); - rssRouter.route('/rss/:page/').get(frontend.rss); - rssRouter.route('/feed/').get(function redirect(req, res) { - redirect301(res, subdir + '/rss/'); - }); - rssRouter.param('page', handlePageParam); - - // Index - indexRouter.route('/').get(frontend.index); - indexRouter.route('/' + routeKeywords.page + '/:page/').get(frontend.index); - indexRouter.param('page', handlePageParam); - indexRouter.use(rssRouter); - - // Tags - tagRouter.route('/').get(frontend.tag); - tagRouter.route('/' + routeKeywords.page + '/:page/').get(frontend.tag); - tagRouter.route('/edit?').get(function redirect(req, res) { - res.redirect(subdir + '/ghost/settings/tags/' + req.params.slug + '/'); - }); - tagRouter.param('page', handlePageParam); - tagRouter.use(rssRouter); - - // Authors - authorRouter.route('/').get(frontend.author); - authorRouter.route('/edit?').get(function redirect(req, res) { - res.redirect(subdir + '/ghost/team/' + req.params.slug + '/'); - }); - authorRouter.route('/' + routeKeywords.page + '/:page/').get(frontend.author); - authorRouter.param('page', handlePageParam); - authorRouter.use(rssRouter); - - // Mount the Routers - router.use('/' + routeKeywords.private + '/', privateRouter); - router.use('/' + routeKeywords.author + '/:slug/', authorRouter); - router.use('/' + routeKeywords.tag + '/:slug/', tagRouter); - router.use('/', indexRouter); - // Post Live Preview router.get('/' + routeKeywords.preview + '/:uuid', frontend.preview); + // Private + router.use('/' + routeKeywords.private + '/', privateRouter); + + // Channels + router.use(channels.router()); + // Default router.get('*', frontend.single); diff --git a/core/server/utils/index.js b/core/server/utils/index.js index 58c15cd51d..f89a53d78e 100644 --- a/core/server/utils/index.js +++ b/core/server/utils/index.js @@ -94,6 +94,11 @@ utils = { base64String += '='; } return base64String; + }, + redirect301: function redirect301(res, path) { + /*jslint unparam:true*/ + res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}); + res.redirect(301, path); } }; diff --git a/core/test/functional/routes/channel_spec.js b/core/test/functional/routes/channel_spec.js new file mode 100644 index 0000000000..351d0b6e28 --- /dev/null +++ b/core/test/functional/routes/channel_spec.js @@ -0,0 +1,554 @@ +/*global describe, it, before, after */ + +// # Channel Route Tests +// As it stands, these tests depend on the database, and as such are integration tests. +// These tests are here to cover the headers sent with requests and high-level redirects that can't be +// tested with the unit tests +var request = require('supertest'), + should = require('should'), + cheerio = require('cheerio'), + + testUtils = require('../../utils'), + ghost = require('../../../../core'); + +describe('Channel Routes', function () { + function doEnd(done) { + return function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + should.not.exist(res.headers['X-CSRF-Token']); + should.not.exist(res.headers['set-cookie']); + should.exist(res.headers.date); + + done(); + }; + } + + before(function (done) { + ghost().then(function (ghostServer) { + // Setup the request object with the ghost express app + request = request(ghostServer.rootApp); + + done(); + }).catch(function (e) { + console.log('Ghost Error: ', e); + console.log(e.stack); + }); + }); + + after(testUtils.teardown); + + describe('Index', function () { + it('should respond with html', function (done) { + request.get('/') + .expect('Content-Type', /html/) + .expect('Cache-Control', testUtils.cacheRules.public) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + var $ = cheerio.load(res.text); + + should.not.exist(res.headers['x-cache-invalidate']); + should.not.exist(res.headers['X-CSRF-Token']); + should.not.exist(res.headers['set-cookie']); + should.exist(res.headers.date); + + $('title').text().should.equal('Ghost'); + $('.content .post').length.should.equal(1); + $('.poweredby').text().should.equal('Proudly published with Ghost'); + $('body.home-template').length.should.equal(1); + $('article.post').length.should.equal(1); + $('article.tag-getting-started').length.should.equal(1); + + done(); + }); + }); + + it('should not have as second page', function (done) { + request.get('/page/2/') + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) + .end(doEnd(done)); + }); + + describe('RSS', function () { + before(function (done) { + testUtils.initData().then(function () { + return testUtils.fixtures.overrideOwnerUser(); + }).then(function () { + done(); + }); + }); + + after(testUtils.teardown); + + it('should 301 redirect with CC=1year without slash', function (done) { + request.get('/rss') + .expect('Location', '/rss/') + .expect('Cache-Control', testUtils.cacheRules.year) + .expect(301) + .end(doEnd(done)); + }); + + it('should respond with 200 & CC=public', function (done) { + request.get('/rss/') + .expect('Content-Type', 'text/xml; charset=utf-8') + .expect('Cache-Control', testUtils.cacheRules.public) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + should.not.exist(res.headers['x-cache-invalidate']); + should.not.exist(res.headers['X-CSRF-Token']); + should.not.exist(res.headers['set-cookie']); + should.exist(res.headers.date); + // The remainder of the XML is tested in the unit/xml_spec.js + res.text.should.match(/^<\?xml version="1.0" encoding="UTF-8"\?>