From 32bbf2ba572b6b6af8fd32eb87ed58d776106e64 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Sun, 16 Jun 2013 20:22:01 +0100 Subject: [PATCH 1/2] issue #165, issue #124 - cleaning up ghostGlobals - ghost.js - globals/globalConfig has become settings / settingsCache to make it clearer - app.js - the ghostGlobals local cache is gone, and the use of res.locals has been cleaned up and simplified, although this needs to be properly split into frontend and admin locals (to be finished in #124) - frontend/index.js - doesn't need to be passed globals and nav properties as res.locals does this for us --- app.js | 42 ++++++++++-------------------- core/frontend/controllers/index.js | 4 +-- core/ghost.js | 4 +-- 3 files changed, 18 insertions(+), 32 deletions(-) diff --git a/app.js b/app.js index 40867cc606..9dc64f6f05 100755 --- a/app.js +++ b/app.js @@ -18,20 +18,19 @@ filters = require('./core/frontend/filters'), helpers = require('./core/frontend/helpers'), - // ## Variables + // ## Custom Middleware auth, authAPI, ghostLocals, + + // ## Variables loading = when.defer(), /** * Create new Ghost object * @type {Ghost} */ - ghost = new Ghost(), - // This is assigned after the call to ghost.init() below - ghostGlobals; - + ghost = new Ghost(); ghost.app().configure('development', function () { @@ -44,8 +43,6 @@ ghost.app().use(express.cookieSession({ cookie: { maxAge: 60000000 }})); ghost.app().use(ghost.initTheme(ghost.app())); ghost.app().use(flash()); - // bind locals - options which appear in every view - perhaps this should be admin only - }); /** @@ -75,18 +72,19 @@ /** * Expose the standard locals that every external page should have available; - * path, navItems and ghostGlobals + * path, navItems and settingsCache */ ghostLocals = function (req, res, next) { ghost.doFilter('ghostNavItems', {path: req.path, navItems: []}, function (navData) { // Make sure we have a locals value. res.locals = res.locals || {}; - - - // Extend it with nav data and ghostGlobals + // Extend it with nav data and settings _.extend(res.locals, navData, { - ghostGlobals: ghost.globals() + messages: req.flash(), + settings: ghost.settings(), + availableThemes: ghost.paths().availableThemes, + availablePlugins: ghost.paths().availablePlugins }); next(); @@ -97,21 +95,9 @@ ghost.loaded = loading.promise; when.all([ghost.init(), filters.loadCoreFilters(ghost), helpers.loadCoreHelpers(ghost)]).then(function () { - // Assign the globals we have loaded - ghostGlobals = ghost.globals(); - ghost.app().use(function (req, res, next) { - res.locals.messages = req.flash(); - res.locals.siteTitle = ghostGlobals.title; - res.locals.siteDescription = ghostGlobals.description; - res.locals.siteUrl = ghostGlobals.url; - res.locals.activeTheme = ghostGlobals.activeTheme; - res.locals.activePlugin = ghostGlobals.activePlugin; - res.locals.availableThemes = ghost.paths().availableThemes; - res.locals.availablePlugins = ghost.paths().availablePlugins; - - next(); - }); + // post init config + ghost.app().use(ghostLocals); /** * API routes.. @@ -151,8 +137,8 @@ * Frontend routes.. * @todo dynamic routing, homepage generator, filters ETC ETC */ - ghost.app().get('/:slug', ghostLocals, frontend.single); - ghost.app().get('/', ghostLocals, frontend.homepage); + ghost.app().get('/:slug', frontend.single); + ghost.app().get('/', frontend.homepage); ghost.app().listen(3333, function () { console.log("Express server listening on port " + 3333); diff --git a/core/frontend/controllers/index.js b/core/frontend/controllers/index.js index 4815865c83..bc0ac314c9 100644 --- a/core/frontend/controllers/index.js +++ b/core/frontend/controllers/index.js @@ -16,14 +16,14 @@ 'homepage': function (req, res) { api.posts.browse().then(function (page) { ghost.doFilter('prePostsRender', page.posts, function (posts) { - res.render('index', {posts: posts, ghostGlobals: res.locals.ghostGlobals, navItems: res.locals.navItems}); + res.render('index', {posts: posts}); }); }); }, 'single': function (req, res) { api.posts.read({'slug': req.params.slug}).then(function (post) { ghost.doFilter('prePostsRender', post.toJSON(), function (post) { - res.render('single', {post: post, ghostGlobals: res.locals.ghostGlobals, navItems: res.locals.navItems}); + res.render('single', {post: post}); }); }); } diff --git a/core/ghost.js b/core/ghost.js index ed7926fc87..79633432f3 100644 --- a/core/ghost.js +++ b/core/ghost.js @@ -94,7 +94,7 @@ config: function () { return config; }, // there's no management here to be sure this has loaded - globals: function () { return instance.globalConfig; }, + settings: function () { return instance.settingsCache; }, dataProvider: models, statuses: function () { return statuses; }, polyglot: function () { return polyglot; }, @@ -134,7 +134,7 @@ } }); - self.globalConfig = settings; + self.settingsCache = settings; }, errors.logAndThrowError); }, errors.logAndThrowError); }; From 50eb91fe5188c92e5f1905752e97f52d48fb3a57 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Sun, 16 Jun 2013 20:39:54 +0100 Subject: [PATCH 2/2] issue #165 - reloading settings - ghost.js - split the settings loading out of ghost.init, so that we have a function for loading / reloading settings - api.js - implemented a new requestHandler, the cachedSettingsRequestHandler which handles all aspects of local caching for settings when making requests - app.js - updated the settings api routes to use the new cached request handler --- app.js | 6 ++-- core/ghost.js | 17 ++++++++-- core/shared/api.js | 61 +++++++++++++++++++++++++++-------- core/test/ghost/ghost_spec.js | 8 ++--- 4 files changed, 68 insertions(+), 24 deletions(-) diff --git a/app.js b/app.js index 9dc64f6f05..0234f99fc1 100755 --- a/app.js +++ b/app.js @@ -108,9 +108,9 @@ ghost.app().get('/api/v0.1/posts/:id', authAPI, api.requestHandler(api.posts.read)); ghost.app().put('/api/v0.1/posts/:id', authAPI, api.requestHandler(api.posts.edit)); ghost.app().del('/api/v0.1/posts/:id', authAPI, api.requestHandler(api.posts.destroy)); - ghost.app().get('/api/v0.1/settings', authAPI, api.requestHandler(api.settings.browse)); - ghost.app().get('/api/v0.1/settings/:key', authAPI, api.requestHandler(api.settings.read)); - ghost.app().put('/api/v0.1/settings', authAPI, api.requestHandler(api.settings.edit)); + ghost.app().get('/api/v0.1/settings', authAPI, api.cachedSettingsRequestHandler(api.settings.browse)); + ghost.app().get('/api/v0.1/settings/:key', authAPI, api.cachedSettingsRequestHandler(api.settings.read)); + ghost.app().put('/api/v0.1/settings', authAPI, api.cachedSettingsRequestHandler(api.settings.edit)); /** * Admin routes.. diff --git a/core/ghost.js b/core/ghost.js index 79633432f3..341bd92c0e 100644 --- a/core/ghost.js +++ b/core/ghost.js @@ -87,8 +87,6 @@ // load Plugins... // var f = new FancyFirstChar(ghost).init(); - - _.extend(instance, { app: function () { return app; }, config: function () { return config; }, @@ -125,6 +123,19 @@ var self = this; return when.join(instance.dataProvider.init(), instance.getPaths()).then(function () { + return self.updateSettingsCache(); + }, errors.logAndThrowError); + }; + + + Ghost.prototype.updateSettingsCache = function (settings) { + var self = this; + + settings = settings || {}; + + if (!_.isEmpty(settings)) { + self.settingsCache = settings; + } else { // TODO: this should use api.browse return models.Settings.findAll().then(function (result) { var settings = {}; @@ -136,7 +147,7 @@ self.settingsCache = settings; }, errors.logAndThrowError); - }, errors.logAndThrowError); + } }; /** diff --git a/core/shared/api.js b/core/shared/api.js index 0e0a3f407c..69f7cc94df 100644 --- a/core/shared/api.js +++ b/core/shared/api.js @@ -11,6 +11,8 @@ var Ghost = require('../ghost'), _ = require('underscore'), + when = require('when'), + errors = require('./errorHandling'), ghost = new Ghost(), dataProvider = ghost.dataProvider, @@ -18,6 +20,7 @@ users, settings, requestHandler, + cachedSettingsRequestHandler, settingsObject, settingsCollection; @@ -25,37 +28,37 @@ posts = { // takes filter / pagination parameters // returns a page of posts in a json response - browse: function (options) { + browse: function browse(options) { return dataProvider.Post.findPage(options); }, // takes an identifier (id or slug?) // returns a single post in a json response - read: function (args) { + read: function read(args) { return dataProvider.Post.findOne(args); }, // takes a json object with all the properties which should be updated // returns the resulting post in a json response - edit: function (postData) { + edit: function edit(postData) { return dataProvider.Post.edit(postData); }, // takes a json object representing a post, // returns the resulting post in a json response - add: function (postData) { + add: function add(postData) { return dataProvider.Post.add(postData); }, // takes an identifier (id or slug?) // returns a json response with the id of the deleted post - destroy: function (args) { + destroy: function destroy(args) { return dataProvider.Post.destroy(args.id); } }; // # Users users = { - add: function (postData) { + add: function add(postData) { return dataProvider.User.add(postData); }, - check: function (postData) { + check: function check(postData) { return dataProvider.User.check(postData); } }; @@ -78,21 +81,17 @@ }; settings = { - browse: function (options) { + browse: function browse(options) { return dataProvider.Settings.browse(options).then(settingsObject); }, - read: function (options) { + read: function read(options) { return dataProvider.Settings.read(options.key).then(function (setting) { return _.pick(setting.toJSON(), 'key', 'value'); }); }, - edit: function (settings) { + edit: function edit(settings) { settings = settingsCollection(settings); return dataProvider.Settings.edit(settings).then(settingsObject); - }, - add: function (settings) { - settings = settingsCollection(settings); - return dataProvider.Settings.add(settings).then(settingsObject); } }; @@ -114,8 +113,42 @@ }; }; + cachedSettingsRequestHandler = function (apiMethod) { + if (!ghost.settings()) { + return requestHandler(apiMethod); + } + + return function (req, res) { + var options = _.extend(req.body, req.query, req.params), + promise; + + switch (apiMethod.name) { + case 'browse': + promise = when(ghost.settings()); + break; + case 'read': + promise = when(ghost.settings()[options.key]); + break; + case 'edit': + promise = apiMethod(options).then(function (result) { + ghost.updateSettingsCache(result); + return result; + }); + break; + default: + errors.logAndThrowError(new Error('Unknown method name for settings API: ' + apiMethod.name)); + } + return promise.then(function (result) { + res.json(result || {}); + }, function (error) { + res.json(400, {error: error}); + }); + }; + }; + module.exports.posts = posts; module.exports.users = users; module.exports.settings = settings; module.exports.requestHandler = requestHandler; + module.exports.cachedSettingsRequestHandler = cachedSettingsRequestHandler; }()); \ No newline at end of file diff --git a/core/test/ghost/ghost_spec.js b/core/test/ghost/ghost_spec.js index 84c27b1a85..f02282e9c6 100644 --- a/core/test/ghost/ghost_spec.js +++ b/core/test/ghost/ghost_spec.js @@ -27,19 +27,19 @@ } }, dataProviderInitSpy = sinon.spy(fakeDataProvider, "init"), - oldDataProvder = ghost.dataProvider; + oldDataProvider = ghost.dataProvider; ghost.dataProvider = fakeDataProvider; - should.not.exist(ghost.globals()); + should.not.exist(ghost.settings()); ghost.init().then(function () { - should.exist(ghost.globals()); + should.exist(ghost.settings()); dataProviderInitSpy.called.should.equal(true); - ghost.dataProvider = oldDataProvder; + ghost.dataProvider = oldDataProvider; done(); }).then(null, done);