Moved backup path calculation outside redirects module

refs https://github.com/TryGhost/Toolbox/issues/139

- Having tight coupling with backup file path calculation for redirects makes it extremely hard to test. In addition, having it injected will make it easier to swap this dependency to the mechanism similar to one used for routes files

core/server/services/redirects/api.js

@@ -1,6 +1,5 @@
 const fs = require('fs-extra');
 const path = require('path');
-const moment = require('moment-timezone');
 const yaml = require('js-yaml');
 
 const logging = require('@tryghost/logging');
@@ -118,16 +117,6 @@ const parseRedirectsFile = (content, ext) => {
     throw new errors.IncorrectUsageError();
 };
 
-/**
- * @param {string} filePath
- * @returns {string}
- */
-const getBackupRedirectsFilePath = (filePath) => {
-    const {dir, name, ext} = path.parse(filePath);
-
-    return path.join(dir, `${name}-${moment().format('YYYY-MM-DD-HH-mm-ss')}${ext}`);
-};
-
 /**
  * @typedef {object} IRedirectManager
  */
@@ -137,17 +126,21 @@ class CustomRedirectsAPI {
      * @param {object} config
      * @param {string} config.basePath
      * @param {Function} config.validate - validates redirects configuration
+     * @param {Function} config.getBackupFilePath
      * @param {IRedirectManager} config.redirectManager
      */
-    constructor({basePath, validate, redirectManager}) {
+    constructor({basePath, validate, redirectManager, getBackupFilePath}) {
         /** @private */
         this.basePath = basePath;
 
         /** @private */
         this.redirectManager = redirectManager;
 
-        /**private */
+        /** @private */
         this.validate = validate;
+
+        /** @private */
+        this.getBackupFilePath = getBackupFilePath;
     }
 
     async init() {

core/server/services/redirects/index.js

@@ -4,6 +4,7 @@ const urlUtils = require('../../../shared/url-utils');
 const DynamicRedirectManager = require('@tryghost/express-dynamic-redirects');
 const CustomRedirectsAPI = require('./api');
 const validation = require('./validation');
+const {getBackupRedirectsFilePath} = require('./utils');
 
 let customRedirectsAPI;
 let redirectManager;
@@ -20,6 +21,7 @@ module.exports = {
         customRedirectsAPI = new CustomRedirectsAPI({
             basePath: config.getContentPath('data'),
             redirectManager,
+            getBackupFilePath: getBackupRedirectsFilePath,
             validate: validation.validate.bind(validation)
         });
 

core/server/services/redirects/utils.js

@@ -0,0 +1,14 @@
+const path = require('path');
+const moment = require('moment-timezone');
+
+/**
+ * @param {string} filePath
+ * @returns {string}
+ */
+const getBackupRedirectsFilePath = (filePath) => {
+    const {dir, name, ext} = path.parse(filePath);
+
+    return path.join(dir, `${name}-${moment().format('YYYY-MM-DD-HH-mm-ss')}${ext}`);
+};
+
+module.exports.getBackupRedirectsFilePath = getBackupRedirectsFilePath;

test/unit/server/services/redirects/api.test.js

@@ -9,10 +9,11 @@ const CustomRedirectsAPI = require('../../../../../core/server/services/redirect
 
 describe('UNIT: redirects CustomRedirectsAPI class', function () {
     let customRedirectsAPI;
+    let redirectManager;
     const basePath = path.join(__dirname, '../../../../utils/fixtures/data/');
 
     before(function () {
-        const redirectManager = new DynamicRedirectManager({
+        redirectManager = new DynamicRedirectManager({
             permanentMaxAge: 100,
             getSubdirectoryURL: (pathname) => {
                 return `/ghost/${pathname}`;
@@ -20,8 +21,11 @@ describe('UNIT: redirects CustomRedirectsAPI class', function () {
         });
 
         customRedirectsAPI = new CustomRedirectsAPI({
-            basePath
-        }, redirectManager);
+            basePath,
+            redirectManager,
+            getBackupFilePath: () => path.join(basePath, 'backup.json'),
+            validate: () => {}
+        });
     });
 
     beforeEach(function () {
@@ -36,7 +40,7 @@ describe('UNIT: redirects CustomRedirectsAPI class', function () {
 
     describe('init', function () {
         it('initializes without errors when redirects file is not present', async function () {
-            const redirectManager = new DynamicRedirectManager({
+            redirectManager = new DynamicRedirectManager({
                 permanentMaxAge: 100,
                 getSubdirectoryURL: (pathname) => {
                     return `/ghost/${pathname}`;
@@ -44,8 +48,11 @@ describe('UNIT: redirects CustomRedirectsAPI class', function () {
             });
 
             customRedirectsAPI = new CustomRedirectsAPI({
-                basePath
-            }, redirectManager);
+                basePath,
+                redirectManager,
+                getBackupFilePath: {},
+                validate: () => {}
+            });
 
             await customRedirectsAPI.init();
             logging.error.called.should.be.false();