From 4529ab514c9d299d4cdb86bc4abb0bc3c948cbd5 Mon Sep 17 00:00:00 2001 From: Naz Gargol Date: Mon, 1 Jul 2019 16:56:23 +0200 Subject: [PATCH] Themes controllers code extraction (#10818) refs #10790 - Extracted 'setFromZip' method into themes services - Extracted 'activate' method - Extracted 'destroy' method - Extracted 'download' method - The method name here tries to follow 'setFrom...` convention we've agreed upon. So, in this case, we have get() which returns JSON response and getZip() which returns a file --- core/frontend/services/themes/activate.js | 32 +++++ core/frontend/services/themes/index.js | 46 +++---- core/frontend/services/themes/settings.js | 123 ++++++++++++++++++ core/server/api/v0.1/themes.js | 146 ++++------------------ core/server/api/v2/themes.js | 131 ++----------------- 5 files changed, 209 insertions(+), 269 deletions(-) create mode 100644 core/frontend/services/themes/activate.js create mode 100644 core/frontend/services/themes/settings.js diff --git a/core/frontend/services/themes/activate.js b/core/frontend/services/themes/activate.js new file mode 100644 index 0000000000..c6fafe1289 --- /dev/null +++ b/core/frontend/services/themes/activate.js @@ -0,0 +1,32 @@ +const common = require('../../../server/lib/common'); +const active = require('./active'); + +function activate(loadedTheme, checkedTheme, error) { + // no need to check the score, activation should be used in combination with validate.check + // Use the two theme objects to set the current active theme + try { + let previousGhostAPI; + + if (active.get()) { + previousGhostAPI = active.get().engine('ghost-api'); + } + + active.set(loadedTheme, checkedTheme, error); + const currentGhostAPI = active.get().engine('ghost-api'); + + common.events.emit('services.themes.activated'); + + if (previousGhostAPI !== undefined && (previousGhostAPI !== currentGhostAPI)) { + common.events.emit('services.themes.api.changed'); + const siteApp = require('../../../server/web/site/app'); + siteApp.reload(); + } + } catch (err) { + common.logging.error(new common.errors.InternalServerError({ + message: common.i18n.t('errors.middleware.themehandler.activateFailed', {theme: loadedTheme.name}), + err: err + })); + } +} + +module.exports = activate; diff --git a/core/frontend/services/themes/index.js b/core/frontend/services/themes/index.js index bd394bd8ae..47fc40d657 100644 --- a/core/frontend/services/themes/index.js +++ b/core/frontend/services/themes/index.js @@ -3,6 +3,7 @@ const debug = require('ghost-ignition').debug('themes'); const common = require('../../../server/lib/common'); const themeLoader = require('./loader'); const active = require('./active'); +const activate = require('./activate'); const validate = require('./validate'); const Storage = require('./Storage'); const settingsCache = require('../../../server/services/settings/cache'); @@ -16,8 +17,7 @@ module.exports = { // Init themes module // TODO: move this once we're clear what needs to happen here init: function initThemes() { - var activeThemeName = settingsCache.get('active_theme'), - self = this; + var activeThemeName = settingsCache.get('active_theme'); debug('init themes', activeThemeName); @@ -46,7 +46,7 @@ module.exports = { common.logging.error(checkError); - self.activate(theme, checkedTheme, checkError); + activate(theme, checkedTheme, checkError); } else { // CASE: inform that the theme has errors, but not fatal (theme still works) if (checkedTheme.results.error.length) { @@ -63,7 +63,7 @@ module.exports = { debug('Activating theme (method A on boot)', activeThemeName); - self.activate(theme, checkedTheme); + activate(theme, checkedTheme); } }); }) @@ -97,32 +97,24 @@ module.exports = { return engineDefaults['ghost-api']; } }, - activate: function activate(loadedTheme, checkedTheme, error) { - // no need to check the score, activation should be used in combination with validate.check - // Use the two theme objects to set the current active theme - try { - let previousGhostAPI; + activate: function (themeName) { + const loadedTheme = this.list.get(themeName); - if (this.getActive()) { - previousGhostAPI = this.getApiVersion(); - } - - active.set(loadedTheme, checkedTheme, error); - const currentGhostAPI = this.getApiVersion(); - - common.events.emit('services.themes.activated'); - - if (previousGhostAPI !== undefined && (previousGhostAPI !== currentGhostAPI)) { - common.events.emit('services.themes.api.changed'); - const siteApp = require('../../../server/web/site/app'); - siteApp.reload(); - } - } catch (err) { - common.logging.error(new common.errors.InternalServerError({ - message: common.i18n.t('errors.middleware.themehandler.activateFailed', {theme: loadedTheme.name}), - err: err + if (!loadedTheme) { + return Promise.reject(new common.errors.ValidationError({ + message: common.i18n.t('notices.data.validation.index.themeCannotBeActivated', {themeName: themeName}), + errorDetails: themeName })); } + + return validate.checkSafe(loadedTheme) + .then((checkedTheme) => { + debug('Activating theme (method B on API "activate")', themeName); + activate(loadedTheme, checkedTheme); + + return checkedTheme; + }); }, + settings: require('./settings'), middleware: require('./middleware') }; diff --git a/core/frontend/services/themes/settings.js b/core/frontend/services/themes/settings.js new file mode 100644 index 0000000000..5105ea319f --- /dev/null +++ b/core/frontend/services/themes/settings.js @@ -0,0 +1,123 @@ +const fs = require('fs-extra'); + +const activate = require('./activate'); +const validate = require('./validate'); +const list = require('./list'); +const Storage = require('./Storage'); +const themeLoader = require('./loader'); +const toJSON = require('./to-json'); + +const settingsCache = require('../../../server/services/settings/cache'); +const common = require('../../../server/lib/common'); +const debug = require('ghost-ignition').debug('api:themes'); + +let themeStorage; + +const getStorage = () => { + themeStorage = themeStorage || new Storage(); + + return themeStorage; +}; + +module.exports = { + get: require('./to-json'), + getZip: (themeName) => { + const theme = list.get(themeName); + + if (!theme) { + return Promise.reject(new common.errors.BadRequestError({ + message: common.i18n.t('errors.api.themes.invalidThemeName') + })); + } + + return getStorage().serve({ + name: themeName + }); + }, + setFromZip: (zip) => { + const shortName = getStorage().getSanitizedFileName(zip.name.split('.zip')[0]); + + // check if zip name is casper.zip + if (zip.name === 'casper.zip') { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.api.themes.overrideCasper') + }); + } + + let checkedTheme; + + return validate.checkSafe(zip, true) + .then((_checkedTheme) => { + checkedTheme = _checkedTheme; + + return getStorage().exists(shortName); + }) + .then((themeExists) => { + // CASE: delete existing theme + if (themeExists) { + return getStorage().delete(shortName); + } + }) + .then(() => { + // CASE: store extracted theme + return getStorage().save({ + name: shortName, + path: checkedTheme.path + }); + }) + .then(() => { + // CASE: loads the theme from the fs & sets the theme on the themeList + return themeLoader.loadOneTheme(shortName); + }) + .then((loadedTheme) => { + // CASE: if this is the active theme, we are overriding + if (shortName === settingsCache.get('active_theme')) { + debug('Activating theme (method C, on API "override")', shortName); + activate(loadedTheme, checkedTheme); + + // CASE: clear cache + this.headers.cacheInvalidate = true; + } + + // @TODO: unify the name across gscan and Ghost! + return toJSON(shortName, checkedTheme); + }) + .finally(() => { + // @TODO: we should probably do this as part of saving the theme + // CASE: remove extracted dir from gscan + // happens in background + if (checkedTheme) { + fs.remove(checkedTheme.path) + .catch((err) => { + common.logging.error(new common.errors.GhostError({err: err})); + }); + } + }); + }, + destroy: function (themeName) { + if (themeName === 'casper') { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.api.themes.destroyCasper') + }); + } + + if (themeName === settingsCache.get('active_theme')) { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.api.themes.destroyActive') + }); + } + + const theme = list.get(themeName); + + if (!theme) { + throw new common.errors.NotFoundError({ + message: common.i18n.t('errors.api.themes.themeDoesNotExist') + }); + } + + return getStorage().delete(themeName) + .then(() => { + list.del(themeName); + }); + } +}; diff --git a/core/server/api/v0.1/themes.js b/core/server/api/v0.1/themes.js index 6b0c3ea9d5..248618131c 100644 --- a/core/server/api/v0.1/themes.js +++ b/core/server/api/v0.1/themes.js @@ -1,14 +1,9 @@ // # Themes API // RESTful API for Themes -const debug = require('ghost-ignition').debug('api:themes'), - Promise = require('bluebird'), - fs = require('fs-extra'), - localUtils = require('./utils'), +const localUtils = require('./utils'), common = require('../../lib/common'), models = require('../../models'), - settingsCache = require('../../services/settings/cache'), - themeService = require('../../../frontend/services/themes'), - themeList = themeService.list; + themeService = require('../../../frontend/services/themes'); let themes; @@ -31,7 +26,7 @@ themes = { // Main action .then(() => { // Return JSON result - return themeService.toJSON(); + return themeService.settings.get(); }); }, @@ -40,40 +35,24 @@ themes = { newSettings = [{ key: 'active_theme', value: themeName - }], - loadedTheme, - checkedTheme; + }]; return localUtils // Permissions .handlePermissions('themes', 'activate')(options) - // Validation + // Validation & activation .then(() => { - loadedTheme = themeList.get(themeName); - - if (!loadedTheme) { - return Promise.reject(new common.errors.ValidationError({ - message: common.i18n.t('notices.data.validation.index.themeCannotBeActivated', {themeName: themeName}), - errorDetails: newSettings - })); - } - - return themeService.validate.checkSafe(loadedTheme); + return themeService.activate(themeName); }) // Update setting - .then((_checkedTheme) => { - checkedTheme = _checkedTheme; - // We use the model, not the API here, as we don't want to trigger permissions - return models.Settings.edit(newSettings, options); + .then((checkedTheme) => { + // @NOTE: We use the model, not the API here, as we don't want to trigger permissions + return models.Settings.edit(newSettings, options) + .then(() => checkedTheme); }) - // Call activate - .then(() => { - // Activate! (sort of) - debug('Activating theme (method B on API "activate")', themeName); - themeService.activate(loadedTheme, checkedTheme); - + .then((checkedTheme) => { // Return JSON result - return themeService.toJSON(themeName, checkedTheme); + return themeService.settings.get(themeName, checkedTheme); }); }, @@ -84,91 +63,31 @@ themes = { options.originalname = options.originalname.toLowerCase(); let zip = { - path: options.path, - name: options.originalname, - shortName: themeService.storage.getSanitizedFileName(options.originalname.split('.zip')[0]) - }, - checkedTheme; - - // check if zip name is casper.zip - if (zip.name === 'casper.zip') { - throw new common.errors.ValidationError({message: common.i18n.t('errors.api.themes.overrideCasper')}); - } + path: options.path, + name: options.originalname + }; return localUtils // Permissions .handlePermissions('themes', 'add')(options) // Validation .then(() => { - return themeService.validate.checkSafe(zip, true); + return themeService.settings.setFromZip(zip); }) - // More validation (existence check) - .then((_checkedTheme) => { - checkedTheme = _checkedTheme; - - return themeService.storage.exists(zip.shortName); - }) - // If the theme existed we need to delete it - .then((themeExists) => { - // delete existing theme - if (themeExists) { - return themeService.storage.delete(zip.shortName); - } - }) - .then(() => { - // store extracted theme - return themeService.storage.save({ - name: zip.shortName, - path: checkedTheme.path - }); - }) - .then(() => { - // Loads the theme from the filesystem - // Sets the theme on the themeList - return themeService.loadOne(zip.shortName); - }) - .then((loadedTheme) => { - // If this is the active theme, we are overriding - // This is a special case of activation - if (zip.shortName === settingsCache.get('active_theme')) { - // Activate! (sort of) - debug('Activating theme (method C, on API "override")', zip.shortName); - themeService.activate(loadedTheme, checkedTheme); - } - + .then((theme) => { common.events.emit('theme.uploaded'); - - // @TODO: unify the name across gscan and Ghost! - return themeService.toJSON(zip.shortName, checkedTheme); - }) - .finally(() => { - // @TODO we should probably do this as part of saving the theme - // remove extracted dir from gscan - // happens in background - if (checkedTheme) { - fs.remove(checkedTheme.path) - .catch((err) => { - common.logging.error(new common.errors.GhostError({err: err})); - }); - } + return theme; }); }, download(options) { - let themeName = options.name, - theme = themeList.get(themeName); - - if (!theme) { - return Promise.reject(new common.errors.BadRequestError({message: common.i18n.t('errors.api.themes.invalidThemeName')})); - } + let themeName = options.name; return localUtils // Permissions .handlePermissions('themes', 'read')(options) .then(() => { - return themeService.storage.serve({ - name: themeName - }); + return themeService.settings.getZip(themeName); }); }, @@ -177,35 +96,14 @@ themes = { * remove theme folder */ destroy(options) { - let themeName = options.name, - theme; + let themeName = options.name; return localUtils // Permissions .handlePermissions('themes', 'destroy')(options) // Validation .then(() => { - if (themeName === 'casper') { - throw new common.errors.ValidationError({message: common.i18n.t('errors.api.themes.destroyCasper')}); - } - - if (themeName === settingsCache.get('active_theme')) { - throw new common.errors.ValidationError({message: common.i18n.t('errors.api.themes.destroyActive')}); - } - - theme = themeList.get(themeName); - - if (!theme) { - throw new common.errors.NotFoundError({message: common.i18n.t('errors.api.themes.themeDoesNotExist')}); - } - - // Actually do the deletion here - return themeService.storage.delete(themeName); - }) - // And some extra stuff to maintain state here - .then(() => { - themeList.del(themeName); - // Delete returns an empty 204 response + return themeService.settings.destroy(themeName); }); } }; diff --git a/core/server/api/v2/themes.js b/core/server/api/v2/themes.js index 2abe7dbb8d..4821da73e0 100644 --- a/core/server/api/v2/themes.js +++ b/core/server/api/v2/themes.js @@ -1,9 +1,5 @@ -const Promise = require('bluebird'); -const fs = require('fs-extra'); -const debug = require('ghost-ignition').debug('api:themes'); const common = require('../../lib/common'); const themeService = require('../../../frontend/services/themes'); -const settingsCache = require('../../services/settings/cache'); const models = require('../../models'); module.exports = { @@ -12,7 +8,7 @@ module.exports = { browse: { permissions: true, query() { - return themeService.toJSON(); + return themeService.settings.get(); } }, @@ -33,34 +29,19 @@ module.exports = { permissions: true, query(frame) { let themeName = frame.options.name; - let checkedTheme; - const newSettings = [{ key: 'active_theme', value: themeName }]; - const loadedTheme = themeService.list.get(themeName); - - if (!loadedTheme) { - return Promise.reject(new common.errors.ValidationError({ - message: common.i18n.t('notices.data.validation.index.themeCannotBeActivated', {themeName: themeName}), - errorDetails: newSettings - })); - } - - return themeService.validate.checkSafe(loadedTheme) - .then((_checkedTheme) => { - checkedTheme = _checkedTheme; - + return themeService.activate(themeName) + .then((checkedTheme) => { // @NOTE: we use the model, not the API here, as we don't want to trigger permissions - return models.Settings.edit(newSettings, frame.options); + return models.Settings.edit(newSettings, frame.options) + .then(() => checkedTheme); }) - .then(() => { - debug('Activating theme (method B on API "activate")', themeName); - themeService.activate(loadedTheme, checkedTheme); - - return themeService.toJSON(themeName, checkedTheme); + .then((checkedTheme) => { + return themeService.settings.get(themeName, checkedTheme); }); } }, @@ -76,67 +57,13 @@ module.exports = { let zip = { path: frame.file.path, - name: frame.file.originalname, - shortName: themeService.storage.getSanitizedFileName(frame.file.originalname.split('.zip')[0]) + name: frame.file.originalname }; - let checkedTheme; - - // check if zip name is casper.zip - if (zip.name === 'casper.zip') { - throw new common.errors.ValidationError({ - message: common.i18n.t('errors.api.themes.overrideCasper') - }); - } - - return themeService.validate.checkSafe(zip, true) - .then((_checkedTheme) => { - checkedTheme = _checkedTheme; - - return themeService.storage.exists(zip.shortName); - }) - .then((themeExists) => { - // CASE: delete existing theme - if (themeExists) { - return themeService.storage.delete(zip.shortName); - } - }) - .then(() => { - // CASE: store extracted theme - return themeService.storage.save({ - name: zip.shortName, - path: checkedTheme.path - }); - }) - .then(() => { - // CASE: loads the theme from the fs & sets the theme on the themeList - return themeService.loadOne(zip.shortName); - }) - .then((loadedTheme) => { - // CASE: if this is the active theme, we are overriding - if (zip.shortName === settingsCache.get('active_theme')) { - debug('Activating theme (method C, on API "override")', zip.shortName); - themeService.activate(loadedTheme, checkedTheme); - - // CASE: clear cache - this.headers.cacheInvalidate = true; - } - + return themeService.settings.setFromZip(zip) + .then((theme) => { common.events.emit('theme.uploaded'); - - // @TODO: unify the name across gscan and Ghost! - return themeService.toJSON(zip.shortName, checkedTheme); - }) - .finally(() => { - // @TODO: we should probably do this as part of saving the theme - // CASE: remove extracted dir from gscan - // happens in background - if (checkedTheme) { - fs.remove(checkedTheme.path) - .catch((err) => { - common.logging.error(new common.errors.GhostError({err: err})); - }); - } + return theme; }); } }, @@ -157,17 +84,8 @@ module.exports = { }, query(frame) { let themeName = frame.options.name; - const theme = themeService.list.get(themeName); - if (!theme) { - return Promise.reject(new common.errors.BadRequestError({ - message: common.i18n.t('errors.api.themes.invalidThemeName') - })); - } - - return themeService.storage.serve({ - name: themeName - }); + return themeService.settings.getZip(themeName); } }, @@ -190,30 +108,7 @@ module.exports = { query(frame) { let themeName = frame.options.name; - if (themeName === 'casper') { - throw new common.errors.ValidationError({ - message: common.i18n.t('errors.api.themes.destroyCasper') - }); - } - - if (themeName === settingsCache.get('active_theme')) { - throw new common.errors.ValidationError({ - message: common.i18n.t('errors.api.themes.destroyActive') - }); - } - - const theme = themeService.list.get(themeName); - - if (!theme) { - throw new common.errors.NotFoundError({ - message: common.i18n.t('errors.api.themes.themeDoesNotExist') - }); - } - - return themeService.storage.delete(themeName) - .then(() => { - themeService.list.del(themeName); - }); + return themeService.settings.destroy(themeName); } } };