Skip to content

Commit

Permalink
Transfer of route paths from hard-code to config
Browse files Browse the repository at this point in the history 
issue TryGhost#4519
- Removal of hard-coded route paths from controller file
- Creation of route config in example config file
- New aliases for other rss paths
- Substitution of hard-coded paths in response context creation
- Added unit tests for new routes and aliases
  • Loading branch information
@katiefenn
katiefenn authored and Katie Fenn committed Jan 20, 2015
1 parent c946230 commit cbb88e5
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 33 deletions.
117 changes: 112 additions & 5 deletions config.example.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,28 @@ config = {
host: '127.0.0.1',
// Port to be passed to node's `net.Server#listen()`, for iisnode set this to `process.env.PORT`
port: '2368'
}
},
routes: {
rss: {path: '/rss', controller: 'rss'},
pageRss: {path: '/rss/:page/', controller: 'rss'},
authorRss: {path: '/author/:slug/rss/', controller: 'rss'},
authorPageRss: {path: '/author/:slug/rss/:page/', controller: 'rss'},
tagPage: {path: '/tag/:slug/page/:page/', controller: 'tag'},
tagRss: {path: '/tag/:slug/rss/', controller: 'rss'},
tagPageRss: {path: '/tag/:slug/rss/:page', controller: 'rss'},
tag: {path: '/tag/:slug/', controller: 'tag'},
authorPage: {path: '/author/:slug/page/:page/', controller: 'author'},
author: {path: '/author/:slug/', controller: 'author'},
page: {path: '/page/:page/', controller: 'homepage'},
homepage: {path: '/', controller: 'homepage'},
single: {path: '*', controller: 'single'}
},
aliases: [
{reqPath: '/feed/', resPath: '/rss/', status: 301},
{reqPath: '/feed/:page/', resPath: '/rss/:page/', status: 301},
{reqPath: '/author/:slug/feed/', resPath: '/author/:slug/rss/', status: 301},
{reqPath: '/author/:slug/feed/:page/', resPath: '/author/:slug/rss/:page/', status: 301}
]
},

// ### Development **(default)**
Expand Down Expand Up @@ -64,7 +85,30 @@ config = {
},
paths: {
contentPath: path.join(__dirname, '/content/')
}
},
// Routes to map URL paths to controllers that handle requests
routes: {
rss: {path: '/rss', controller: 'rss'},
pageRss: {path: '/rss/:page/', controller: 'rss'},
authorRss: {path: '/author/:slug/rss/', controller: 'rss'},
authorPageRss: {path: '/author/:slug/rss/:page/', controller: 'rss'},
tagPage: {path: '/tag/:slug/page/:page/', controller: 'tag'},
tagRss: {path: '/tag/:slug/rss/', controller: 'rss'},
tagPageRss: {path: '/tag/:slug/rss/:page', controller: 'rss'},
tag: {path: '/tag/:slug/', controller: 'tag'},
authorPage: {path: '/author/:slug/page/:page/', controller: 'author'},
author: {path: '/author/:slug/', controller: 'author'},
page: {path: '/page/:page/', controller: 'homepage'},
homepage: {path: '/', controller: 'homepage'},
single: {path: '*', controller: 'single'}
},
// Aliases to redirect alternative paths
aliases: [
{reqPath: '/feed/', resPath: '/rss/', status: 301},
{reqPath: '/feed/:page/', resPath: '/rss/:page/', status: 301},
{reqPath: '/author/:slug/feed/', resPath: '/author/:slug/rss/', status: 301},
{reqPath: '/author/:slug/feed/:page/', resPath: '/author/:slug/rss/:page/', status: 301}
]
},

// **Developers only need to edit below here**
Expand All @@ -84,7 +128,28 @@ config = {
host: '127.0.0.1',
port: '2369'
},
logging: false
logging: false,
routes: {
rss: {path: '/rss', controller: 'rss'},
pageRss: {path: '/rss/:page/', controller: 'rss'},
authorRss: {path: '/author/:slug/rss/', controller: 'rss'},
authorPageRss: {path: '/author/:slug/rss/:page/', controller: 'rss'},
tagPage: {path: '/tag/:slug/page/:page/', controller: 'tag'},
tagRss: {path: '/tag/:slug/rss/', controller: 'rss'},
tagPageRss: {path: '/tag/:slug/rss/:page', controller: 'rss'},
tag: {path: '/tag/:slug/', controller: 'tag'},
authorPage: {path: '/author/:slug/page/:page/', controller: 'author'},
author: {path: '/author/:slug/', controller: 'author'},
page: {path: '/page/:page/', controller: 'homepage'},
homepage: {path: '/', controller: 'homepage'},
single: {path: '*', controller: 'single'}
},
aliases: [
{reqPath: '/feed/', resPath: '/rss/', status: 301},
{reqPath: '/feed/:page/', resPath: '/rss/:page/', status: 301},
{reqPath: '/author/:slug/feed/', resPath: '/author/:slug/rss/', status: 301},
{reqPath: '/author/:slug/feed/:page/', resPath: '/author/:slug/rss/:page/', status: 301}
]
},

// ### Testing MySQL
Expand All @@ -105,7 +170,28 @@ config = {
host: '127.0.0.1',
port: '2369'
},
logging: false
logging: false,
routes: {
rss: {path: '/rss', controller: 'rss'},
pageRss: {path: '/rss/:page/', controller: 'rss'},
authorRss: {path: '/author/:slug/rss/', controller: 'rss'},
authorPageRss: {path: '/author/:slug/rss/:page/', controller: 'rss'},
tagPage: {path: '/tag/:slug/page/:page/', controller: 'tag'},
tagRss: {path: '/tag/:slug/rss/', controller: 'rss'},
tagPageRss: {path: '/tag/:slug/rss/:page', controller: 'rss'},
tag: {path: '/tag/:slug/', controller: 'tag'},
authorPage: {path: '/author/:slug/page/:page/', controller: 'author'},
author: {path: '/author/:slug/', controller: 'author'},
page: {path: '/page/:page/', controller: 'homepage'},
homepage: {path: '/', controller: 'homepage'},
single: {path: '*', controller: 'single'}
},
aliases: [
{reqPath: '/feed/', resPath: '/rss/', status: 301},
{reqPath: '/feed/:page/', resPath: '/rss/:page/', status: 301},
{reqPath: '/author/:slug/feed/', resPath: '/author/:slug/rss/', status: 301},
{reqPath: '/author/:slug/feed/:page/', resPath: '/author/:slug/rss/:page/', status: 301}
]
},

// ### Testing pg
Expand All @@ -126,7 +212,28 @@ config = {
host: '127.0.0.1',
port: '2369'
},
logging: false
logging: false,
routes: {
rss: {path: '/rss', controller: 'rss'},
pageRss: {path: '/rss/:page/', controller: 'rss'},
authorRss: {path: '/author/:slug/rss/', controller: 'rss'},
authorPageRss: {path: '/author/:slug/rss/:page/', controller: 'rss'},
tagPage: {path: '/tag/:slug/page/:page/', controller: 'tag'},
tagRss: {path: '/tag/:slug/rss/', controller: 'rss'},
tagPageRss: {path: '/tag/:slug/rss/:page', controller: 'rss'},
tag: {path: '/tag/:slug/', controller: 'tag'},
authorPage: {path: '/author/:slug/page/:page/', controller: 'author'},
author: {path: '/author/:slug/', controller: 'author'},
page: {path: '/page/:page/', controller: 'homepage'},
homepage: {path: '/', controller: 'homepage'},
single: {path: '*', controller: 'single'}
},
aliases: [
{reqPath: '/feed/', resPath: '/rss/', status: 301},
{reqPath: '/feed/:page/', resPath: '/rss/:page/', status: 301},
{reqPath: '/author/:slug/feed/', resPath: '/author/:slug/rss/', status: 301},
{reqPath: '/author/:slug/feed/:page/', resPath: '/author/:slug/rss/:page/', status: 301}
]
}
};

Expand Down
15 changes: 9 additions & 6 deletions core/server/controllers/frontend.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,23 +90,26 @@ function handleError(next) {

function setResponseContext(req, res, data) {
var contexts = [],
pageParam = req.params.page !== undefined ? parseInt(req.params.page, 10) : 1;
pageParam = req.params.page !== undefined ? parseInt(req.params.page, 10) : 1,
routes = config.routes;

// paged context
if (!isNaN(pageParam) && pageParam > 1) {
contexts.push('paged');
}

if (req.route.path === '/page/:page/') {
if (req.route.path === routes.page.path) {
contexts.push('index');
} else if (req.route.path === '/') {
} else if (req.route.path === routes.homepage.path) {
contexts.push('home');
contexts.push('index');
} else if (/\/rss\/(:page\/)?$/.test(req.route.path)) {
} else if (_.has([routes.rss.path, routes.pageRss.path], req.route.path)) {
contexts.push('rss');
} else if (/^\/tag\//.test(req.route.path)) {
} else if (_.has([routes.tag.path, routes.tagPage.path], req.route.path)) {
contexts.push('tag');
} else if (/^\/author\//.test(req.route.path)) {
} else if (
_.has([routes.author.path, routes.authorPage.path, routes.authorPageRss.path], req.route.path)
) {
contexts.push('author');
} else if (data && data.post && data.post.page) {
contexts.push('page');
Expand Down
47 changes: 25 additions & 22 deletions core/server/routes/frontend.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var frontend = require('../controllers/frontend'),
config = require('../config'),
express = require('express'),
utils = require('../utils'),
_ = require('lodash'),

frontendRoutes;

Expand All @@ -28,30 +29,32 @@ frontendRoutes = function () {
});

// ### Frontend routes
router.get('/rss/', frontend.rss);
router.get('/rss/:page/', frontend.rss);
router.get('/feed/', function redirect(req, res) {
/*jshint unused:true*/
res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S});
res.redirect(301, subdir + '/rss/');
_.forOwn(config.routes, function configureRoute(route) {
router.get(route.path, frontend[route.controller]);
});

// Tags
router.get('/tag/:slug/rss/', frontend.rss);
router.get('/tag/:slug/rss/:page/', frontend.rss);
router.get('/tag/:slug/page/:page/', frontend.tag);
router.get('/tag/:slug/', frontend.tag);

// Authors
router.get('/author/:slug/rss/', frontend.rss);
router.get('/author/:slug/rss/:page/', frontend.rss);
router.get('/author/:slug/page/:page/', frontend.author);
router.get('/author/:slug/', frontend.author);

// Default
router.get('/page/:page/', frontend.homepage);
router.get('/', frontend.homepage);
router.get('*', frontend.single);
// ### Frontend aliases
_.forOwn(config.aliases, function configureAlias(aliasConfig) {
router.get(aliasConfig.reqPath, function redirect(req, res) {
// Get response path from config
var aliasConfig = _.find(config.aliases, function isPath(alias) {
return alias.reqPath === this.path;
}, req.route),
path = aliasConfig.resPath;

// Substitute route params
_.forOwn(req.params, function (value, key) {
path = path.replace(':' + key, value);
});

if (_.has(aliasConfig, 'status')) {
res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S});
res.redirect(aliasConfig.status, subdir + path);
} else {
res.redirect(subdir + path);
}
});
});

return router;
};
Expand Down
52 changes: 52 additions & 0 deletions core/test/functional/routes/frontend_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,14 @@ describe('Frontend Routing', function () {
.end(doEnd(done));
});

it('should redirect /feed/1/ to /rss/1/', function (done) {
request.get('/feed/1/')
.expect('Location', '/rss/1/')
.expect('Cache-Control', testUtils.cacheRules.year)
.expect(301)
.end(doEnd(done));
});

it('should respond with xml', function (done) {
request.get('/rss/2/')
.expect('Content-Type', /xml/)
Expand Down Expand Up @@ -552,6 +560,14 @@ describe('Frontend Routing', function () {
.end(doEnd(done));
});

it('should redirect /author/ghost-owner/feed/ to /author/ghost-owner/rss/', function (done) {
request.get('/author/ghost-owner/feed/')
.expect('Location', '/author/ghost-owner/rss/')
.expect('Cache-Control', testUtils.cacheRules.year)
.expect(301)
.end(doEnd(done));
});

it('should respond with xml', function (done) {
request.get('/author/ghost-owner/rss/')
.expect('Content-Type', /xml/)
Expand All @@ -560,6 +576,12 @@ describe('Frontend Routing', function () {
.end(doEnd(done));
});

it('should redirect /author/ghost-owner/feed/1/ to /author/ghost-owner/rss/1/', function (done) {
request.get('/author/ghost-owner/feed/1/')
.expect('Location', '/author/ghost-owner/rss/1/')
.end(doEnd(done));
});

it('should redirect page 1', function (done) {
request.get('/author/ghost-owner/rss/1/')
.expect('Location', '/author/ghost-owner/rss/')
Expand Down Expand Up @@ -688,6 +710,36 @@ describe('Frontend Routing', function () {
});
});

describe('Route aliases', function () {
before(function (done) {
testUtils.initData().then(function () {
return testUtils.fixtures.overrideOwnerUser();
}).then(function () {
done();
});
});

after(testUtils.teardown);

it('should redirect an alias to its destination path', function (done) {
request.get('/feed/')
.expect('Location', '/rss/')
.end(doEnd(done));
});

it('should redirect using status code configuration when set', function (done) {
request.get('/feed/')
.expect(301)
.end(doEnd(done));
});

it('should substitute parameters from request paths to response paths', function (done) {
request.get('/feed/1/')
.expect('Location', '/rss/1/')
.end(doEnd(done));
});
});

describe('Subdirectory (no slash)', function () {
var forkedGhost, request;
before(function (done) {
Expand Down

0 comments on commit cbb88e5

Please sign in to comment.