From be27db46ebc539d24f6151a9a0117f12356ae65c Mon Sep 17 00:00:00 2001 From: Naz Gargol Date: Fri, 21 Jun 2019 16:50:16 +0200 Subject: [PATCH] Extracted frontend code from redirects API controllers (#10798) refs #10790 - The code was moved out of controllers to reduce the number of coupling points between the API controllers and "frontend" services - A nice side effect of this move is a decreased amount of code that will need to be maintained and reusability between existing controllers - Calling just a few methods from frontend services on API level makes it easier to abstract fronted away from API --- core/frontend/services/redirects/index.js | 5 ++ core/frontend/services/redirects/settings.js | 75 +++++++++++++++++ .../frontend/services/redirects/validation.js | 27 ++++++ core/server/api/v0.1/redirects.js | 82 ++----------------- core/server/api/v2/redirects.js | 75 ++--------------- core/server/data/validation/index.js | 27 +----- 6 files changed, 121 insertions(+), 170 deletions(-) create mode 100644 core/frontend/services/redirects/index.js create mode 100644 core/frontend/services/redirects/settings.js create mode 100644 core/frontend/services/redirects/validation.js diff --git a/core/frontend/services/redirects/index.js b/core/frontend/services/redirects/index.js new file mode 100644 index 0000000000..fd3eefd1f1 --- /dev/null +++ b/core/frontend/services/redirects/index.js @@ -0,0 +1,5 @@ +module.exports = { + get settings() { + return require('./settings'); + } +}; diff --git a/core/frontend/services/redirects/settings.js b/core/frontend/services/redirects/settings.js new file mode 100644 index 0000000000..7947f34246 --- /dev/null +++ b/core/frontend/services/redirects/settings.js @@ -0,0 +1,75 @@ +const fs = require('fs-extra'); +const path = require('path'); +const Promise = require('bluebird'); +const moment = require('moment-timezone'); + +const validation = require('./validation'); + +const config = require('../../../server/config'); +const common = require('../../../server/lib/common'); + +const readRedirectsFile = (redirectsPath) => { + return fs.readFile(redirectsPath, 'utf-8') + .then((content) => { + try { + content = JSON.parse(content); + } catch (err) { + throw new common.errors.BadRequestError({ + message: common.i18n.t('errors.general.jsonParse', {context: err.message}) + }); + } + + return content; + }) + .catch((err) => { + if (err.code === 'ENOENT') { + return Promise.resolve([]); + } + + if (common.errors.utils.isIgnitionError(err)) { + throw err; + } + + throw new common.errors.NotFoundError({ + err: err + }); + }); +}; + +const setFromFilePath = (filePath) => { + const redirectsPath = path.join(config.getContentPath('data'), 'redirects.json'); + const backupRedirectsPath = path.join(config.getContentPath('data'), `redirects-${moment().format('YYYY-MM-DD-HH-mm-ss')}.json`); + + return fs.pathExists(redirectsPath) + .then((exists) => { + if (!exists) { + return null; + } + + return fs.pathExists(backupRedirectsPath) + .then((exists) => { + if (!exists) { + return null; + } + + return fs.unlink(backupRedirectsPath); + }) + .then(() => { + return fs.move(redirectsPath, backupRedirectsPath); + }); + }) + .then(() => { + return readRedirectsFile(filePath) + .then((content) => { + validation.validate(content); + return fs.writeFile(redirectsPath, JSON.stringify(content), 'utf-8'); + }); + }); +}; + +const get = () => { + return readRedirectsFile(path.join(config.getContentPath('data'), 'redirects.json')); +}; + +module.exports.get = get; +module.exports.setFromFilePath = setFromFilePath; diff --git a/core/frontend/services/redirects/validation.js b/core/frontend/services/redirects/validation.js new file mode 100644 index 0000000000..f279f62aba --- /dev/null +++ b/core/frontend/services/redirects/validation.js @@ -0,0 +1,27 @@ +const _ = require('lodash'); +const common = require('../../../server/lib/common'); + +/** + * Redirects are file based at the moment, but they will live in the database in the future. + * See V2 of https://github.com/TryGhost/Ghost/issues/7707. + */ +const validate = (redirects) => { + if (!_.isArray(redirects)) { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.utils.redirectsWrongFormat'), + help: 'https://docs.ghost.org/concepts/redirects/' + }); + } + + _.each(redirects, function (redirect) { + if (!redirect.from || !redirect.to) { + throw new common.errors.ValidationError({ + message: common.i18n.t('errors.utils.redirectsWrongFormat'), + context: redirect, + help: 'https://docs.ghost.org/concepts/redirects/' + }); + } + }); +}; + +module.exports.validate = validate; diff --git a/core/server/api/v0.1/redirects.js b/core/server/api/v0.1/redirects.js index a2ce1869b0..fb91b7208f 100644 --- a/core/server/api/v0.1/redirects.js +++ b/core/server/api/v0.1/redirects.js @@ -1,88 +1,22 @@ -const fs = require('fs-extra'), - Promise = require('bluebird'), - moment = require('moment'), - path = require('path'), - config = require('../../config'), - common = require('../../lib/common'), - validation = require('../../data/validation'), - localUtils = require('./utils'), - web = require('../../web'); +const localUtils = require('./utils'), + web = require('../../web'), + redirects = require('../../../frontend/services/redirects'); -let redirectsAPI, - _private = {}; - -_private.readRedirectsFile = (customRedirectsPath) => { - const redirectsPath = customRedirectsPath || path.join(config.getContentPath('data'), 'redirects.json'); - - return fs.readFile(redirectsPath, 'utf-8') - .then((content) => { - try { - content = JSON.parse(content); - } catch (err) { - throw new common.errors.BadRequestError({ - message: common.i18n.t('errors.general.jsonParse', {context: err.message}) - }); - } - - return content; - }) - .catch((err) => { - if (err.code === 'ENOENT') { - return Promise.resolve([]); - } - - if (common.errors.utils.isIgnitionError(err)) { - throw err; - } - - throw new common.errors.NotFoundError({ - err: err - }); - }); -}; +let redirectsAPI; redirectsAPI = { download(options) { return localUtils.handlePermissions('redirects', 'download')(options) .then(() => { - return _private.readRedirectsFile(); + return redirects.settings.get(); }); }, upload(options) { - const redirectsPath = path.join(config.getContentPath('data'), 'redirects.json'), - backupRedirectsPath = path.join(config.getContentPath('data'), `redirects-${moment().format('YYYY-MM-DD-HH-mm-ss')}.json`); - return localUtils.handlePermissions('redirects', 'upload')(options) + .then(() => redirects.settings.setFromFilePath(options.path)) .then(() => { - return fs.pathExists(redirectsPath) - .then((exists) => { - if (!exists) { - return null; - } - - return fs.pathExists(backupRedirectsPath) - .then((exists) => { - if (!exists) { - return null; - } - - return fs.unlink(backupRedirectsPath); - }) - .then(() => { - return fs.move(redirectsPath, backupRedirectsPath); - }); - }) - .then(() => { - return _private.readRedirectsFile(options.path) - .then((content) => { - validation.validateRedirects(content); - return fs.writeFile(redirectsPath, JSON.stringify(content), 'utf-8'); - }) - .then(() => { - // CASE: trigger that redirects are getting re-registered - web.shared.middlewares.customRedirects.reload(); - }); - }); + // CASE: trigger that redirects are getting re-registered + web.shared.middlewares.customRedirects.reload(); }); } }; diff --git a/core/server/api/v2/redirects.js b/core/server/api/v2/redirects.js index d14fc2a3b6..161d01343a 100644 --- a/core/server/api/v2/redirects.js +++ b/core/server/api/v2/redirects.js @@ -1,43 +1,5 @@ -const fs = require('fs-extra'); -const path = require('path'); -const Promise = require('bluebird'); -const moment = require('moment-timezone'); -const config = require('../../config'); -const common = require('../../lib/common'); -const validation = require('../../data/validation'); const web = require('../../web'); - -const _private = {}; - -_private.readRedirectsFile = (customRedirectsPath) => { - const redirectsPath = customRedirectsPath || path.join(config.getContentPath('data'), 'redirects.json'); - - return fs.readFile(redirectsPath, 'utf-8') - .then((content) => { - try { - content = JSON.parse(content); - } catch (err) { - throw new common.errors.BadRequestError({ - message: common.i18n.t('errors.general.jsonParse', {context: err.message}) - }); - } - - return content; - }) - .catch((err) => { - if (err.code === 'ENOENT') { - return Promise.resolve([]); - } - - if (common.errors.utils.isIgnitionError(err)) { - throw err; - } - - throw new common.errors.NotFoundError({ - err: err - }); - }); -}; +const redirects = require('../../../frontend/services/redirects'); module.exports = { docName: 'redirects', @@ -51,7 +13,7 @@ module.exports = { }, permissions: true, query() { - return _private.readRedirectsFile(); + return redirects.settings.get(); } }, @@ -61,37 +23,10 @@ module.exports = { cacheInvalidate: true }, query(frame) { - const redirectsPath = path.join(config.getContentPath('data'), 'redirects.json'); - const backupRedirectsPath = path.join(config.getContentPath('data'), `redirects-${moment().format('YYYY-MM-DD-HH-mm-ss')}.json`); - - return fs.pathExists(redirectsPath) - .then((exists) => { - if (!exists) { - return null; - } - - return fs.pathExists(backupRedirectsPath) - .then((exists) => { - if (!exists) { - return null; - } - - return fs.unlink(backupRedirectsPath); - }) - .then(() => { - return fs.move(redirectsPath, backupRedirectsPath); - }); - }) + return redirects.settings.setFromFilePath(frame.file.path) .then(() => { - return _private.readRedirectsFile(frame.file.path) - .then((content) => { - validation.validateRedirects(content); - return fs.writeFile(redirectsPath, JSON.stringify(content), 'utf-8'); - }) - .then(() => { - // CASE: trigger that redirects are getting re-registered - web.shared.middlewares.customRedirects.reload(); - }); + // CASE: trigger that redirects are getting re-registered + web.shared.middlewares.customRedirects.reload(); }); } } diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index d3d35ac8d0..6b4a617610 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -11,7 +11,6 @@ var schema = require('../schema').tables, validatePassword, validateSchema, validateSettings, - validateRedirects, validate; function assertString(input) { @@ -356,34 +355,10 @@ validate = function validate(value, key, validations, tableName) { return validationErrors; }; -/** - * Redirects are file based at the moment, but they will live in the database in the future. - * See V2 of https://github.com/TryGhost/Ghost/issues/7707. - */ -validateRedirects = function validateRedirects(redirects) { - if (!_.isArray(redirects)) { - throw new common.errors.ValidationError({ - message: common.i18n.t('errors.utils.redirectsWrongFormat'), - help: 'https://docs.ghost.org/concepts/redirects/' - }); - } - - _.each(redirects, function (redirect) { - if (!redirect.from || !redirect.to) { - throw new common.errors.ValidationError({ - message: common.i18n.t('errors.utils.redirectsWrongFormat'), - context: redirect, - help: 'https://docs.ghost.org/concepts/redirects/' - }); - } - }); -}; - module.exports = { validate: validate, validator: validator, validatePassword: validatePassword, validateSchema: validateSchema, - validateSettings: validateSettings, - validateRedirects: validateRedirects + validateSettings: validateSettings };