From ab49d1eed6b41cf1aad1df99386c9d5589809f9b Mon Sep 17 00:00:00 2001 From: kirrg001 Date: Sun, 21 Apr 2019 23:55:22 +0200 Subject: [PATCH] Added comments for routing service no issue - jsdoc - inline comments --- .../services/routing/CollectionRouter.js | 43 +++++++++- core/server/services/routing/ParentRouter.js | 81 ++++++++++++++++--- core/server/services/routing/PreviewRouter.js | 17 ++++ core/server/services/routing/RSSRouter.js | 17 ++++ .../services/routing/StaticPagesRouter.js | 24 ++++++ .../services/routing/StaticRoutesRouter.js | 36 +++++++++ .../server/services/routing/TaxonomyRouter.js | 31 +++++++ core/server/services/routing/bootstrap.js | 27 ++++++- .../services/routing/controllers/channel.js | 16 +++- .../routing/controllers/collection.js | 24 +++++- .../services/routing/controllers/entry.js | 11 ++- .../services/routing/controllers/preview.js | 12 ++- .../services/routing/controllers/rss.js | 15 +++- .../services/routing/controllers/static.js | 13 ++- .../services/routing/helpers/context.js | 4 +- .../services/routing/helpers/entry-lookup.js | 15 +++- core/server/services/routing/helpers/error.js | 7 +- .../services/routing/helpers/fetch-data.js | 21 ++--- .../routing/helpers/format-response.js | 8 +- .../routing/helpers/render-entries.js | 7 ++ .../services/routing/helpers/render-entry.js | 11 +-- .../services/routing/helpers/renderer.js | 13 ++- .../server/services/routing/helpers/secure.js | 15 +++- .../services/routing/helpers/templates.js | 15 ++-- .../routing/middlewares/page-param.js | 8 ++ core/server/services/routing/registry.js | 41 +++++++++- 26 files changed, 462 insertions(+), 70 deletions(-) diff --git a/core/server/services/routing/CollectionRouter.js b/core/server/services/routing/CollectionRouter.js index 7dfd45fb52..c679363622 100644 --- a/core/server/services/routing/CollectionRouter.js +++ b/core/server/services/routing/CollectionRouter.js @@ -7,6 +7,11 @@ const controllers = require('./controllers'); const middlewares = require('./middlewares'); const RSSRouter = require('./RSSRouter'); +/** + * @description Collection Router for post resource. + * + * Fundamental router to define where resources live and how their url structure is. + */ class CollectionRouter extends ParentRouter { constructor(mainRoute, object, RESOURCE_CONFIG) { super('CollectionRouter'); @@ -15,7 +20,7 @@ class CollectionRouter extends ParentRouter { this.routerName = mainRoute === '/' ? 'index' : mainRoute.replace(/\//g, ''); - // NOTE: index/parent route e.g. /, /podcast/, /magic/ ;) + // NOTE: this.route === index/parent route e.g. /, /podcast/, /magic/ this.route = { value: mainRoute }; @@ -55,6 +60,10 @@ class CollectionRouter extends ParentRouter { this._listeners(); } + /** + * @description Register all routes of this router. + * @private + */ _registerRoutes() { // REGISTER: context middleware for this collection this.router().use(this._prepareEntriesContext.bind(this)); @@ -85,8 +94,7 @@ class CollectionRouter extends ParentRouter { } /** - * We attach context information of the router to the request. - * By this we can e.g. access the router options in controllers. + * @description Prepare index context for further middlewares/controllers. */ _prepareEntriesContext(req, res, next) { res.routerOptions = { @@ -108,12 +116,19 @@ class CollectionRouter extends ParentRouter { next(); } + /** + * @description Prepare entry context for further middlewares/controllers. + */ _prepareEntryContext(req, res, next) { res.routerOptions.context = ['post']; res.routerOptions.type = 'entry'; next(); } + /** + * @description This router has listeners to react on changes which happen in Ghost. + * @private + */ _listeners() { /** * CASE: timezone changes @@ -126,6 +141,11 @@ class CollectionRouter extends ParentRouter { common.events.on('settings.active_timezone.edited', this._onTimezoneEditedListener); } + /** + * @description Helper function to handle a timezone change. + * @param settingModel + * @private + */ _onTimezoneEdited(settingModel) { const newTimezone = settingModel.attributes.value, previousTimezone = settingModel._previousAttributes.value; @@ -136,20 +156,37 @@ class CollectionRouter extends ParentRouter { if (this.getPermalinks().getValue().match(/:year|:month|:day/)) { debug('_onTimezoneEdited: trigger regeneration'); + + // @NOTE: The connected url generator will listen on this event and regenerate urls. this.emit('updated'); } } + /** + * @description Get resource type of this router (always "posts") + * @returns {string} + */ getResourceType() { + // @TODO: resourceAlias can be removed? We removed it. Looks like a last left over. Needs double checking. return this.RESOURCE_CONFIG.resourceAlias || this.RESOURCE_CONFIG.resource; } + /** + * @description Get index route e.g. /, /blog/ + * @param {Object} options + * @returns {String} + */ getRoute(options) { options = options || {}; return urlService.utils.createUrl(this.route.value, options.absolute, options.secure); } + /** + * @description Generate rss url. + * @param {Object} options + * @returns {String} + */ getRssUrl(options) { if (!this.rss) { return null; diff --git a/core/server/services/routing/ParentRouter.js b/core/server/services/routing/ParentRouter.js index 19d730575a..8d330d9500 100644 --- a/core/server/services/routing/ParentRouter.js +++ b/core/server/services/routing/ParentRouter.js @@ -1,8 +1,9 @@ /** - * # Router + * # Parent Router * - * A wrapper around express.Router - * Intended to be extended anywhere that routes need to be registered in Ghost + * A wrapper around express.Router, which is controlled in Ghost. + * + * Intended to be extended anywhere that routes need to be registered in Ghost. * Only allows for .use and .get at the moment - we don't have clear use-cases for anything else yet. */ @@ -13,9 +14,19 @@ const debug = require('ghost-ignition').debug('services:routing:ParentRouter'), url = require('url'), security = require('../../lib/security'), urlService = require('../url'), - // This the route registry for the whole site registry = require('./registry'); +/** + * @description Inherited express router, which gives control to us. + * + * Purposes: + * - give the router a correct name + * - give the router a correct parent + * + * @param {Object} options + * @returns {Express-Router} + * @constructor + */ function GhostRouter(options) { const router = express.Router(options); @@ -34,9 +45,6 @@ function GhostRouter(options) { return innerRouter; } -/** - * We expose a very limited amount of express.Router via specialist methods - */ class ParentRouter extends EventEmitter { constructor(name) { super(); @@ -47,6 +55,12 @@ class ParentRouter extends EventEmitter { this._router = GhostRouter({mergeParams: true, parent: this}); } + /** + * @description Helper function to find the site router in the express router stack. + * @param {Object} req + * @returns {Express-Router} + * @private + */ _getSiteRouter(req) { let siteRouter = null; @@ -62,10 +76,19 @@ class ParentRouter extends EventEmitter { return siteRouter; } + /** + * @description Helper function to handle redirects across routers. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @param {String} slug + * @private + */ _respectDominantRouter(req, res, next, slug) { let siteRouter = this._getSiteRouter(req); let targetRoute = null; + // CASE: iterate over routers and check whether a router has a redirect for the target slug enabled. siteRouter.handle.stack.every((router) => { if (router.handle.parent && router.handle.parent.isRedirectEnabled && router.handle.parent.isRedirectEnabled(this.getResourceType(), slug)) { targetRoute = router.handle.parent.getRoute(); @@ -93,6 +116,11 @@ class ParentRouter extends EventEmitter { next(); } + /** + * @description Mount a router on a router (sub-routing) + * @param {String} path + * @param {Express-Router} router + */ mountRouter(path, router) { if (arguments.length === 1) { router = path; @@ -105,12 +133,24 @@ class ParentRouter extends EventEmitter { } } + /** + * @description Mount a route on this router. + * @param {String} path + * @param {Function} controller + */ mountRoute(path, controller) { debug(this.name + ': mountRoute for', path, controller.name); registry.setRoute(this.name, path); this._router.get(path, controller); } + /** + * @description Unmount route. + * + * Not used at the moment, but useful to keep for e.g. deregister routes on runtime. + * + * @param {String} path + */ unmountRoute(path) { let indexToRemove = null; @@ -125,21 +165,38 @@ class ParentRouter extends EventEmitter { } } + /** + * @description Very important function to get the actual express router, which satisfies express. + * @returns {Express-Router} + */ router() { return this._router; } + /** + * @description Get configured permalinks of this router. + * @returns {Object} + */ getPermalinks() { return this.permalinks; } + /** + * @description Get configured filter of this router. + * @returns {String} + */ getFilter() { return this.filter; } /** - * Will return the full route including subdirectory. - * Do not use this function to mount routes for now, because the subdirectory is already mounted. + * @description Get main route of this router. + * + * Will return the full route including subdirectory. Do not use this function to mount routes for now, + * because the subdirectory is already mounted as exclusive feature (independent of dynamic routing). + * + * @param {Object} options + * @returns {String} */ getRoute(options) { options = options || {}; @@ -147,6 +204,12 @@ class ParentRouter extends EventEmitter { return urlService.utils.createUrl(this.route.value, options.absolute, options.secure); } + /** + * @description Figure out if the router has a redirect enabled. + * @param {String} routerType + * @param {String} slug + * @returns {boolean} + */ isRedirectEnabled(routerType, slug) { debug('isRedirectEnabled', this.name, this.route && this.route.value, routerType, slug); diff --git a/core/server/services/routing/PreviewRouter.js b/core/server/services/routing/PreviewRouter.js index f47fbc498c..91ab22ffad 100644 --- a/core/server/services/routing/PreviewRouter.js +++ b/core/server/services/routing/PreviewRouter.js @@ -2,23 +2,40 @@ const ParentRouter = require('./ParentRouter'); const urlService = require('../url'); const controllers = require('./controllers'); +/** + * @description Preview Router. + */ class PreviewRouter extends ParentRouter { constructor(RESOURCE_CONFIG) { super('PreviewRouter'); this.RESOURCE_CONFIG = RESOURCE_CONFIG.QUERY.preview; + // @NOTE: hardcoded, not configureable this.route = {value: '/p/'}; this._registerRoutes(); } + /** + * @description Register all routes of this router. + * @private + */ _registerRoutes() { + // REGISTER: prepare context this.router().use(this._prepareContext.bind(this)); + // REGISTER: actual preview route this.mountRoute(urlService.utils.urlJoin(this.route.value, ':uuid', ':options?'), controllers.preview); } + /** + * @description Prepare context for further middlewares/controllers. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @private + */ _prepareContext(req, res, next) { res.routerOptions = { type: 'entry', diff --git a/core/server/services/routing/RSSRouter.js b/core/server/services/routing/RSSRouter.js index 79131cae72..034d3bdf0a 100644 --- a/core/server/services/routing/RSSRouter.js +++ b/core/server/services/routing/RSSRouter.js @@ -4,6 +4,11 @@ const urlService = require('../url'); const controllers = require('./controllers'); const middlewares = require('./middlewares'); +/** + * @description RSS Router, which should be used as a sub-router in other routes. + * + * "/rss" -> RSS Router + */ class RSSRouter extends ParentRouter { constructor() { super('RSSRouter'); @@ -12,17 +17,29 @@ class RSSRouter extends ParentRouter { this._registerRoutes(); } + /** + * @description Register all routes of this router. + * @private + */ _registerRoutes() { this.mountRoute(this.route.value, controllers.rss); // REGISTER: pagination this.router().param('page', middlewares.pageParam); + + // REGISTER: actual rss route this.mountRoute(urlService.utils.urlJoin(this.route.value, ':page(\\d+)'), controllers.rss); // REGISTER: redirect rule this.mountRoute('/feed/', this._redirectFeedRequest.bind(this)); } + /** + * @description Simple controller function to redirect /rss to /feed + * @param {Object} req + * @param {Object}res + * @private + */ _redirectFeedRequest(req, res) { urlService .utils diff --git a/core/server/services/routing/StaticPagesRouter.js b/core/server/services/routing/StaticPagesRouter.js index 594699286c..7e8108735f 100644 --- a/core/server/services/routing/StaticPagesRouter.js +++ b/core/server/services/routing/StaticPagesRouter.js @@ -4,12 +4,16 @@ const ParentRouter = require('./ParentRouter'); const controllers = require('./controllers'); const common = require('../../lib/common'); +/** + * @description Resource: pages + */ class StaticPagesRouter extends ParentRouter { constructor(RESOURCE_CONFIG) { super('StaticPagesRouter'); this.RESOURCE_CONFIG = RESOURCE_CONFIG.QUERY.page; + // @NOTE: Permalink is always /:slug, not configure able this.permalinks = { value: '/:slug/' }; @@ -31,7 +35,12 @@ class StaticPagesRouter extends ParentRouter { this._registerRoutes(); } + /** + * @description Register all routes of this router. + * @private + */ _registerRoutes() { + // REGISTER: prepare context this.router().use(this._prepareContext.bind(this)); this.router().param('slug', this._respectDominantRouter.bind(this)); @@ -42,6 +51,13 @@ class StaticPagesRouter extends ParentRouter { common.events.emit('router.created', this); } + /** + * @description Prepare context for futher middlewares/controllers. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @private + */ _prepareContext(req, res, next) { res.routerOptions = { type: 'entry', @@ -55,10 +71,18 @@ class StaticPagesRouter extends ParentRouter { next(); } + /** + * @description Resource type. + * @returns {string} + */ getResourceType() { return 'pages'; } + /** + * @description This router has no index/default route. "/:slug/" is dynamic. + * @returns {null} + */ getRoute() { return null; } diff --git a/core/server/services/routing/StaticRoutesRouter.js b/core/server/services/routing/StaticRoutesRouter.js index 24e452f09b..d00e56a3ca 100644 --- a/core/server/services/routing/StaticRoutesRouter.js +++ b/core/server/services/routing/StaticRoutesRouter.js @@ -6,6 +6,9 @@ const controllers = require('./controllers'); const middlewares = require('./middlewares'); const ParentRouter = require('./ParentRouter'); +/** + * @description Template routes allow you to map individual URLs to specific template files within a Ghost theme + */ class StaticRoutesRouter extends ParentRouter { constructor(mainRoute, object) { super('StaticRoutesRouter'); @@ -17,6 +20,8 @@ class StaticRoutesRouter extends ParentRouter { debug(this.route.value, this.templates); + // CASE 1: Route is channel (controller: channel) - a stream of posts + // CASE 2: Route is just a static page e.g. landing page if (this.isChannel(object)) { this.templates = this.templates.reverse(); this.rss = object.rss !== false; @@ -35,7 +40,12 @@ class StaticRoutesRouter extends ParentRouter { } } + /** + * @description Register all channel routes of this router (...if the router is a channel) + * @private + */ _registerChannelRoutes() { + // REGISTER: prepare context object this.router().use(this._prepareChannelContext.bind(this)); // REGISTER: is rss enabled? @@ -54,6 +64,13 @@ class StaticRoutesRouter extends ParentRouter { common.events.emit('router.created', this); } + /** + * @description Prepare channel context for further middlewares/controllers. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @private + */ _prepareChannelContext(req, res, next) { res.routerOptions = { type: this.controller, @@ -69,13 +86,27 @@ class StaticRoutesRouter extends ParentRouter { next(); } + /** + * @description Register all static routes of this router (...if the router is just a static route) + * @private + */ _registerStaticRoute() { + // REGISTER: prepare context object this.router().use(this._prepareStaticRouteContext.bind(this)); + + // REGISTER: static route this.mountRoute(this.route.value, controllers.static); common.events.emit('router.created', this); } + /** + * @description Prepare static route context for further middlewares/controllers. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @private + */ _prepareStaticRouteContext(req, res, next) { res.routerOptions = { type: 'custom', @@ -89,6 +120,11 @@ class StaticRoutesRouter extends ParentRouter { next(); } + /** + * @description Helper function to figure out if this router is a channel. + * @param {Object} object + * @returns {boolean} + */ isChannel(object) { if (object && object.controller && object.controller === 'channel') { return true; diff --git a/core/server/services/routing/TaxonomyRouter.js b/core/server/services/routing/TaxonomyRouter.js index 969a19433b..5103e80de1 100644 --- a/core/server/services/routing/TaxonomyRouter.js +++ b/core/server/services/routing/TaxonomyRouter.js @@ -6,6 +6,10 @@ const urlService = require('../url'); const controllers = require('./controllers'); const middlewares = require('./middlewares'); +/** + * @description Taxonomies are groupings of posts based on a common relation. + * Taxonomies do not change the url of a resource. + */ class TaxonomyRouter extends ParentRouter { constructor(key, permalinks, RESOURCE_CONFIG) { super('Taxonomy'); @@ -26,10 +30,15 @@ class TaxonomyRouter extends ParentRouter { this._registerRoutes(); } + /** + * @description Register all routes of this router. + * @private + */ _registerRoutes() { // REGISTER: context middleware this.router().use(this._prepareContext.bind(this)); + // REGISTER: redirects across routers this.router().param('slug', this._respectDominantRouter.bind(this)); // REGISTER: enable rss by default @@ -43,11 +52,19 @@ class TaxonomyRouter extends ParentRouter { this.router().param('page', middlewares.pageParam); this.mountRoute(urlService.utils.urlJoin(this.permalinks.value, 'page', ':page(\\d+)'), controllers.channel); + // REGISTER: edit redirect to admin client e.g. /tag/:slug/edit this.mountRoute(urlService.utils.urlJoin(this.permalinks.value, 'edit'), this._redirectEditOption.bind(this)); common.events.emit('router.created', this); } + /** + * @description Prepare context for routing middlewares/controllers. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @private + */ _prepareContext(req, res, next) { res.routerOptions = { type: 'channel', @@ -64,14 +81,28 @@ class TaxonomyRouter extends ParentRouter { next(); } + /** + * @description Simple controller function to redirect /edit to admin client + * @param {Object} req + * @param {Object} res + * @private + */ _redirectEditOption(req, res) { urlService.utils.redirectToAdmin(302, res, this.RESOURCE_CONFIG.TAXONOMIES[this.taxonomyKey].editRedirect.replace(':slug', req.params.slug)); } + /** + * @description Get resource type e.g. tags or authors + * @returns {*} + */ getResourceType() { return this.RESOURCE_CONFIG.TAXONOMIES[this.taxonomyKey].resource; } + /** + * @description There is no default/index route for taxonomies e.g. /tag/ does not exist, only /tag/:slug/ + * @returns {null} + */ getRoute() { return null; } diff --git a/core/server/services/routing/bootstrap.js b/core/server/services/routing/bootstrap.js index 89d60269f6..345e738f20 100644 --- a/core/server/services/routing/bootstrap.js +++ b/core/server/services/routing/bootstrap.js @@ -13,6 +13,19 @@ const ParentRouter = require('./ParentRouter'); const registry = require('./registry'); let siteRouter; +/** + * @description The `init` function will return the wrapped parent express router and will start creating all + * routers if you pass the option "start: true". + * + * CASES: + * - if Ghost starts, it will first init the site app with the wrapper router and then call `start` + * separately, because it could be that your blog goes into maintenance mode + * - if you upload your routes.yaml in the admin client, we will re-initialise routing + * - + * + * @param {Object} options + * @returns {ExpressRouter} + */ module.exports.init = (options = {start: false}) => { debug('bootstrap'); @@ -32,10 +45,17 @@ module.exports.init = (options = {start: false}) => { }; /** - * Create a set of default and dynamic routers defined in the routing yaml. + * @description This function will create the routers based on the routes.yaml config. * - * @TODO: - * - is the PreviewRouter an app? + * The routers are created in a specific order. This order defines who can get a resource first or + * who can dominant other routers. + * + * 1. Preview Router: Is the strongest and is an inbuilt feature, which you can never override. + * 2. Static Routes: Very strong, because you can override any urls and redirect to a static route. + * 3. Taxonomies: Stronger than collections, because it's an inbuilt feature. + * 4. Collections + * 5. Static Pages: Weaker than collections, because we first try to find a post slug and fallback to lookup a static page. + * 6. Apps: Weakest */ module.exports.start = () => { const apiVersion = themeService.getApiVersion(); @@ -46,6 +66,7 @@ module.exports.start = () => { siteRouter.mountRouter(previewRouter.router()); registry.setRouter('previewRouter', previewRouter); + // NOTE: Get the routes.yaml config const dynamicRoutes = settingsService.get('routes'); _.each(dynamicRoutes.routes, (value, key) => { diff --git a/core/server/services/routing/controllers/channel.js b/core/server/services/routing/controllers/channel.js index 8b6903cb81..8008305694 100644 --- a/core/server/services/routing/controllers/channel.js +++ b/core/server/services/routing/controllers/channel.js @@ -5,7 +5,16 @@ const _ = require('lodash'), themes = require('../../themes'), helpers = require('../helpers'); -// @TODO: the collection+rss controller does almost the same +/** + * @description Channel controller. + * + * @TODO: The collection+rss controller do almost the same. Merge! + * + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @returns {Promise} + */ module.exports = function channelController(req, res, next) { debug('channelController', req.params, res.routerOptions); @@ -46,11 +55,10 @@ module.exports = function channelController(req, res, next) { } // Format data 1 - // @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. + // @TODO: See helpers/secure for explanation. helpers.secure(req, result.posts); - // @TODO: get rid of this O_O + // @TODO: See helpers/secure for explanation. _.each(result.data, function (data) { helpers.secure(req, data); }); diff --git a/core/server/services/routing/controllers/collection.js b/core/server/services/routing/controllers/collection.js index 4d94e37a76..c2dc5da89d 100644 --- a/core/server/services/routing/controllers/collection.js +++ b/core/server/services/routing/controllers/collection.js @@ -6,6 +6,13 @@ const _ = require('lodash'), themes = require('../../themes'), helpers = require('../helpers'); +/** + * @description Collection controller. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @returns {Promise} + */ module.exports = function collectionController(req, res, next) { debug('collectionController', req.params, res.routerOptions); @@ -47,7 +54,17 @@ module.exports = function collectionController(req, res, next) { debug(result.posts.length); - // CASE: does this post belong to this collection? + /** + * CASE: + * + * Does this post belong to this collection? + * A post can only live in one collection. If you make use of multiple collections and you mis-use your routes.yaml, + * it can happen that your database query will load the same posts, but we cannot show a post on two + * different urls. This helper is only a prevention, but it's not a solution for the user, because + * it will break pagination (e.g. you load 10 posts from database, but you only render 9). + * + * People should always invert their filters to ensure that the database query loads unique posts per collection. + */ result.posts = _.filter(result.posts, (post) => { if (urlService.owns(res.routerOptions.identifier, post.id)) { return post; @@ -57,11 +74,10 @@ module.exports = function collectionController(req, res, next) { }); // Format data 1 - // @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. + // @TODO: See helpers/secure for explanation. helpers.secure(req, result.posts); - // @TODO: get rid of this O_O + // @TODO: See helpers/secure for explanation. _.each(result.data, function (data) { helpers.secure(req, data); }); diff --git a/core/server/services/routing/controllers/entry.js b/core/server/services/routing/controllers/entry.js index 79319de05a..21fb121d26 100644 --- a/core/server/services/routing/controllers/entry.js +++ b/core/server/services/routing/controllers/entry.js @@ -4,9 +4,11 @@ const debug = require('ghost-ignition').debug('services:routing:controllers:entr helpers = require('../helpers'); /** - * @TODO: - * - use `filter` for `findOne`? - * - always execute `next` until no router want's to serve and 404's + * @description Entry controller. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @returns {Promise} */ module.exports = function entryController(req, res, next) { debug('entryController', res.routerOptions); @@ -64,7 +66,8 @@ module.exports = function entryController(req, res, next) { * Ensure we redirect to the correct post url including subdirectory. * * @NOTE: - * This file is used for v0.1 and v2. v0.1 returns relative urls, v2 returns absolute urls. + * Keep in mind, that the logic here is used for v0.1 and v2. + * v0.1 returns relative urls, v2 returns absolute urls. * * @TODO: * Simplify if we drop v0.1. diff --git a/core/server/services/routing/controllers/preview.js b/core/server/services/routing/controllers/preview.js index e6d4b5c8e2..ebab3c7ac6 100644 --- a/core/server/services/routing/controllers/preview.js +++ b/core/server/services/routing/controllers/preview.js @@ -2,6 +2,13 @@ const debug = require('ghost-ignition').debug('services:routing:controllers:prev urlService = require('../../url'), helpers = require('../helpers'); +/** + * @description Preview Controller. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @returns {Promise} + */ module.exports = function previewController(req, res, next) { debug('previewController'); @@ -10,6 +17,7 @@ module.exports = function previewController(req, res, next) { const params = { uuid: req.params.uuid, status: 'all', + // @TODO: Remove "author" if we drop v0.1 include: 'author,authors,tags' }; @@ -23,7 +31,8 @@ module.exports = function previewController(req, res, next) { } if (req.params.options && req.params.options.toLowerCase() === 'edit') { - // @TODO: we don't know which resource type it is, because it's a generic preview handler + // @TODO: we don't know which resource type it is, because it's a generic preview handler and the + // preview API returns {previews: []} // @TODO: figure out how to solve better const resourceType = post.page ? 'page' : 'post'; @@ -38,6 +47,7 @@ module.exports = function previewController(req, res, next) { return urlService.utils.redirect301(res, urlService.getUrlByResourceId(post.id, {withSubdirectory: true})); } + // @TODO: See helpers/secure helpers.secure(req, post); const renderer = helpers.renderEntry(req, res); diff --git a/core/server/services/routing/controllers/rss.js b/core/server/services/routing/controllers/rss.js index dbdd5590fa..55e7a97b21 100644 --- a/core/server/services/routing/controllers/rss.js +++ b/core/server/services/routing/controllers/rss.js @@ -7,7 +7,7 @@ const _ = require('lodash'), rssService = require('../../rss'), helpers = require('../helpers'); -// @TODO: is this the right logic? move to UrlService utils +// @TODO: move to services/url/utils function getBaseUrlForRSSReq(originalUrl, pageParam) { return url.parse(originalUrl).pathname.replace(new RegExp('/' + pageParam + '/$'), '/'); } @@ -21,7 +21,14 @@ function getTitle(relatedData) { return titleStart + settingsCache.get('title'); } -// @TODO: the collection controller does almost the same +/** + * @description RSS controller. + * + * @TODO: The collection controller does almost the same. Merge! + * @param {Object} req + * @param {Object} res + * @param {Function} next + */ module.exports = function rssController(req, res, next) { debug('rssController'); @@ -30,8 +37,8 @@ module.exports = function rssController(req, res, next) { slug: req.params.slug ? security.string.safe(req.params.slug) : undefined }; - // CASE: we are using an rss cache - url must be normalised (without pagination) - // @TODO: this belongs to the rss service + // CASE: Ghost is using an rss cache - we have to normalise the (without pagination) + // @TODO: This belongs to the rss service O_o const baseUrl = getBaseUrlForRSSReq(req.originalUrl, pathOptions.page); helpers.fetchData(pathOptions, res.routerOptions, res.locals) diff --git a/core/server/services/routing/controllers/static.js b/core/server/services/routing/controllers/static.js index 2d9f3c094d..218a73454a 100644 --- a/core/server/services/routing/controllers/static.js +++ b/core/server/services/routing/controllers/static.js @@ -14,6 +14,7 @@ function processQuery(query, locals) { // We override the `include` property for now, because the full data set is required anyway. if (_.get(query, 'resource') === 'posts') { _.extend(query.options, { + // @TODO: Remove "author" when we drop v0.1 include: 'author,authors,tags' }); } @@ -26,10 +27,16 @@ function processQuery(query, locals) { }); } - // Return a promise for the api query return api[query.controller][query.type](query.options); } +/** + * @description Static route controller. + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @returns {Promise} + */ module.exports = function staticController(req, res, next) { debug('staticController', res.routerOptions); @@ -51,13 +58,13 @@ module.exports = function staticController(req, res, next) { if (config.type === 'browse') { response.data[name].meta = result[name].meta; - // @TODO: remove in v3 + // @TODO: remove in Ghost 3.0 (see https://github.com/TryGhost/Ghost/issues/10434) response.data[name][config.resource] = result[name][config.resource]; } }); } - // @TODO: get rid of this O_O + // @TODO: See helpers/secure for more context. _.each(response.data, function (data) { helpers.secure(req, data); }); diff --git a/core/server/services/routing/helpers/context.js b/core/server/services/routing/helpers/context.js index 816605c233..897b84fc6c 100644 --- a/core/server/services/routing/helpers/context.js +++ b/core/server/services/routing/helpers/context.js @@ -11,7 +11,7 @@ * 3. data - used for telling the difference between posts and pages */ const labs = require('../../labs'), - // @TODO: fix this, this is app specific and should be dynamic + // @TODO: fix this!! These regexes are app specific and should be dynamic. They should not belong here.... // routeKeywords.private: 'private' privatePattern = new RegExp('^\\/private\\/'), // routeKeywords.subscribe: 'subscribe' @@ -27,6 +27,7 @@ function setResponseContext(req, res, data) { res.locals.context = []; // If we don't have a relativeUrl, we can't detect the context, so return + // See shared/middlewares/ghost-locals if (!res.locals.relativeUrl) { return; } @@ -63,6 +64,7 @@ function setResponseContext(req, res, data) { } } + // @TODO: remove first if condition when we drop v0.1 if (data && data.post && data.post.page) { if (!res.locals.context.includes('page')) { res.locals.context.push('page'); diff --git a/core/server/services/routing/helpers/entry-lookup.js b/core/server/services/routing/helpers/entry-lookup.js index 4ccd98adf7..0138f6e29d 100644 --- a/core/server/services/routing/helpers/entry-lookup.js +++ b/core/server/services/routing/helpers/entry-lookup.js @@ -5,6 +5,13 @@ const debug = require('ghost-ignition').debug('services:routing:helpers:entry-lo const routeMatch = require('path-match')(); const config = require('../../../config'); +/** + * @description Query API for a single entry/resource. + * @param {String} postUrl + * @param {Object} routerOptions + * @param {Objec†} locals + * @returns {*} + */ function entryLookup(postUrl, routerOptions, locals) { debug(postUrl); @@ -32,11 +39,11 @@ function entryLookup(postUrl, routerOptions, locals) { isEditURL = true; } - /** - * Query database to find entry. - * @deprecated: `author`, will be removed in Ghost 3.0 - */ let options = { + /** + * @deprecated: `author`, will be removed in Ghost 3.0 + * @TODO: Remove "author" when we drop v0.1 + */ include: 'author,authors,tags' }; diff --git a/core/server/services/routing/helpers/error.js b/core/server/services/routing/helpers/error.js index 62cca657b4..ce658479fd 100644 --- a/core/server/services/routing/helpers/error.js +++ b/core/server/services/routing/helpers/error.js @@ -1,8 +1,13 @@ const common = require('../../../lib/common'); +/** + * @description Centralised error handling for API requests. + * @param {Function} next + * @returns {Closure} handleError + */ function handleError(next) { return function handleError(err) { - // If we've thrown an error message of type: 'NotFound' then we found no path match. + // CASE: if we've thrown an error message of type: 'NotFound' then we found no path match, try next router! if (err.errorType === 'NotFoundError') { return next(); } diff --git a/core/server/services/routing/helpers/fetch-data.js b/core/server/services/routing/helpers/fetch-data.js index 9a3ab46775..ff824278dd 100644 --- a/core/server/services/routing/helpers/fetch-data.js +++ b/core/server/services/routing/helpers/fetch-data.js @@ -16,29 +16,31 @@ const queryDefaults = { }; /** - * @deprecated: `author`, will be removed in Ghost 3.0 + * The theme expects to have access to the relations by default e.g. {{post.authors}} */ const defaultQueryOptions = { options: { + /** + * @deprecated: `author`, will be removed in Ghost 3.0 + * @TODO: Remove "author" when we drop v0.1 + */ include: 'author,authors,tags' } }; -/** - * Default post query needs to always include author, authors & tags - */ const defaultPostQuery = _.cloneDeep(queryDefaults); defaultPostQuery.options = defaultQueryOptions.options; /** - * ## Process Query + * @description Process query request. + * * Takes a 'query' object, ensures that type, resource and options are set * Replaces occurrences of `%s` in options with slugParam * Converts the query config to a promise for the result * - * @param {{type: String, resource: String, options: Object}} query + * @param {Object} query * @param {String} slugParam - * @returns {Promise} promise for an API call + * @returns {Promise} */ function processQuery(query, slugParam, locals) { const api = require('../../../api')[locals.apiVersion]; @@ -55,12 +57,13 @@ function processQuery(query, slugParam, locals) { if (config.get('enableDeveloperExperiments')) { query.options.context = {member: locals.member}; } - // Return a promise for the api query + return (api[query.controller] || api[query.resource])[query.type](query.options); } /** - * ## Fetch Data + * @description Fetch data from API helper for controllers. + * * Calls out to get posts per page, builds the final posts query & builds any additional queries * Wraps the queries using Promise.props to ensure it gets named responses * Does a first round of formatting on the response, and returns diff --git a/core/server/services/routing/helpers/format-response.js b/core/server/services/routing/helpers/format-response.js index f0b0c5c979..77fbf5e1b9 100644 --- a/core/server/services/routing/helpers/format-response.js +++ b/core/server/services/routing/helpers/format-response.js @@ -1,8 +1,8 @@ const _ = require('lodash'); /** - * formats variables for handlebars in multi-post contexts. - * If extraValues are available, they are merged in the final value + * @description Formats API response into handlebars/theme format. + * * @return {Object} containing page variables */ function formatPageResponse(result) { @@ -32,12 +32,14 @@ function formatPageResponse(result) { } /** - * similar to formatPageResponse, but for entries (post or page) + * @description Format a single resource for handlebars. * * @TODO * In the future, we should return {page: entry} or {post:entry). * But for now, we would break the themes if we just change it. * + * @see https://github.com/TryGhost/Ghost/issues/10042. + * * @return {Object} containing page variables */ function formatResponse(post) { diff --git a/core/server/services/routing/helpers/render-entries.js b/core/server/services/routing/helpers/render-entries.js index 6ef58c2765..bd3d1e4750 100644 --- a/core/server/services/routing/helpers/render-entries.js +++ b/core/server/services/routing/helpers/render-entries.js @@ -2,6 +2,13 @@ const debug = require('ghost-ignition').debug('services:routing:helpers:render-e formatResponse = require('./format-response'), renderer = require('./renderer'); +/** + * @description Helper to handle rendering multiple resources. + * + * @param {Object} req + * @param {Object} res + * @returns {Closure) + */ module.exports = function renderEntries(req, res) { debug('renderEntries called'); return function renderEntries(result) { diff --git a/core/server/services/routing/helpers/render-entry.js b/core/server/services/routing/helpers/render-entry.js index 8b1f4b22d5..2c684bdd38 100644 --- a/core/server/services/routing/helpers/render-entry.js +++ b/core/server/services/routing/helpers/render-entry.js @@ -1,11 +1,12 @@ const debug = require('ghost-ignition').debug('services:routing:helpers:render-post'), formatResponse = require('./format-response'), renderer = require('./renderer'); -/* - * Sets the response context around an entry (post or page) - * and renders it with the correct template. - * Used by post preview and entry methods. - * Returns a function that takes the entry to be rendered. +/** + * @description Helper to handle rendering multiple resources. + * + * @param {Object} req + * @param {Object} res + * @returns {Closure) */ module.exports = function renderEntry(req, res) { debug('renderEntry called'); diff --git a/core/server/services/routing/helpers/renderer.js b/core/server/services/routing/helpers/renderer.js index 1b14ce8275..2515638c0d 100644 --- a/core/server/services/routing/helpers/renderer.js +++ b/core/server/services/routing/helpers/renderer.js @@ -2,22 +2,29 @@ const debug = require('ghost-ignition').debug('services:routing:helpers:renderer setContext = require('./context'), templates = require('./templates'); +/** + * @description Helper function to finally render the data. + * @param {Object} req + * @param {Object} res + * @param {Object} data + */ module.exports = function renderer(req, res, data) { - // Context + // Set response context setContext(req, res, data); - // Template + // Set template templates.setTemplate(req, res, data); - // Render Call debug('Rendering template: ' + res._template + ' for: ' + req.originalUrl); debug('res.locals', res.locals); + // CASE: You can set the content type of the page in your routes.yaml file if (res.routerOptions && res.routerOptions.contentType) { if (res.routerOptions.templates.indexOf(res._template) !== -1) { res.type(res.routerOptions.contentType); } } + // Render Call res.render(res._template, data); }; diff --git a/core/server/services/routing/helpers/secure.js b/core/server/services/routing/helpers/secure.js index c91e4b2e3d..4c96054071 100644 --- a/core/server/services/routing/helpers/secure.js +++ b/core/server/services/routing/helpers/secure.js @@ -1,6 +1,15 @@ -// TODO: figure out how to remove the need for this -// Add Request context parameter to the data object -// to be passed down to the templates +/** + * @description Tiny/Weird helper to attach the information if the request is HTTPS or HTTP. + * + * It's used in services/url/utils (search for ".secure"). + * + * We forward the resource into handlebars and if you use the URL helper, we want to know if the original request + * was https or not to generate the correct URL...but that is only true, if your blog url is set to HTTP, but NGINX supports HTTPS. + * + * @TODO: Drop in Ghost 3.0, because we will only support https. + * @param {Object} req + * @param {Object} data + */ function setRequestIsSecure(req, data) { (Array.isArray(data) ? data : [data]).forEach(function forEach(d) { d.secure = req.secure; diff --git a/core/server/services/routing/helpers/templates.js b/core/server/services/routing/helpers/templates.js index 654622134f..0ad42f84ae 100644 --- a/core/server/services/routing/helpers/templates.js +++ b/core/server/services/routing/helpers/templates.js @@ -2,7 +2,6 @@ // // Figure out which template should be used to render a request // based on the templates which are allowed, and what is available in the theme -// TODO: consider where this should live as it deals with collections, entries, and errors const _ = require('lodash'), path = require('path'), url = require('url'), @@ -11,7 +10,7 @@ const _ = require('lodash'), _private = {}; /** - * ## Get Error Template Hierarchy + * @description Get Error Template Hierarchy * * Fetch the ordered list of templates that can be used to render this error statusCode. * @@ -34,7 +33,7 @@ _private.getErrorTemplateHierarchy = function getErrorTemplateHierarchy(statusCo }; /** - * ## Get Template Hierarchy + * @description Get Template Hierarchy * * Fetch the ordered list of templates that can be used to render this request. * 'index' is the default / fallback @@ -72,7 +71,7 @@ _private.getEntriesTemplateHierarchy = function getEntriesTemplateHierarchy(rout }; /** - * ## Get Entry Template Hierarchy + * @description Get Entry Template Hierarchy * * Fetch the ordered list of templates that can be used to render this request. * 'post' is the default / fallback @@ -101,7 +100,7 @@ _private.getEntryTemplateHierarchy = function getEntryTemplateHierarchy(postObje }; /** - * ## Pick Template + * @description Pick Template * * Taking the ordered list of allowed templates for this request * Cycle through and find the first one which has a match in the theme @@ -157,6 +156,12 @@ _private.getTemplateForError = function getTemplateForError(statusCode) { return _private.pickTemplate(templateList, fallback); }; +/** + * @description Set template for express. Express will render the template you set here. + * @param {Object} req + * @param {Object} res + * @param {Object} data + */ module.exports.setTemplate = function setTemplate(req, res, data) { if (res._template && !req.err) { return; diff --git a/core/server/services/routing/middlewares/page-param.js b/core/server/services/routing/middlewares/page-param.js index 4ed75640c1..38e2ad03c1 100644 --- a/core/server/services/routing/middlewares/page-param.js +++ b/core/server/services/routing/middlewares/page-param.js @@ -1,6 +1,14 @@ const common = require('../../../lib/common'), urlService = require('../../url'); +/** + * @description Middleware, which validates and interprets the page param e.g. /page/1 + * @param {Object} req + * @param {Object} res + * @param {Function} next + * @param {Number} page + * @returns {*} + */ module.exports = function handlePageParam(req, res, next, page) { // routeKeywords.page: 'page' const pageRegex = new RegExp('/page/(.*)?/'), diff --git a/core/server/services/routing/registry.js b/core/server/services/routing/registry.js index 88538d1033..abda8ee16f 100644 --- a/core/server/services/routing/registry.js +++ b/core/server/services/routing/registry.js @@ -2,31 +2,64 @@ const _ = require('lodash'); let routes = []; let routers = {}; +/** + * @description The router registry is helpful for debugging purposes and let's you search existing routes and routers. + */ module.exports = { + /** + * @description Get's called if you register a url pattern in express. + * @param {String} routerName + * @param {String} route + */ setRoute(routerName, route) { routes.push({route: route, from: routerName}); }, + /** + * @description Get's called if you register a router in express. + * @param {String} name + * @param {Express-Router} router + */ setRouter(name, router) { routers[name] = router; }, + /** + * @description Get all registered routes. + * @returns {Array} + */ getAllRoutes() { return _.cloneDeep(routes); }, + /** + * @description Get router by name. + * @param {String} name + * @returns {Express-Router} + */ getRouter(name) { return routers[name]; }, /** - * https://github.com/TryGhost/Team/issues/65#issuecomment-393622816 + * * * Hierarchy for primary rss url: * * - index collection (/) * - if you only have one collection, we take this rss url */ + /** + * @description Helper to figure out the primary rss url. + * + * If you either configure multiple collections or your collection does not live on "/", we need to know + * what your primary rss url is, otherwise /rss could be 404. + * + * More context: https://github.com/TryGhost/Team/issues/65#issuecomment-393622816 + * + * @param {Object} options + * @returns {String} + */ getRssUrl(options) { let rssUrl = null; @@ -55,10 +88,16 @@ module.exports = { return rssUrl; }, + /** + * @description Reset all routes. + */ resetAllRoutes() { routes = []; }, + /** + * @description Reset all routers. + */ resetAllRouters() { _.each(routers, (value) => { value.reset();