From 22017b8edefb170f1b30337200fbf758bd28069c Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Mon, 25 Sep 2017 19:35:57 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20=20Backup=20redirects.json=20fil?= =?UTF-8?q?e=20before=20overriding=20(#9051)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #9028 - if you upload a redirects file and a redirects file exists already, we backup this file to `data/redirects-YYYY-MM-DD-HH-mm-ss.json` - decrease chance of random test failures by not comparing date format with seconds --- core/server/api/redirects.js | 31 +++++--- .../functional/routes/api/redirects_spec.js | 72 ++++++++++++++++++- core/test/utils/index.js | 9 ++- 3 files changed, 98 insertions(+), 14 deletions(-) diff --git a/core/server/api/redirects.js b/core/server/api/redirects.js index 0df7ec10af..7239e708f4 100644 --- a/core/server/api/redirects.js +++ b/core/server/api/redirects.js @@ -1,7 +1,8 @@ 'use strict'; -const fs = require('fs'), +const fs = require('fs-extra'), Promise = require('bluebird'), + moment = require('moment'), path = require('path'), config = require('../config'), errors = require('../errors'), @@ -51,20 +52,30 @@ redirectsAPI = { }); }, upload: function upload(options) { - let redirectsPath = path.join(config.getContentPath('data'), 'redirects.json'); + let 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 apiUtils.handlePermissions('redirects', 'upload')(options) - .then(function () { - return Promise.promisify(fs.unlink)(redirectsPath) - .catch(function handleError(err) { - // CASE: ignore file not found - if (err.code === 'ENOENT') { - return Promise.resolve(); + .then(function backupOldRedirectsFile() { + return Promise.promisify(fs.pathExists)(redirectsPath) + .then(function (exists) { + if (!exists) { + return null; } - throw err; + return Promise.promisify(fs.pathExists)(backupRedirectsPath) + .then(function (exists) { + if (!exists) { + return null; + } + + return Promise.promisify(fs.unlink)(backupRedirectsPath); + }) + .then(function () { + return Promise.promisify(fs.move)(redirectsPath, backupRedirectsPath); + }); }) - .finally(function overrideFile() { + .then(function overrideFile() { return _private.readRedirectsFile(options.path) .then(function (content) { globalUtils.validateRedirects(content); diff --git a/core/test/functional/routes/api/redirects_spec.js b/core/test/functional/routes/api/redirects_spec.js index 8bb82ffbac..b7bf33b248 100644 --- a/core/test/functional/routes/api/redirects_spec.js +++ b/core/test/functional/routes/api/redirects_spec.js @@ -87,8 +87,8 @@ describe('Redirects API', function () { describe('Upload', function () { describe('Ensure re-registering redirects works', function () { - var startGhost = function () { - return ghost().then(function (_ghostServer) { + var startGhost = function (options) { + return ghost(options).then(function (_ghostServer) { ghostServer = _ghostServer; return ghostServer.start(); }).then(function () { @@ -108,6 +108,50 @@ describe('Redirects API', function () { afterEach(stopGhost); + it('no redirects file exists', function (done) { + return startGhost({redirectsFile: false}) + .then(function () { + return new Promise(function (resolve) { + setTimeout(resolve, 100); + }); + }) + .then(function () { + return request + .get('/my-old-blog-post/') + .expect(404); + }) + .then(function () { + // Provide a redirects file in the root directory of the content test folder + fs.writeFileSync(path.join(config.get('paths:contentPath'), 'redirects-init.json'), JSON.stringify([{from: 'k', to: 'l'}])); + + return new Promise(function (resolve) { + setTimeout(resolve, 100); + }); + }) + .then(function () { + return request + .post(testUtils.API.getApiQuery('redirects/json/?client_id=ghost-admin&client_secret=not_available')) + .set('Authorization', 'Bearer ' + accesstoken) + .set('Origin', testUtils.API.getURL()) + .attach('redirects', path.join(config.get('paths:contentPath'), 'redirects-init.json')) + .expect('Content-Type', /application\/json/) + .expect(200); + }) + .then(function () { + return request + .get('/k/') + .expect(302); + }) + .then(function (response) { + response.headers.location.should.eql('/l'); + + var dataFiles = fs.readdirSync(config.getContentPath('data')); + dataFiles.join(',').match(/(redirects)/g).length.should.eql(1); + done(); + }) + .catch(done); + }); + it('override', function (done) { return startGhost() .then(function () { @@ -162,6 +206,30 @@ describe('Redirects API', function () { }) .then(function (response) { response.headers.location.should.eql('/d'); + + var dataFiles = fs.readdirSync(config.getContentPath('data')); + dataFiles.join(',').match(/(redirects)/g).length.should.eql(2); + + // Provide another redirects file in the root directory of the content test folder + fs.writeFileSync(path.join(config.get('paths:contentPath'), 'redirects-something.json'), JSON.stringify([{from: 'e', to: 'b'}])); + + return new Promise(function (resolve) { + setTimeout(resolve, 1000 * 2); + }); + }) + .then(function () { + // Override redirects file again and ensure the backup file works twice + return request + .post(testUtils.API.getApiQuery('redirects/json/?client_id=ghost-admin&client_secret=not_available')) + .set('Authorization', 'Bearer ' + accesstoken) + .set('Origin', testUtils.API.getURL()) + .attach('redirects', path.join(config.get('paths:contentPath'), 'redirects-something.json')) + .expect('Content-Type', /application\/json/) + .expect(200); + }) + .then(function () { + var dataFiles = fs.readdirSync(config.getContentPath('data')); + dataFiles.join(',').match(/(redirects)/g).length.should.eql(3); done(); }) .catch(done); diff --git a/core/test/utils/index.js b/core/test/utils/index.js index 4549af4ab8..82814a2132 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -816,7 +816,9 @@ unmockNotExistingModule = function unmockNotExistingModule() { * 1. sephiroth init db * 2. start ghost */ -startGhost = function startGhost() { +startGhost = function startGhost(options) { + options = options || {redirectsFile: true}; + var contentFolderForTests = path.join(os.tmpdir(), uuid.v1(), 'ghost-test'); /** @@ -835,7 +837,10 @@ startGhost = function startGhost() { // Copy all themes into the new test content folder. Default active theme is always casper. If you want to use a different theme, you have to set the active theme (e.g. stub) fs.copySync(path.join(__dirname, 'fixtures', 'themes'), path.join(contentFolderForTests, 'themes')); - fs.copySync(path.join(__dirname, 'fixtures', 'data'), path.join(contentFolderForTests, 'data')); + + if (options.redirectsFile) { + fs.copySync(path.join(__dirname, 'fixtures', 'data', 'redirects.json'), path.join(contentFolderForTests, 'data', 'redirects.json')); + } return knexMigrator.reset() .then(function initialiseDatabase() {