diff --git a/core/server/api/themes.js b/core/server/api/themes.js index 5a5e87d567..979b7ddce6 100644 --- a/core/server/api/themes.js +++ b/core/server/api/themes.js @@ -3,13 +3,10 @@ var debug = require('debug')('ghost:api:themes'), Promise = require('bluebird'), fs = require('fs-extra'), - config = require('../config'), errors = require('../errors'), events = require('../events'), logging = require('../logging'), - storage = require('../storage'), apiUtils = require('./utils'), - utils = require('./../utils'), i18n = require('../i18n'), settingsModel = require('../models/settings').Settings, settingsCache = require('../settings/cache'), @@ -82,11 +79,10 @@ themes = { // consistent filename uploads options.originalname = options.originalname.toLowerCase(); - var storageAdapter = storage.getStorage('themes'), - zip = { + var zip = { path: options.path, name: options.originalname, - shortName: storageAdapter.getSanitizedFileName(options.originalname.split('.zip')[0]) + shortName: themeUtils.storage.getSanitizedFileName(options.originalname.split('.zip')[0]) }, checkedTheme; @@ -106,22 +102,22 @@ themes = { .then(function checkExists(_checkedTheme) { checkedTheme = _checkedTheme; - return storageAdapter.exists(utils.url.urlJoin(config.getContentPath('themes'), zip.shortName)); + return themeUtils.storage.exists(zip.shortName); }) // If the theme existed we need to delete it .then(function removeOldTheme(themeExists) { // delete existing theme if (themeExists) { - return storageAdapter.delete(zip.shortName, config.getContentPath('themes')); + return themeUtils.storage.delete(zip.shortName); } }) .then(function storeNewTheme() { events.emit('theme.uploaded', zip.shortName); // store extracted theme - return storageAdapter.save({ + return themeUtils.storage.save({ name: zip.shortName, path: checkedTheme.path - }, config.getContentPath('themes')); + }); }) .then(function loadNewTheme() { // Loads the theme from the filesystem @@ -163,8 +159,7 @@ themes = { download: function download(options) { var themeName = options.name, - theme = themeList.get(themeName), - storageAdapter = storage.getStorage('themes'); + theme = themeList.get(themeName); if (!theme) { return Promise.reject(new errors.BadRequestError({message: i18n.t('errors.api.themes.invalidRequest')})); @@ -175,7 +170,9 @@ themes = { .handlePermissions('themes', 'read')(options) .then(function sendTheme() { events.emit('theme.downloaded', themeName); - return storageAdapter.serve({isTheme: true, name: themeName}); + return themeUtils.storage.serve({ + name: themeName + }); }); }, @@ -185,8 +182,7 @@ themes = { */ destroy: function destroy(options) { var themeName = options.name, - theme, - storageAdapter = storage.getStorage('themes'); + theme; return apiUtils // Permissions @@ -208,7 +204,7 @@ themes = { } // Actually do the deletion here - return storageAdapter.delete(themeName, config.getContentPath('themes')); + return themeUtils.storage.delete(themeName); }) // And some extra stuff to maintain state here .then(function deleteTheme() { diff --git a/core/server/config/defaults.json b/core/server/config/defaults.json index ddc918e101..984d8e0232 100644 --- a/core/server/config/defaults.json +++ b/core/server/config/defaults.json @@ -9,9 +9,7 @@ "contentPath": "content/" }, "storage": { - "active": { - "images": "local-file-store" - } + "active": "LocalFileStorage" }, "scheduling": { "active": "SchedulingDefault" diff --git a/core/server/config/overrides.json b/core/server/config/overrides.json index 657f2946ad..0d6535346a 100644 --- a/core/server/config/overrides.json +++ b/core/server/config/overrides.json @@ -55,11 +55,6 @@ "contentTypes": ["application/zip", "application/x-zip-compressed", "application/octet-stream"] } }, - "storage": { - "active": { - "themes": "local-file-store" - } - }, "times": { "cannotScheduleAPostBeforeInMinutes": 2, "publishAPostBySchedulerToleranceInMinutes": 2 diff --git a/core/server/data/importer/handlers/image.js b/core/server/data/importer/handlers/image.js index b9bbed6b35..5b8c8ee9df 100644 --- a/core/server/data/importer/handlers/image.js +++ b/core/server/data/importer/handlers/image.js @@ -36,7 +36,7 @@ ImageHandler = { }); return Promise.map(files, function (image) { - return store.getUniqueFileName(store, image, image.targetDir).then(function (targetFilename) { + return store.getUniqueFileName(image, image.targetDir).then(function (targetFilename) { image.newPath = utils.url.urlJoin('/', utils.url.getSubdir(), utils.url.STATIC_IMAGE_URL_PREFIX, path.relative(config.getContentPath('images'), targetFilename)); diff --git a/core/server/middleware/serve-favicon.js b/core/server/middleware/serve-favicon.js index 75d9235b46..9e48d524ba 100644 --- a/core/server/middleware/serve-favicon.js +++ b/core/server/middleware/serve-favicon.js @@ -48,17 +48,18 @@ function serveFavicon() { return res.redirect(302, '/favicon' + originalExtension); } - storage.getStorage().read({path: filePath}).then(function readFile(buf, err) { - if (err) { - return next(err); - } + storage.getStorage() + .read({path: filePath}) + .then(function readFile(buf) { + iconType = settingsCache.get('icon').match(/\/favicon\.ico$/i) ? 'x-icon' : 'png'; + content = buildContentResponse(iconType, buf); - iconType = settingsCache.get('icon').match(/\/favicon\.ico$/i) ? 'x-icon' : 'png'; - content = buildContentResponse(iconType, buf); - - res.writeHead(200, content.headers); - res.end(content.body); - }); + res.writeHead(200, content.headers); + res.end(content.body); + }) + .catch(function (err) { + next(err); + }); } else { filePath = path.join(config.get('paths:corePath'), 'shared', 'favicon.ico'); originalExtension = path.extname(filePath).toLowerCase(); diff --git a/core/server/storage/LocalFileStorage.js b/core/server/storage/LocalFileStorage.js new file mode 100644 index 0000000000..7340a621e7 --- /dev/null +++ b/core/server/storage/LocalFileStorage.js @@ -0,0 +1,135 @@ +// jscs:disable requireMultipleVarDecl + +'use strict'; + +// # Local File System Image Storage module +// The (default) module for storing images, using the local file system + +var serveStatic = require('express').static, + fs = require('fs-extra'), + path = require('path'), + Promise = require('bluebird'), + config = require('../config'), + errors = require('../errors'), + i18n = require('../i18n'), + utils = require('../utils'), + StorageBase = require('ghost-storage-base'); + +class LocalFileStore extends StorageBase { + + constructor() { + super(); + + this.storagePath = config.getContentPath('images'); + } + + /** + * Saves the image to storage (the file system) + * - image is the express image object + * - returns a promise which ultimately returns the full url to the uploaded image + * + * @param image + * @param targetDir + * @returns {*} + */ + save(image, targetDir) { + var targetFilename, + self = this; + + // NOTE: the base implementation of `getTargetDir` returns the format this.storagePath/YYYY/MM + targetDir = targetDir || this.getTargetDir(this.storagePath); + + return this.getUniqueFileName(image, targetDir).then(function (filename) { + targetFilename = filename; + return Promise.promisify(fs.mkdirs)(targetDir); + }).then(function () { + return Promise.promisify(fs.copy)(image.path, targetFilename); + }).then(function () { + // The src for the image must be in URI format, not a file system path, which in Windows uses \ + // For local file system storage can use relative path so add a slash + var fullUrl = ( + utils.url.urlJoin('/', utils.url.getSubdir(), + utils.url.STATIC_IMAGE_URL_PREFIX, + path.relative(self.storagePath, targetFilename)) + ).replace(new RegExp('\\' + path.sep, 'g'), '/'); + + return fullUrl; + }).catch(function (e) { + return Promise.reject(e); + }); + } + + exists(fileName, targetDir) { + var filePath = path.join(targetDir || this.storagePath, fileName); + + return new Promise(function (resolve) { + fs.stat(filePath, function (err) { + var exists = !err; + resolve(exists); + }); + }); + } + + /** + * For some reason send divides the max age number by 1000 + * Fallthrough: false ensures that if an image isn't found, it automatically 404s + * Wrap server static errors + * + * @returns {serveStaticContent} + */ + serve() { + var self = this; + + return function serveStaticContent(req, res, next) { + return serveStatic(self.storagePath, {maxAge: utils.ONE_YEAR_MS, fallthrough: false})(req, res, function (err) { + if (err) { + if (err.statusCode === 404) { + return next(new errors.NotFoundError({message: i18n.t('errors.errors.pageNotFound')})); + } + + return next(new errors.GhostError({err: err})); + } + + next(); + }); + }; + } + + /** + * Not implemented. + * @returns {Promise.<*>} + */ + delete() { + return Promise.reject('not implemented'); + } + + /** + * Reads bytes from disk for a target image + * - path of target image (without content path!) + * + * @param options + */ + read(options) { + options = options || {}; + + // remove trailing slashes + options.path = (options.path || '').replace(/\/$|\\$/, ''); + + var targetPath = path.join(this.storagePath, options.path); + + return new Promise(function (resolve, reject) { + fs.readFile(targetPath, function (err, bytes) { + if (err) { + return reject(new errors.GhostError({ + err: err, + message: 'Could not read image: ' + targetPath + })); + } + + resolve(bytes); + }); + }); + } +} + +module.exports = LocalFileStore; diff --git a/core/server/storage/base.js b/core/server/storage/base.js deleted file mode 100644 index 2a6b7b252a..0000000000 --- a/core/server/storage/base.js +++ /dev/null @@ -1,68 +0,0 @@ -var moment = require('moment'), - path = require('path'); - -function StorageBase() { - Object.defineProperty(this, 'requiredFns', { - value: ['exists', 'save', 'serve', 'delete', 'read'], - writable: false - }); -} - -StorageBase.prototype.getTargetDir = function (baseDir) { - var m = moment(), - month = m.format('MM'), - year = m.format('YYYY'); - - if (baseDir) { - return path.join(baseDir, year, month); - } - - return path.join(year, month); -}; - -StorageBase.prototype.generateUnique = function (store, dir, name, ext, i) { - var self = this, - filename, - append = ''; - - if (i) { - append = '-' + i; - } - - if (ext) { - filename = path.join(dir, name + append + ext); - } else { - filename = path.join(dir, name + append); - } - - return store.exists(filename).then(function (exists) { - if (exists) { - i = i + 1; - return self.generateUnique(store, dir, name, ext, i); - } else { - return filename; - } - }); -}; - -StorageBase.prototype.getUniqueFileName = function (store, image, targetDir) { - var ext = path.extname(image.name), name; - - // poor extension validation - // .1 is not a valid extension - if (!ext.match(/.\d/)) { - name = this.getSanitizedFileName(path.basename(image.name, ext)); - return this.generateUnique(store, targetDir, name, ext, 0); - } else { - name = this.getSanitizedFileName(path.basename(image.name)); - return this.generateUnique(store, targetDir, name, null, 0); - } -}; - -StorageBase.prototype.getSanitizedFileName = function getSanitizedFileName(fileName) { - // below only matches ascii characters, @, and . - // unicode filenames like город.zip would therefore resolve to ----.zip - return fileName.replace(/[^\w@.]/gi, '-'); -}; - -module.exports = StorageBase; diff --git a/core/server/storage/index.js b/core/server/storage/index.js index 0cf1000638..96a9cd682d 100644 --- a/core/server/storage/index.js +++ b/core/server/storage/index.js @@ -1,55 +1,62 @@ var errors = require('../errors'), config = require('../config'), - Base = require('./base'), + StorageBase = require('ghost-storage-base'), _ = require('lodash'), storage = {}; /** - * type: images|themes + * type: images */ -function getStorage(type) { - type = type || 'images'; - - var storageChoice = config.get('storage').active[type], - storageConfig; - - // CASE: we only allow local-file-storage for themes - // @TODO: https://github.com/TryGhost/Ghost/issues/7246 - if (type === 'themes') { - storageChoice = 'local-file-store'; - } +function getStorage() { + var storageChoice = config.get('storage:active'), + storageConfig, + CustomStorage, + customStorage; storageConfig = config.get('storage')[storageChoice]; // CASE: type does not exist if (!storageChoice) { throw new errors.IncorrectUsageError({ - message: 'No adapter found for type: ' + type + message: 'No adapter found' }); } - // cache? + // CASE: cached if (storage[storageChoice]) { return storage[storageChoice]; } // CASE: load adapter from custom path (.../content/storage) try { - storage[storageChoice] = require(config.getContentPath('storage') + storageChoice); + CustomStorage = require(config.getContentPath('storage') + storageChoice); } catch (err) { - // CASE: only throw error if module does exist - if (err.code !== 'MODULE_NOT_FOUND') { - throw new errors.IncorrectUsageError({err: err}); + if (err.message.match(/strict mode/gi)) { + throw new errors.IncorrectUsageError({ + message: 'Your custom storage adapter must use strict mode.', + help: 'Add \'use strict\'; on top of your adapter.', + err: err + }); } // CASE: if module not found it can be an error within the adapter (cannot find bluebird for example) else if (err.code === 'MODULE_NOT_FOUND' && err.message.indexOf(config.getContentPath('storage') + storageChoice) === -1) { - throw new errors.IncorrectUsageError({err: err}); + throw new errors.IncorrectUsageError({ + message: 'We have detected an error in your custom storage adapter.', + err: err + }); + } + // CASE: only throw error if module does exist + else if (err.code !== 'MODULE_NOT_FOUND') { + throw new errors.IncorrectUsageError({ + message: 'We have detected an unknown error in your custom storage adapter.', + err: err + }); } } - // CASE: either storage[storageChoice] is already set or why check for in the default storage path + // CASE: check in the default storage path try { - storage[storageChoice] = storage[storageChoice] || require(config.get('paths').internalStoragePath + storageChoice); + CustomStorage = CustomStorage || require(config.get('paths').internalStoragePath + storageChoice); } catch (err) { if (err.code === 'MODULE_NOT_FOUND') { throw new errors.IncorrectUsageError({ @@ -57,24 +64,35 @@ function getStorage(type) { context: 'We cannot find your adapter in: ' + config.getContentPath('storage') + ' or: ' + config.get('paths').internalStoragePath }); } else { - throw new errors.IncorrectUsageError({err: err}); + throw new errors.IncorrectUsageError({ + message: 'We have detected an error in your custom storage adapter.', + err: err + }); } } - storage[storageChoice] = new storage[storageChoice](storageConfig); + customStorage = new CustomStorage(storageConfig); - if (!(storage[storageChoice] instanceof Base)) { - throw new errors.IncorrectUsageError({message: 'Your storage adapter does not inherit from the Storage Base.'}); + // CASE: if multiple StorageBase modules are installed, instanceof could return false + if (Object.getPrototypeOf(CustomStorage).name !== StorageBase.name) { + throw new errors.IncorrectUsageError({ + message: 'Your storage adapter does not inherit from the Storage Base.' + }); } - if (!storage[storageChoice].requiredFns) { - throw new errors.IncorrectUsageError({message:'Your storage adapter does not provide the minimum required functions.'}); + if (!customStorage.requiredFns) { + throw new errors.IncorrectUsageError({ + message: 'Your storage adapter does not provide the minimum required functions.' + }); } - if (_.xor(storage[storageChoice].requiredFns, Object.keys(_.pick(Object.getPrototypeOf(storage[storageChoice]), storage[storageChoice].requiredFns))).length) { - throw new errors.IncorrectUsageError({message:'Your storage adapter does not provide the minimum required functions.'}); + if (_.xor(customStorage.requiredFns, Object.keys(_.pick(Object.getPrototypeOf(customStorage), customStorage.requiredFns))).length) { + throw new errors.IncorrectUsageError({ + message: 'Your storage adapter does not provide the minimum required functions.' + }); } + storage[storageChoice] = customStorage; return storage[storageChoice]; } diff --git a/core/server/storage/local-file-store.js b/core/server/storage/local-file-store.js deleted file mode 100644 index 643449cf95..0000000000 --- a/core/server/storage/local-file-store.js +++ /dev/null @@ -1,152 +0,0 @@ -// # Local File System Image Storage module -// The (default) module for storing images, using the local file system - -var serveStatic = require('express').static, - fs = require('fs-extra'), - os = require('os'), - path = require('path'), - util = require('util'), - Promise = require('bluebird'), - config = require('../config'), - errors = require('../errors'), - i18n = require('../i18n'), - utils = require('../utils'), - BaseStore = require('./base'), - remove = Promise.promisify(fs.remove); - -function LocalFileStore() { - BaseStore.call(this); -} - -util.inherits(LocalFileStore, BaseStore); - -// ### Save -// Saves the image to storage (the file system) -// - image is the express image object -// - returns a promise which ultimately returns the full url to the uploaded image -LocalFileStore.prototype.save = function save(image, targetDir) { - targetDir = targetDir || this.getTargetDir(config.getContentPath('images')); - var targetFilename; - - return this.getUniqueFileName(this, image, targetDir).then(function (filename) { - targetFilename = filename; - return Promise.promisify(fs.mkdirs)(targetDir); - }).then(function () { - return Promise.promisify(fs.copy)(image.path, targetFilename); - }).then(function () { - // The src for the image must be in URI format, not a file system path, which in Windows uses \ - // For local file system storage can use relative path so add a slash - var fullUrl = ( - utils.url.urlJoin('/', utils.url.getSubdir(), - utils.url.STATIC_IMAGE_URL_PREFIX, - path.relative(config.getContentPath('images'), targetFilename)) - ).replace(new RegExp('\\' + path.sep, 'g'), '/'); - - return fullUrl; - }).catch(function (e) { - return Promise.reject(e); - }); -}; - -LocalFileStore.prototype.exists = function exists(filename) { - return new Promise(function (resolve) { - fs.stat(filename, function (err) { - var exists = !err; - resolve(exists); - }); - }); -}; - -// middleware for serving the files -LocalFileStore.prototype.serve = function serve(options) { - options = options || {}; - - // CASE: serve themes - // serveStatic can't be used to serve themes, because - // download files depending on the route (see `send` npm module) - if (options.isTheme) { - return function downloadTheme(req, res, next) { - var themeName = options.name, - themePath = path.join(config.getContentPath('themes'), themeName), - zipName = themeName + '.zip', - // store this in a unique temporary folder - zipBasePath = path.join(os.tmpdir(), utils.uid(10)), - zipPath = path.join(zipBasePath, zipName), - stream; - - Promise.promisify(fs.ensureDir)(zipBasePath) - .then(function () { - return Promise.promisify(utils.zipFolder)(themePath, zipPath); - }) - .then(function (length) { - res.set({ - 'Content-disposition': 'attachment; filename={themeName}.zip'.replace('{themeName}', themeName), - 'Content-Type': 'application/zip', - 'Content-Length': length - }); - - stream = fs.createReadStream(zipPath); - stream.pipe(res); - }) - .catch(function (err) { - next(err); - }) - .finally(function () { - remove(zipBasePath); - }); - }; - } else { - // CASE: serve images - // For some reason send divides the max age number by 1000 - // Fallthrough: false ensures that if an image isn't found, it automatically 404s - // Wrap server static errors - return function serveStaticContent(req, res, next) { - return serveStatic(config.getContentPath('images'), {maxAge: utils.ONE_YEAR_MS, fallthrough: false})(req, res, function (err) { - if (err) { - if (err.statusCode === 404) { - return next(new errors.NotFoundError({message: i18n.t('errors.errors.pageNotFound')})); - } - - return next(new errors.GhostError({err: err})); - } - - next(); - }); - }; - } -}; - -LocalFileStore.prototype.delete = function deleteFile(fileName, targetDir) { - targetDir = targetDir || this.getTargetDir(config.getContentPath('images')); - - var pathToDelete = path.join(targetDir, fileName); - return remove(pathToDelete); -}; - -/** - * Reads bytes from disk for a target image - * path: path of target image (without content path!) - */ -LocalFileStore.prototype.read = function read(options) { - options = options || {}; - - // remove trailing slashes - options.path = (options.path || '').replace(/\/$|\\$/, ''); - - var targetPath = path.join(config.getContentPath('images'), options.path); - - return new Promise(function (resolve, reject) { - fs.readFile(targetPath, function (err, bytes) { - if (err) { - return reject(new errors.GhostError({ - err: err, - message: 'Could not read image: ' + targetPath - })); - } - - resolve(bytes); - }); - }); -}; - -module.exports = LocalFileStore; diff --git a/core/server/themes/Storage.js b/core/server/themes/Storage.js new file mode 100644 index 0000000000..596ff63e91 --- /dev/null +++ b/core/server/themes/Storage.js @@ -0,0 +1,68 @@ +// jscs:disable requireMultipleVarDecl + +'use strict'; + +var fs = require('fs-extra'), + os = require('os'), + path = require('path'), + Promise = require('bluebird'), + config = require('../config'), + utils = require('../utils'), + LocalFileStorage = require('../storage/LocalFileStorage'), + remove = Promise.promisify(fs.remove); + +/** + * @TODO: combine with loader.js? + */ +class ThemeStorage extends LocalFileStorage { + constructor() { + super(); + + this.storagePath = config.getContentPath('themes'); + } + + getTargetDir() { + return this.storagePath; + } + + serve(options) { + var self = this; + + return function downloadTheme(req, res, next) { + var themeName = options.name, + themePath = path.join(self.storagePath, themeName), + zipName = themeName + '.zip', + // store this in a unique temporary folder + zipBasePath = path.join(os.tmpdir(), utils.uid(10)), + zipPath = path.join(zipBasePath, zipName), + stream; + + Promise.promisify(fs.ensureDir)(zipBasePath) + .then(function () { + return Promise.promisify(utils.zipFolder)(themePath, zipPath); + }) + .then(function (length) { + res.set({ + 'Content-disposition': 'attachment; filename={themeName}.zip'.replace('{themeName}', themeName), + 'Content-Type': 'application/zip', + 'Content-Length': length + }); + + stream = fs.createReadStream(zipPath); + stream.pipe(res); + }) + .catch(function (err) { + next(err); + }) + .finally(function () { + remove(zipBasePath); + }); + }; + } + + delete(fileName) { + return remove(path.join(this.storagePath, fileName)); + } +} + +module.exports = ThemeStorage; diff --git a/core/server/themes/index.js b/core/server/themes/index.js index cb837f29b4..19d3851dc1 100644 --- a/core/server/themes/index.js +++ b/core/server/themes/index.js @@ -7,7 +7,9 @@ var debug = require('debug')('ghost:themes'), themeLoader = require('./loader'), active = require('./active'), validate = require('./validate'), - settingsCache = require('../settings/cache'); + Storage = require('./Storage'), + settingsCache = require('../settings/cache'), + themeStorage; // @TODO: reduce the amount of things we expose to the outside world // Make this a nice clean sensible API we can all understand! @@ -50,6 +52,11 @@ module.exports = { // Load themes, soon to be removed and exposed via specific function. loadAll: themeLoader.loadAllThemes, loadOne: themeLoader.loadOneTheme, + get storage() { + themeStorage = themeStorage || new Storage(); + + return themeStorage; + }, list: require('./list'), validate: validate, toJSON: require('./to-json'), diff --git a/core/test/unit/config/index_spec.js b/core/test/unit/config/index_spec.js index fd1246d305..3ee8306e56 100644 --- a/core/test/unit/config/index_spec.js +++ b/core/test/unit/config/index_spec.js @@ -118,10 +118,7 @@ describe('Config', function () { it('should default to local-file-store', function () { configUtils.config.get('paths').should.have.property('internalStoragePath', path.join(configUtils.config.get('paths').corePath, '/server/storage/')); - configUtils.config.get('storage').should.have.property('active', { - images: 'local-file-store', - themes: 'local-file-store' - }); + configUtils.config.get('storage').should.have.property('active', 'LocalFileStorage'); }); it('no effect: setting a custom active storage as string', function () { diff --git a/core/test/unit/importer_spec.js b/core/test/unit/importer_spec.js index c900529360..11a4b5dea6 100644 --- a/core/test/unit/importer_spec.js +++ b/core/test/unit/importer_spec.js @@ -370,9 +370,9 @@ describe('Importer', function () { ImageHandler.loadFile(_.clone(file)).then(function () { storageSpy.calledOnce.should.be.true(); storeSpy.calledOnce.should.be.true(); - storeSpy.firstCall.args[1].originalPath.should.equal('test-image.jpeg'); - storeSpy.firstCall.args[1].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); - storeSpy.firstCall.args[1].newPath.should.eql('/content/images/test-image.jpeg'); + storeSpy.firstCall.args[0].originalPath.should.equal('test-image.jpeg'); + storeSpy.firstCall.args[0].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); + storeSpy.firstCall.args[0].newPath.should.eql('/content/images/test-image.jpeg'); done(); }).catch(done); @@ -390,9 +390,9 @@ describe('Importer', function () { ImageHandler.loadFile(_.clone(file)).then(function () { storageSpy.calledOnce.should.be.true(); storeSpy.calledOnce.should.be.true(); - storeSpy.firstCall.args[1].originalPath.should.equal('photos/my-cat.jpeg'); - storeSpy.firstCall.args[1].targetDir.should.match(/(\/|\\)content(\/|\\)images(\/|\\)photos$/); - storeSpy.firstCall.args[1].newPath.should.eql('/content/images/photos/my-cat.jpeg'); + storeSpy.firstCall.args[0].originalPath.should.equal('photos/my-cat.jpeg'); + storeSpy.firstCall.args[0].targetDir.should.match(/(\/|\\)content(\/|\\)images(\/|\\)photos$/); + storeSpy.firstCall.args[0].newPath.should.eql('/content/images/photos/my-cat.jpeg'); done(); }).catch(done); @@ -410,9 +410,9 @@ describe('Importer', function () { ImageHandler.loadFile(_.clone(file)).then(function () { storageSpy.calledOnce.should.be.true(); storeSpy.calledOnce.should.be.true(); - storeSpy.firstCall.args[1].originalPath.should.equal('content/images/my-cat.jpeg'); - storeSpy.firstCall.args[1].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); - storeSpy.firstCall.args[1].newPath.should.eql('/content/images/my-cat.jpeg'); + storeSpy.firstCall.args[0].originalPath.should.equal('content/images/my-cat.jpeg'); + storeSpy.firstCall.args[0].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); + storeSpy.firstCall.args[0].newPath.should.eql('/content/images/my-cat.jpeg'); done(); }).catch(done); @@ -432,9 +432,9 @@ describe('Importer', function () { ImageHandler.loadFile(_.clone(file)).then(function () { storageSpy.calledOnce.should.be.true(); storeSpy.calledOnce.should.be.true(); - storeSpy.firstCall.args[1].originalPath.should.equal('test-image.jpeg'); - storeSpy.firstCall.args[1].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); - storeSpy.firstCall.args[1].newPath.should.eql('/subdir/content/images/test-image.jpeg'); + storeSpy.firstCall.args[0].originalPath.should.equal('test-image.jpeg'); + storeSpy.firstCall.args[0].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); + storeSpy.firstCall.args[0].newPath.should.eql('/subdir/content/images/test-image.jpeg'); done(); }).catch(done); @@ -463,18 +463,18 @@ describe('Importer', function () { ImageHandler.loadFile(_.clone(files)).then(function () { storageSpy.calledOnce.should.be.true(); storeSpy.callCount.should.eql(4); - storeSpy.firstCall.args[1].originalPath.should.equal('testing.png'); - storeSpy.firstCall.args[1].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); - storeSpy.firstCall.args[1].newPath.should.eql('/content/images/testing.png'); - storeSpy.secondCall.args[1].originalPath.should.equal('photo/kitten.jpg'); - storeSpy.secondCall.args[1].targetDir.should.match(/(\/|\\)content(\/|\\)images(\/|\\)photo$/); - storeSpy.secondCall.args[1].newPath.should.eql('/content/images/photo/kitten.jpg'); - storeSpy.thirdCall.args[1].originalPath.should.equal('content/images/animated/bunny.gif'); - storeSpy.thirdCall.args[1].targetDir.should.match(/(\/|\\)content(\/|\\)images(\/|\\)animated$/); - storeSpy.thirdCall.args[1].newPath.should.eql('/content/images/animated/bunny.gif'); - storeSpy.lastCall.args[1].originalPath.should.equal('images/puppy.jpg'); - storeSpy.lastCall.args[1].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); - storeSpy.lastCall.args[1].newPath.should.eql('/content/images/puppy.jpg'); + storeSpy.firstCall.args[0].originalPath.should.equal('testing.png'); + storeSpy.firstCall.args[0].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); + storeSpy.firstCall.args[0].newPath.should.eql('/content/images/testing.png'); + storeSpy.secondCall.args[0].originalPath.should.equal('photo/kitten.jpg'); + storeSpy.secondCall.args[0].targetDir.should.match(/(\/|\\)content(\/|\\)images(\/|\\)photo$/); + storeSpy.secondCall.args[0].newPath.should.eql('/content/images/photo/kitten.jpg'); + storeSpy.thirdCall.args[0].originalPath.should.equal('content/images/animated/bunny.gif'); + storeSpy.thirdCall.args[0].targetDir.should.match(/(\/|\\)content(\/|\\)images(\/|\\)animated$/); + storeSpy.thirdCall.args[0].newPath.should.eql('/content/images/animated/bunny.gif'); + storeSpy.lastCall.args[0].originalPath.should.equal('images/puppy.jpg'); + storeSpy.lastCall.args[0].targetDir.should.match(/(\/|\\)content(\/|\\)images$/); + storeSpy.lastCall.args[0].newPath.should.eql('/content/images/puppy.jpg'); done(); }).catch(done); diff --git a/core/test/unit/middleware/serve-favicon_spec.js b/core/test/unit/middleware/serve-favicon_spec.js index 67eef2c404..d2ec0f77ec 100644 --- a/core/test/unit/middleware/serve-favicon_spec.js +++ b/core/test/unit/middleware/serve-favicon_spec.js @@ -3,13 +3,14 @@ var should = require('should'), // jshint ignore:line express = require('express'), serveFavicon = require('../../../server/middleware/serve-favicon'), settingsCache = require('../../../server/settings/cache'), + storage = require('../../../server/storage'), configUtils = require('../../utils/configUtils'), path = require('path'), sandbox = sinon.sandbox.create(); describe('Serve Favicon', function () { - var req, res, next, blogApp, localSettingsCache = {}; + var req, res, next, blogApp, localSettingsCache = {}, originalStoragePath; beforeEach(function () { req = sinon.spy(); @@ -21,12 +22,15 @@ describe('Serve Favicon', function () { sandbox.stub(settingsCache, 'get', function (key) { return localSettingsCache[key]; }); + + originalStoragePath = storage.getStorage().storagePath; }); afterEach(function () { sandbox.restore(); configUtils.restore(); localSettingsCache = {}; + storage.getStorage().storagePath = originalStoragePath; }); describe('serveFavicon', function () { @@ -48,7 +52,7 @@ describe('Serve Favicon', function () { var middleware = serveFavicon(); req.path = '/favicon.png'; - configUtils.set('paths:contentPath', path.join(__dirname, '../../../test/utils/fixtures/')); + storage.getStorage().storagePath = path.join(__dirname, '../../../test/utils/fixtures/images/'); localSettingsCache.icon = 'favicon.png'; res = { @@ -68,7 +72,7 @@ describe('Serve Favicon', function () { var middleware = serveFavicon(); req.path = '/favicon.ico'; - configUtils.set('paths:contentPath', path.join(__dirname, '../../../test/utils/fixtures/')); + storage.getStorage().storagePath = path.join(__dirname, '../../../test/utils/fixtures/images/'); localSettingsCache.icon = 'favicon.ico'; res = { diff --git a/core/test/unit/storage/local-file-store_spec.js b/core/test/unit/storage/LocalFileStorage_spec.js similarity index 98% rename from core/test/unit/storage/local-file-store_spec.js rename to core/test/unit/storage/LocalFileStorage_spec.js index 53f1874f55..1526197330 100644 --- a/core/test/unit/storage/local-file-store_spec.js +++ b/core/test/unit/storage/LocalFileStorage_spec.js @@ -4,7 +4,7 @@ var should = require('should'), // jshint ignore:line moment = require('moment'), path = require('path'), errors = require('../../../server/errors'), - LocalFileStore = require('../../../server/storage/local-file-store'), + LocalFileStore = require('../../../server/storage/LocalFileStorage'), localFileStore, configUtils = require('../../utils/configUtils'), @@ -150,7 +150,7 @@ describe('Local File System Storage', function () { describe('read image', function () { beforeEach(function () { // we have some example images in our test utils folder - configUtils.set('paths:contentPath', path.join(__dirname, '../../utils/fixtures')); + localFileStore.storagePath = path.join(__dirname, '../../utils/fixtures/images/'); }); it('success', function (done) { diff --git a/core/test/unit/storage/base_spec.js b/core/test/unit/storage/base_spec.js deleted file mode 100644 index a5b950bc25..0000000000 --- a/core/test/unit/storage/base_spec.js +++ /dev/null @@ -1,9 +0,0 @@ -var should = require('should'), // jshint ignore:line - storage = require('../../../server/storage'); - -describe('storage: base_spec', function () { - it('escape non accepted characters in filenames', function () { - var chosenStorage = storage.getStorage('themes'); - chosenStorage.getSanitizedFileName('(abc*@#123).zip').should.eql('-abc-@-123-.zip'); - }); -}); diff --git a/core/test/unit/storage/index_spec.js b/core/test/unit/storage/index_spec.js index 075c54f91d..eb08ae06e6 100644 --- a/core/test/unit/storage/index_spec.js +++ b/core/test/unit/storage/index_spec.js @@ -1,9 +1,10 @@ var should = require('should'), // jshint ignore:line fs = require('fs-extra'), + StorageBase = require('ghost-storage-base'), configUtils = require('../../utils/configUtils'), storage = require('../../../server/storage'), errors = require('../../../server/errors'), - localFileStorage = require('../../../server/storage/local-file-store'); + LocalFileStorage = require('../../../server/storage/LocalFileStorage'); describe('storage: index_spec', function () { var scope = {adapter: null}; @@ -23,124 +24,71 @@ describe('storage: index_spec', function () { configUtils.restore(); }); - describe('default ghost storage config', function () { - it('load without a type', function () { - var chosenStorage = storage.getStorage(); - (chosenStorage instanceof localFileStorage).should.eql(true); - }); - - it('load with themes type', function () { - var chosenStorage = storage.getStorage('themes'); - (chosenStorage instanceof localFileStorage).should.eql(true); - }); - - it('load with unknown type', function () { - try { - storage.getStorage('theme'); - } catch (err) { - (err instanceof errors.IncorrectUsageError).should.eql(true); - } - }); + it('default image storage is local file storage', function () { + var chosenStorage = storage.getStorage(); + (chosenStorage instanceof StorageBase).should.eql(true); + (chosenStorage instanceof LocalFileStorage).should.eql(true); }); - describe('custom ghost storage config', function () { - it('images storage adapter is custom, themes is default', function () { - scope.adapter = configUtils.config.getContentPath('storage') + 'custom-adapter.js'; + it('custom adapter', function () { + scope.adapter = configUtils.config.getContentPath('storage') + 'custom-adapter.js'; - configUtils.set({ - storage: { - active: { - images: 'custom-adapter' - } - } - }); - - var jsFile = '' + - 'var util = require(\'util\');' + - 'var StorageBase = require(__dirname + \'/../../core/server/storage/base\');' + - 'var AnotherAdapter = function (){ StorageBase.call(this); };' + - 'util.inherits(AnotherAdapter, StorageBase);' + - 'AnotherAdapter.prototype.exists = function (){};' + - 'AnotherAdapter.prototype.save = function (){};' + - 'AnotherAdapter.prototype.serve = function (){};' + - 'AnotherAdapter.prototype.delete = function (){};' + - 'AnotherAdapter.prototype.read = function (){};' + - 'module.exports = AnotherAdapter', chosenStorage; - - fs.writeFileSync(scope.adapter, jsFile); - - chosenStorage = storage.getStorage('themes'); - (chosenStorage instanceof localFileStorage).should.eql(true); - - chosenStorage = storage.getStorage('images'); - (chosenStorage instanceof localFileStorage).should.eql(false); - }); - }); - - describe('adapter validation', function () { - it('create good adapter', function () { - scope.adapter = configUtils.config.getContentPath('storage') + 'another-storage.js'; - - configUtils.set({ - storage: { - active: { - images: 'another-storage' - } - }, - paths: { - storage: __dirname + '/another-storage.js' - } - }); - - var jsFile = '' + - 'var util = require(\'util\');' + - 'var StorageBase = require(__dirname + \'/../../core/server/storage/base\');' + - 'var AnotherAdapter = function (){ StorageBase.call(this); };' + - 'util.inherits(AnotherAdapter, StorageBase);' + - 'AnotherAdapter.prototype.exists = function (){};' + - 'AnotherAdapter.prototype.save = function (){};' + - 'AnotherAdapter.prototype.serve = function (){};' + - 'AnotherAdapter.prototype.delete = function (){};' + - 'AnotherAdapter.prototype.read = function (){};' + - 'module.exports = AnotherAdapter', adapter; - - fs.writeFileSync(scope.adapter, jsFile); - - adapter = storage.getStorage(); - should.exist(adapter); - (adapter instanceof localFileStorage).should.eql(false); - }); - - it('create bad adapter: exists fn is missing', function () { - scope.adapter = configUtils.config.getContentPath('storage') + 'broken-storage.js'; - - configUtils.set({ - storage: { - active: 'broken-storage' - }, - paths: { - storage: __dirname + '/broken-storage.js' - } - }); - - var jsFile = '' + - 'var util = require(\'util\');' + - 'var StorageBase = require(__dirname + \'/../../core/server/storage/base\');' + - 'var AnotherAdapter = function (){ StorageBase.call(this); };' + - 'util.inherits(AnotherAdapter, StorageBase);' + - 'AnotherAdapter.prototype.save = function (){};' + - 'AnotherAdapter.prototype.serve = function (){};' + - 'AnotherAdapter.prototype.delete = function (){};' + - 'module.exports = AnotherAdapter'; - - fs.writeFileSync(scope.adapter, jsFile); - - try { - storage.getStorage(); - } catch (err) { - should.exist(err); - (err instanceof errors.IncorrectUsageError).should.eql(true); + configUtils.set({ + storage: { + active: 'custom-adapter' } }); + + var jsFile = '' + + '\'use strict\';' + + 'var StorageBase = require(\'ghost-storage-base\');' + + 'class AnotherAdapter extends StorageBase {' + + 'exists(){}' + + 'save(){}' + + 'serve(){}' + + 'delete(){}' + + 'read(){}' + + '}' + + 'module.exports = AnotherAdapter', chosenStorage; + + fs.writeFileSync(scope.adapter, jsFile); + + configUtils.config.get('storage:active').should.eql('custom-adapter'); + chosenStorage = storage.getStorage(); + (chosenStorage instanceof LocalFileStorage).should.eql(false); + (chosenStorage instanceof StorageBase).should.eql(true); + }); + + it('create bad adapter: exists fn is missing', function () { + scope.adapter = configUtils.config.getContentPath('storage') + 'broken-storage.js'; + + configUtils.set({ + storage: { + active: 'broken-storage' + }, + paths: { + storage: __dirname + '/broken-storage.js' + } + }); + + var jsFile = '' + + '\'use strict\';' + + 'var StorageBase = require(\'ghost-storage-base\');' + + 'class AnotherAdapter extends StorageBase {' + + 'save(){}' + + 'serve(){}' + + 'delete(){}' + + 'read(){}' + + '}' + + 'module.exports = AnotherAdapter'; + + fs.writeFileSync(scope.adapter, jsFile); + + try { + storage.getStorage(); + } catch (err) { + should.exist(err); + (err instanceof errors.IncorrectUsageError).should.eql(true); + } }); }); diff --git a/package.json b/package.json index 35285e52d6..d261e01ca7 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "fs-extra": "2.1.2", "ghost-gql": "0.0.6", "ghost-ignition": "2.8.10", + "ghost-storage-base": "0.0.1", "glob": "5.0.15", "gscan": "0.2.1", "html-to-text": "3.2.0", diff --git a/yarn.lock b/yarn.lock index 0bede5c84d..4e82a8b0aa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1578,6 +1578,12 @@ ghost-ignition@2.8.10, ghost-ignition@^2.8.2, ghost-ignition@^2.8.7: prettyjson "1.1.3" uuid "^3.0.0" +ghost-storage-base@0.0.1: + version "0.0.1" + resolved "https://registry.yarnpkg.com/ghost-storage-base/-/ghost-storage-base-0.0.1.tgz#b31b57d2e54574a96153a54bf2e9ea599f12bec8" + dependencies: + moment "^2.17.1" + glob-base@^0.3.0: version "0.3.0" resolved "https://registry.yarnpkg.com/glob-base/-/glob-base-0.3.0.tgz#dbb164f6221b1c0b1ccf82aea328b497df0ea3c4" @@ -3196,7 +3202,7 @@ moment-timezone@0.5.13: dependencies: moment ">= 2.9.0" -moment@2.18.1, "moment@>= 2.9.0", moment@^2.10.6, moment@^2.15.2: +moment@2.18.1, "moment@>= 2.9.0", moment@^2.10.6, moment@^2.15.2, moment@^2.17.1: version "2.18.1" resolved "https://registry.yarnpkg.com/moment/-/moment-2.18.1.tgz#c36193dd3ce1c2eed2adb7c802dbbc77a81b1c0f"