From 0046dce39f5c4266eb65cdfb8a078e34591a1fbf Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Thu, 21 Jun 2018 15:46:42 +0200 Subject: [PATCH] Dynamic Routing Beta: Better template support refs #9601 - single or multiple template definition - possible formats: ``` routes: /about/: about ``` ``` routes: /about/: template: about ``` ``` routes: /about/: template: - about - me ``` ``` collections /posts/: template: - posts - general ``` ``` collections /posts/: template: posts ``` --- core/server/apps/amp/lib/router.js | 2 +- .../apps/private-blogging/lib/router.js | 2 +- core/server/apps/subscribers/lib/router.js | 2 +- .../services/routing/CollectionRouter.js | 2 +- .../services/routing/StaticRoutesRouter.js | 9 +- .../services/routing/helpers/templates.js | 2 +- core/server/services/settings/validate.js | 116 +++++++++++------- core/test/integration/web/site_spec.js | 8 +- .../services/routing/CollectionRouter_spec.js | 4 +- .../routing/StaticRoutesRouter_spec.js | 70 +++++++++++ .../routing/helpers/templates_spec.js | 4 +- .../unit/services/settings/validate_spec.js | 81 +++++++++++- 12 files changed, 237 insertions(+), 65 deletions(-) create mode 100644 core/test/unit/services/routing/StaticRoutesRouter_spec.js diff --git a/core/server/apps/amp/lib/router.js b/core/server/apps/amp/lib/router.js index 63b95ca9c4..63d9698ea0 100644 --- a/core/server/apps/amp/lib/router.js +++ b/core/server/apps/amp/lib/router.js @@ -13,7 +13,7 @@ function _renderer(req, res, next) { // @TODO refactor into to something explicit & DRY this up res._route = { type: 'custom', - templateName: templateName, + templates: templateName, defaultTemplate: path.resolve(__dirname, 'views', templateName + '.hbs') }; diff --git a/core/server/apps/private-blogging/lib/router.js b/core/server/apps/private-blogging/lib/router.js index 465730fd6d..76f1bf3af4 100644 --- a/core/server/apps/private-blogging/lib/router.js +++ b/core/server/apps/private-blogging/lib/router.js @@ -12,7 +12,7 @@ function _renderer(req, res) { // @TODO refactor into to something explicit & DRY this up res._route = { type: 'custom', - templateName: templateName, + templates: templateName, defaultTemplate: path.resolve(__dirname, 'views', templateName + '.hbs') }; diff --git a/core/server/apps/subscribers/lib/router.js b/core/server/apps/subscribers/lib/router.js index 38b2a06c52..59c0dd07fb 100644 --- a/core/server/apps/subscribers/lib/router.js +++ b/core/server/apps/subscribers/lib/router.js @@ -18,7 +18,7 @@ function _renderer(req, res) { // @TODO refactor into to something explicit & DRY this up res._route = { type: 'custom', - templateName: templateName, + templates: templateName, defaultTemplate: path.resolve(__dirname, 'views', templateName + '.hbs') }; diff --git a/core/server/services/routing/CollectionRouter.js b/core/server/services/routing/CollectionRouter.js index b8af31adf1..339561dcbf 100644 --- a/core/server/services/routing/CollectionRouter.js +++ b/core/server/services/routing/CollectionRouter.js @@ -27,7 +27,7 @@ class CollectionRouter extends ParentRouter { }; // @NOTE: see helpers/templates - we use unshift to prepend the templates - this.templates = (object.template || []).reverse(); + this.templates = (object.templates || []).reverse(); this.filter = object.filter || 'page:false'; diff --git a/core/server/services/routing/StaticRoutesRouter.js b/core/server/services/routing/StaticRoutesRouter.js index 966d0b0f6a..bb10f2537a 100644 --- a/core/server/services/routing/StaticRoutesRouter.js +++ b/core/server/services/routing/StaticRoutesRouter.js @@ -4,13 +4,13 @@ const helpers = require('./helpers'); const ParentRouter = require('./ParentRouter'); class StaticRoutesRouter extends ParentRouter { - constructor(key, template) { + constructor(key, object) { super('StaticRoutesRouter'); this.route = {value: key}; - this.template = template; + this.templates = object.templates || []; - debug(this.route.value, this.template); + debug(this.route.value, this.templates); this._registerRoutes(); } @@ -24,9 +24,10 @@ class StaticRoutesRouter extends ParentRouter { } _prepareContext(req, res, next) { + // @TODO: index.hbs as fallback for static routes O_O res._route = { type: 'custom', - templateName: this.template, + templates: this.templates, defaultTemplate: 'index' }; diff --git a/core/server/services/routing/helpers/templates.js b/core/server/services/routing/helpers/templates.js index c0c487bdd3..c0936fe92a 100644 --- a/core/server/services/routing/helpers/templates.js +++ b/core/server/services/routing/helpers/templates.js @@ -171,7 +171,7 @@ module.exports.setTemplate = function setTemplate(req, res, data) { switch (routeConfig.type) { case 'custom': - res._template = _private.pickTemplate(routeConfig.templateName, routeConfig.defaultTemplate); + res._template = _private.pickTemplate(routeConfig.templates, routeConfig.defaultTemplate); break; case 'collection': res._template = _private.getTemplateForCollection(res.locals.routerOptions, { diff --git a/core/server/services/settings/validate.js b/core/server/services/settings/validate.js index 26d04ea249..82479f8748 100644 --- a/core/server/services/settings/validate.js +++ b/core/server/services/settings/validate.js @@ -2,6 +2,29 @@ const _ = require('lodash'); const common = require('../../lib/common'); const _private = {}; +_private.validateTemplate = function validateTemplate(object) { + // CASE: /about/: about + if (typeof object === 'string') { + return { + templates: [object] + }; + } + + if (!object.hasOwnProperty('template')) { + object.templates = []; + return object; + } + + if (_.isArray(object.template)) { + object.templates = object.template; + } else { + object.templates = [object.template]; + } + + delete object.template; + return object; +}; + _private.validateRoutes = function validateRoutes(routes) { _.each(routes, (routingTypeObject, routingTypeObjectKey) => { // CASE: we hard-require trailing slashes for the index route @@ -34,6 +57,8 @@ _private.validateRoutes = function validateRoutes(routes) { help: 'e.g. permalink: /{slug}/' }); } + + routes[routingTypeObjectKey] = _private.validateTemplate(routingTypeObject); }); return routes; @@ -72,53 +97,54 @@ _private.validateCollections = function validateCollections(collections) { } // CASE: validate permalink key - if (routingTypeObject.hasOwnProperty('permalink')) { - if (!routingTypeObject.permalink) { - throw new common.errors.ValidationError({ - message: common.i18n.t('errors.services.settings.yaml.validate', { - at: routingTypeObjectKey, - reason: 'Please define a permalink route.' - }), - help: 'e.g. permalink: /{slug}/' - }); - } - // CASE: we hard-require trailing slashes for the value/permalink route - if (!routingTypeObject.permalink.match(/\/$/) && !routingTypeObject.permalink.match(/globals\.permalinks/)) { - throw new common.errors.ValidationError({ - message: common.i18n.t('errors.services.settings.yaml.validate', { - at: routingTypeObject.permalink, - reason: 'A trailing slash is required.' - }) - }); - } - - // CASE: we hard-require leading slashes for the value/permalink route - if (!routingTypeObject.permalink.match(/^\//) && !routingTypeObject.permalink.match(/globals\.permalinks/)) { - throw new common.errors.ValidationError({ - message: common.i18n.t('errors.services.settings.yaml.validate', { - at: routingTypeObject.permalink, - reason: 'A leading slash is required.' - }) - }); - } - - // CASE: notation /:slug/ or /:primary_author/ is not allowed. We only accept /{{...}}/. - if (routingTypeObject.permalink && routingTypeObject.permalink.match(/\/\:\w+/)) { - throw new common.errors.ValidationError({ - message: common.i18n.t('errors.services.settings.yaml.validate', { - at: routingTypeObject.permalink, - reason: 'Please use the following notation e.g. /{slug}/.' - }) - }); - } - - // CASE: transform {.*} into :\w+ notation. This notation is our internal notation e.g. see permalink - // replacement in our UrlService utility. - if (routingTypeObject.permalink.match(/{.*}/)) { - routingTypeObject.permalink = routingTypeObject.permalink.replace(/{(\w+)}/g, ':$1'); - } + if (!routingTypeObject.permalink) { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.services.settings.yaml.validate', { + at: routingTypeObjectKey, + reason: 'Please define a permalink route.' + }), + help: 'e.g. permalink: /{slug}/' + }); } + + // CASE: we hard-require trailing slashes for the value/permalink route + if (!routingTypeObject.permalink.match(/\/$/) && !routingTypeObject.permalink.match(/globals\.permalinks/)) { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.services.settings.yaml.validate', { + at: routingTypeObject.permalink, + reason: 'A trailing slash is required.' + }) + }); + } + + // CASE: we hard-require leading slashes for the value/permalink route + if (!routingTypeObject.permalink.match(/^\//) && !routingTypeObject.permalink.match(/globals\.permalinks/)) { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.services.settings.yaml.validate', { + at: routingTypeObject.permalink, + reason: 'A leading slash is required.' + }) + }); + } + + // CASE: notation /:slug/ or /:primary_author/ is not allowed. We only accept /{{...}}/. + if (routingTypeObject.permalink && routingTypeObject.permalink.match(/\/\:\w+/)) { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.services.settings.yaml.validate', { + at: routingTypeObject.permalink, + reason: 'Please use the following notation e.g. /{slug}/.' + }) + }); + } + + // CASE: transform {.*} into :\w+ notation. This notation is our internal notation e.g. see permalink + // replacement in our UrlService utility. + if (routingTypeObject.permalink.match(/{.*}/)) { + routingTypeObject.permalink = routingTypeObject.permalink.replace(/{(\w+)}/g, ':$1'); + } + + collections[routingTypeObjectKey] = _private.validateTemplate(routingTypeObject); }); return collections; diff --git a/core/test/integration/web/site_spec.js b/core/test/integration/web/site_spec.js index 0f5d78b1cb..e2bb314f54 100644 --- a/core/test/integration/web/site_spec.js +++ b/core/test/integration/web/site_spec.js @@ -722,7 +722,7 @@ describe('Integration - Web - Site', function () { collections: { '/': { permalink: '/:slug/', - template: ['default'] + templates: ['default'] }, '/magic/': { permalink: '/magic/:slug/' @@ -792,7 +792,7 @@ describe('Integration - Web - Site', function () { collections: { '/': { permalink: '/:slug/', - template: ['something', 'default'] + templates: ['something', 'default'] } } }); @@ -844,11 +844,11 @@ describe('Integration - Web - Site', function () { collections: { '/': { permalink: '/:slug/', - template: ['something', 'default'] + templates: ['something', 'default'] }, '/magic/': { permalink: '/magic/:slug/', - template: ['something', 'default'] + templates: ['something', 'default'] } } }); diff --git a/core/test/unit/services/routing/CollectionRouter_spec.js b/core/test/unit/services/routing/CollectionRouter_spec.js index 39d4e4de58..adde371496 100644 --- a/core/test/unit/services/routing/CollectionRouter_spec.js +++ b/core/test/unit/services/routing/CollectionRouter_spec.js @@ -129,7 +129,7 @@ describe('UNIT - services/routing/CollectionRouter', function () { }); it('with templates', function () { - const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/', template: ['home', 'index']}); + const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/', templates: ['home', 'index']}); // they are getting reversed because we unshift the templates in the helper collectionRouter.templates.should.eql(['index', 'home']); @@ -138,7 +138,7 @@ describe('UNIT - services/routing/CollectionRouter', function () { describe('fn: _prepareIndexContext', function () { it('default', function () { - const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/', template: ['home', 'index']}); + const collectionRouter = new CollectionRouter('/magic/', {permalink: '/:slug/', templates: ['home', 'index']}); collectionRouter._prepareIndexContext(req, res, next); diff --git a/core/test/unit/services/routing/StaticRoutesRouter_spec.js b/core/test/unit/services/routing/StaticRoutesRouter_spec.js new file mode 100644 index 0000000000..9bdd4b91ff --- /dev/null +++ b/core/test/unit/services/routing/StaticRoutesRouter_spec.js @@ -0,0 +1,70 @@ +const should = require('should'), + sinon = require('sinon'), + settingsCache = require('../../../../server/services/settings/cache'), + common = require('../../../../server/lib/common'), + StaticRoutesRouter = require('../../../../server/services/routing/StaticRoutesRouter'), + configUtils = require('../../../utils/configUtils'), + sandbox = sinon.sandbox.create(); + +describe('UNIT - services/routing/StaticRoutesRouter', function () { + let req, res, next; + + afterEach(function () { + configUtils.restore(); + }); + + beforeEach(function () { + sandbox.stub(settingsCache, 'get').withArgs('permalinks').returns('/:slug/'); + + sandbox.stub(common.events, 'emit'); + sandbox.stub(common.events, 'on'); + + sandbox.spy(StaticRoutesRouter.prototype, 'mountRoute'); + sandbox.spy(StaticRoutesRouter.prototype, 'mountRouter'); + + req = sandbox.stub(); + res = sandbox.stub(); + next = sandbox.stub(); + + res.locals = {}; + }); + + afterEach(function () { + sandbox.restore(); + }); + + describe('static routes', function () { + it('instantiate: default', function () { + const staticRoutesRouter = new StaticRoutesRouter('/about/', {templates: ['test']}); + should.exist(staticRoutesRouter.router); + + should.not.exist(staticRoutesRouter.getFilter()); + should.not.exist(staticRoutesRouter.getPermalinks()); + + staticRoutesRouter.templates.should.eql(['test']); + + common.events.emit.calledOnce.should.be.true(); + common.events.emit.calledWith('router.created', staticRoutesRouter).should.be.true(); + + staticRoutesRouter.mountRoute.callCount.should.eql(1); + + // parent route + staticRoutesRouter.mountRoute.args[0][0].should.eql('/about/'); + staticRoutesRouter.mountRoute.args[0][1].should.eql(staticRoutesRouter._renderStaticRoute.bind(staticRoutesRouter)); + }); + + it('fn: _prepareContext', function () { + const staticRoutesRouter = new StaticRoutesRouter('/about/', {templates: []}); + + staticRoutesRouter._prepareContext(req, res, next); + next.called.should.be.true(); + res._route.should.eql({ + type: 'custom', + templates: [], + defaultTemplate: 'index' + }); + + res.locals.routerOptions.should.eql({context: []}); + }); + }); +}); diff --git a/core/test/unit/services/routing/helpers/templates_spec.js b/core/test/unit/services/routing/helpers/templates_spec.js index 43c1176ad7..eb5b723e3a 100644 --- a/core/test/unit/services/routing/helpers/templates_spec.js +++ b/core/test/unit/services/routing/helpers/templates_spec.js @@ -423,7 +423,7 @@ describe('templates', function () { it('calls pickTemplate for custom routes', function () { res._route = { type: 'custom', - templateName: 'test', + templates: 'test', defaultTemplate: 'path/to/local/test.hbs' }; @@ -445,7 +445,7 @@ describe('templates', function () { it('calls pickTemplate for custom routes', function () { res._route = { type: 'custom', - templateName: 'test', + templates: 'test', defaultTemplate: 'path/to/local/test.hbs' }; diff --git a/core/test/unit/services/settings/validate_spec.js b/core/test/unit/services/settings/validate_spec.js index bb0ab698bc..198556bad1 100644 --- a/core/test/unit/services/settings/validate_spec.js +++ b/core/test/unit/services/settings/validate_spec.js @@ -205,7 +205,8 @@ describe('UNIT: services/settings/validate', function () { routes: {}, collections: { '/magic/': { - permalink: '{globals.permalinks}' + permalink: '{globals.permalinks}', + templates: [] } } }); @@ -253,12 +254,86 @@ describe('UNIT: services/settings/validate', function () { }, collections: { '/magic/': { - permalink: '/magic/:year/:slug/' + permalink: '/magic/:year/:slug/', + templates: [] }, '/': { - permalink: '/:slug/' + permalink: '/:slug/', + templates: [] } } }); }); + + describe('template definitions', function () { + it('single value', function () { + const object = validate({ + routes: { + '/about/': 'about', + '/me/': { + template: 'me' + } + }, + collections: { + '/': { + permalink: '/{slug}/', + template: 'test' + } + } + }); + + object.should.eql({ + taxonomies: {}, + routes: { + '/about/': { + templates: ['about'] + }, + '/me/': { + templates: ['me'] + } + }, + collections: { + '/': { + permalink: '/:slug/', + templates: ['test'] + } + } + }); + }); + + it('array', function () { + const object = validate({ + routes: { + '/about/': 'about', + '/me/': { + template: ['me'] + } + }, + collections: { + '/': { + permalink: '/{slug}/', + template: ['test'] + } + } + }); + + object.should.eql({ + taxonomies: {}, + routes: { + '/about/': { + templates: ['about'] + }, + '/me/': { + templates: ['me'] + } + }, + collections: { + '/': { + permalink: '/:slug/', + templates: ['test'] + } + } + }); + }); + }); });