Skip to content

Commit

Permalink
Moved fetching client out of our ghost_head helper (TryGhost#9180)
Browse files Browse the repository at this point in the history
refs TryGhost#8995

- move the getClient lookup from ghost_head into middleware
- use res.locals to keep track of the information (res.locals.client)
- make the middleware global to all frontend routes
- ghost_head: get locals from options.data not this (!)
- adapt lot's of tests
  • Loading branch information
kirrg001 authored Oct 26, 2017
1 parent c8cbbc4 commit 88eab98
Show file tree
Hide file tree
Showing 5 changed files with 755 additions and 494 deletions.
6 changes: 5 additions & 1 deletion core/server/blog/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ var debug = require('ghost-ignition').debug('blog'),

// Global/shared middleware
cacheControl = require('../middleware/cache-control'),
urlRedirects = require('../middleware/url-redirects'),
errorHandler = require('../middleware/error-handler'),
frontendClient = require('../middleware/frontend-client'),
maintenance = require('../middleware/maintenance'),
prettyURLs = require('../middleware/pretty-urls'),
urlRedirects = require('../middleware/url-redirects'),

// local middleware
servePublicFile = require('../middleware/serve-public-file'),
Expand Down Expand Up @@ -112,6 +113,9 @@ module.exports = function setupBlogApp() {
// Blog frontend is cacheable
blogApp.use(cacheControl('public'));

// Fetch the frontend client into res.locals
blogApp.use(frontendClient);

debug('General middleware done');

// Set up Frontend routes (including private blogging routes)
Expand Down
81 changes: 52 additions & 29 deletions core/server/helpers/ghost_head.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,18 @@

var proxy = require('./proxy'),
_ = require('lodash'),
Promise = require('bluebird'),
debug = require('ghost-ignition').debug('ghost_head'),

getMetaData = proxy.metaData.get,
getAssetUrl = proxy.metaData.getAssetUrl,
escapeExpression = proxy.escapeExpression,
SafeString = proxy.SafeString,
filters = proxy.filters,
labs = proxy.labs,
api = proxy.api,
logging = proxy.logging,
settingsCache = proxy.settingsCache,
config = proxy.config,
blogIconUtils = proxy.blogIcon;

function getClient() {
if (labs.isSet('publicAPI') === true) {
return api.clients
.read({slug: 'ghost-frontend'})
.then(function handleClient(client) {
client = client.clients[0];

if (client.status === 'enabled') {
return {
id: client.slug,
secret: client.secret
};
}

return {};
});
}
return Promise.resolve({});
}

function writeMetaTag(property, content, type) {
type = type || property.substring(0, 7) === 'twitter' ? 'name' : 'property';
return '<meta ' + type + '="' + property + '" content="' + content + '" />';
Expand Down Expand Up @@ -82,27 +59,73 @@ function getAjaxHelper(clientId, clientSecret) {
'</script>';
}

/**
* **NOTE**
* Express adds `_locals`, see https://github.com/expressjs/express/blob/4.15.4/lib/response.js#L962.
* But `options.data.root.context` is available next to `root._locals.context`, because
* Express creates a `renderOptions` object, see https://github.com/expressjs/express/blob/4.15.4/lib/application.js#L554
* and merges all locals to the root of the object. Very confusing, because the data is available in different layers.
*
* Express forwards the data like this to the hbs engine:
* {
* post: {}, - res.render('view', databaseResponse)
* context: ['post'], - from res.locals
* safeVersion: '1.x', - from res.locals
* _locals: {
* context: ['post'],
* safeVersion: '1.x'
* }
* }
*
* hbs forwards the data to any hbs helper like this
* {
* data: {
* blog: {},
* labs: {},
* config: {},
* root: {
* post: {},
* context: ['post'],
* locals: {...}
* }
* }
*
* `blog`, `labs` and `config` are the templateOptions, search for `hbs.updateTemplateOptions` in the code base.
* Also see how the root object get's created, https://github.com/wycats/handlebars.js/blob/v4.0.6/lib/handlebars/runtime.js#L259
*/
module.exports = function ghost_head(options) {
debug('begin');

// if server error page do nothing
if (this.statusCode >= 500) {
if (options.data.root.statusCode >= 500) {
return;
}

var head = [],
dataRoot = options.data.root,
context = dataRoot._locals.context ? dataRoot._locals.context : null,
client = dataRoot._locals.client,
safeVersion = dataRoot._locals.safeVersion,
postCodeInjection = dataRoot && dataRoot.post ? dataRoot.post.codeinjection_head : null,
globalCodeinjection = settingsCache.get('ghost_head'),
postCodeInjection = options.data.root && options.data.root.post ? options.data.root.post.codeinjection_head : null,
context = this.context ? this.context : null,
useStructuredData = !config.isPrivacyDisabled('useStructuredData'),
safeVersion = this.safeVersion,
referrerPolicy = config.get('referrerPolicy') ? config.get('referrerPolicy') : 'no-referrer-when-downgrade',
favicon = blogIconUtils.getIconUrl(),
iconType = blogIconUtils.getIconType(favicon);

debug('preparation complete, begin fetch');
return Promise
.join(getMetaData(this, options.data.root), getClient(), function handleData(metaData, client) {

/**
* @TODO:
* - getMetaData(dataRoot, dataRoot) -> yes that looks confusing!
* - there is a very mixed usage of `data.context` vs. `root.context` vs `root._locals.context` vs. `this.context`
* - NOTE: getMetaData won't live here anymore soon, see https://github.com/TryGhost/Ghost/issues/8995
* - therefor we get rid of using `getMetaData(this, dataRoot)`
* - dataRoot has access to *ALL* locals, see function description
* - it should not break anything
*/
return getMetaData(dataRoot, dataRoot)
.then(function handleMetaData(metaData) {
debug('end fetch');

if (context) {
Expand Down
29 changes: 29 additions & 0 deletions core/server/middleware/frontend-client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
var api = require('../api'),
labs = require('../utils/labs'),
logging = require('../logging');

module.exports = function getFrontendClient(req, res, next) {
if (labs.isSet('publicAPI') !== true) {
return next();
}

return api.clients
.read({slug: 'ghost-frontend'})
.then(function handleClient(client) {
client = client.clients[0];

if (client.status === 'enabled') {
res.locals.client = {
id: client.slug,
secret: client.secret
};
}

next();
})
.catch(function (err) {
// Log the error, but carry on as this is non-critical
logging.error(err);
next();
});
};
Loading

0 comments on commit 88eab98

Please sign in to comment.