feat(setup): refactor setup command

closes #594, #588
- cleanup setup command code & improve testability
- rework how extensions register setup steps
- make it so an errored setup step doesn't break the entire flow

Author: acburdine

extensions/mysql/index.js

@@ -3,31 +3,29 @@
 const Promise = require('bluebird');
 const mysql = require('mysql');
 const omit = require('lodash/omit');
-const cli = require('../../lib');
 const generator = require('generate-password');
+const {Extension, errors} = require('../../lib');
 
 const localhostAliases = ['localhost', '127.0.0.1'];
-
-class MySQLExtension extends cli.Extension {
-    setup(cmd, argv) {
-        // Case 1: ghost install local OR ghost setup --local
-        // Case 2: ghost install --db sqlite3
-        // Skip in both cases
-        if (argv.local || argv.db === 'sqlite3') {
-            return;
-        }
-
-        cmd.addStage('mysql', this.setupMySQL.bind(this), [], '"ghost" mysql user');
+const {ConfigError, CliError} = errors;
+
+class MySQLExtension extends Extension {
+    setup() {
+        return [{
+            id: 'mysql',
+            name: '"ghost" mysql user',
+            task: (...args) => this.setupMySQL(...args),
+            // Case 1: ghost install local OR ghost setup --local
+            // Case 2: ghost install --db sqlite3
+            // Skip in both cases
+            enabled: ({argv}) => !(argv.local || argv.db === 'sqlite3'),
+            skip: ({instance}) => instance.config.get('database.connection.user') !== 'root'
+        }];
     }
 
-    setupMySQL(argv, ctx, task) {
+    setupMySQL(ctx) {
         const dbconfig = ctx.instance.config.get('database.connection');
 
-        if (dbconfig.user !== 'root') {
-            this.ui.log('MySQL user is not "root", skipping additional user setup', 'yellow');
-            return task.skip();
-        }
-
         return this.ui.listr([{
             title: 'Connecting to database',
             task: () => this.canConnect(ctx, dbconfig)
@@ -53,7 +51,7 @@ class MySQLExtension extends cli.Extension {
 
         return Promise.fromCallback(cb => this.connection.connect(cb)).catch((error) => {
             if (error.code === 'ECONNREFUSED') {
-                return Promise.reject(new cli.errors.ConfigError({
+                return Promise.reject(new ConfigError({
                     message: error.message,
                     config: {
                         'database.connection.host': dbconfig.host,
@@ -63,7 +61,7 @@ class MySQLExtension extends cli.Extension {
                     help: 'Ensure that MySQL is installed and reachable. You can always re-run `ghost setup` to try again.'
                 }));
             } else if (error.code === 'ER_ACCESS_DENIED_ERROR') {
-                return Promise.reject(new cli.errors.ConfigError({
+                return Promise.reject(new ConfigError({
                     message: error.message,
                     config: {
                         'database.connection.user': dbconfig.user,
@@ -74,7 +72,7 @@ class MySQLExtension extends cli.Extension {
                 }));
             }
 
-            return Promise.reject(new cli.errors.CliError({
+            return Promise.reject(new CliError({
                 message: 'Error trying to connect to the MySQL database.',
                 help: 'You can run `ghost config` to re-enter the correct credentials. Alternatively you can run `ghost setup` again.',
                 err: error
@@ -168,11 +166,11 @@ class MySQLExtension extends cli.Extension {
         this.ui.logVerbose(`MySQL: running query > ${queryString}`, 'gray');
         return Promise.fromCallback(cb => this.connection.query(queryString, cb))
             .catch((error) => {
-                if (error instanceof (cli.errors.CliError)) {
+                if (error instanceof CliError) {
                     return Promise.reject(error);
                 }
 
-                return Promise.reject(new cli.errors.CliError({
+                return Promise.reject(new CliError({
                     message: error.message,
                     context: queryString,
                     err: error

extensions/mysql/test/extension-spec.js

@@ -2,100 +2,97 @@
 const expect = require('chai').expect;
 const sinon = require('sinon');
 const proxyquire = require('proxyquire');
+const configStub = require('../../../test/utils/config-stub');
 
 const modulePath = '../index';
 const errors = require('../../../lib/errors');
 
 describe('Unit: Mysql extension', function () {
-    describe('setup hook', function () {
+    it('setup hook works', function () {
         const MysqlExtension = require(modulePath);
-
-        it('does not add stage if --local is true', function () {
-            const instance = new MysqlExtension({}, {}, {}, '/some/dir');
-            const addStageStub = sinon.stub();
-
-            instance.setup({addStage: addStageStub}, {local: true});
-            expect(addStageStub.called).to.be.false;
-        });
-
-        it('does not add stage if db is sqlite3', function () {
-            const instance = new MysqlExtension({}, {}, {}, '/some/dir');
-            const addStageStub = sinon.stub();
-
-            instance.setup({addStage: addStageStub}, {local: false, db: 'sqlite3'});
-            expect(addStageStub.called).to.be.false;
-        });
-
-        it('adds stage if not local and db is not sqlite3', function () {
-            const instance = new MysqlExtension({}, {}, {}, '/some/dir');
-            const addStageStub = sinon.stub();
-
-            instance.setup({addStage: addStageStub}, {local: false, db: 'mysql'});
-            expect(addStageStub.calledOnce).to.be.true;
-            expect(addStageStub.calledWith('mysql')).to.be.true;
-        });
+        const inst = new MysqlExtension({}, {}, {}, '/some/dir');
+        const result = inst.setup();
+
+        expect(result).to.have.length(1);
+        const [task] = result;
+
+        // Check static properties
+        expect(task.id).to.equal('mysql');
+        expect(task.name).to.equal('"ghost" mysql user');
+
+        // Check task functions
+        expect(task.task).to.be.a('function');
+        expect(task.enabled).to.be.a('function');
+        expect(task.skip).to.be.a('function');
+
+        // Check task.task
+        const stub = sinon.stub(inst, 'setupMySQL');
+        task.task('a', 'set', 'of', 'args', true);
+        expect(stub.calledOnce).to.be.true;
+        expect(stub.calledWithExactly('a', 'set', 'of', 'args', true)).to.be.true;
+
+        // Check task.enabled
+        expect(task.enabled({argv: {local: true}})).to.be.false;
+        expect(task.enabled({argv: {local: false, db: 'sqlite3'}})).to.be.false;
+        expect(task.enabled({argv: {local: false}})).to.be.true;
+
+        // Check task.skip
+        const get = sinon.stub();
+        get.onFirstCall().returns('not-root');
+        get.onSecondCall().returns('root');
+
+        expect(task.skip({instance: {config: {get}}})).to.be.true;
+        expect(get.calledOnce).to.be.true;
+        expect(task.skip({instance: {config: {get}}})).to.be.false;
+        expect(get.calledTwice).to.be.true;
     });
 
-    describe('setupMySQL', function () {
+    it('setupMySQL works', function () {
         const MysqlExtension = require(modulePath);
-
-        it('skips if db user is not root', function () {
-            const logStub = sinon.stub();
-            const instance = new MysqlExtension({log: logStub}, {}, {}, '/some/dir');
-            const getStub = sinon.stub().returns({user: 'notroot'});
-            const skipStub = sinon.stub().resolves();
-
-            return instance.setupMySQL({}, {instance: {config: {get: getStub}}}, {skip: skipStub}).then(() => {
-                expect(getStub.calledOnce).to.be.true;
-                expect(getStub.calledWithExactly('database.connection')).to.be.true;
-                expect(logStub.calledOnce).to.be.true;
-                expect(logStub.args[0][0]).to.match(/user is not "root"/);
-                expect(skipStub.calledOnce).to.be.true;
-            });
-        });
-
-        it('returns tasks that call the helpers and cleanup', function () {
-            const listrStub = sinon.stub().callsFake(function (tasks) {
-                expect(tasks).to.be.an.instanceof(Array);
-                expect(tasks).to.have.length(4);
-
-                // Run each task step
-                tasks.forEach(task => task.task());
-                return Promise.resolve();
-            });
-            const instance = new MysqlExtension({listr: listrStub}, {}, {}, '/some/dir');
-            const getStub = sinon.stub().returns({user: 'root'});
-            const saveStub = sinon.stub();
-            const setStub = sinon.stub();
-            setStub.returns({set: setStub, save: saveStub});
-            const endStub = sinon.stub();
-            const skipStub = sinon.stub().resolves();
-            const canConnectStub = sinon.stub(instance, 'canConnect');
-            const createUserStub = sinon.stub(instance, 'createUser').callsFake((ctx) => {
-                ctx.mysql = {username: 'testuser', password: 'testpass'};
-            });
-            const grantPermissionsStub = sinon.stub(instance, 'grantPermissions');
-
-            instance.connection = {end: endStub};
-
-            return instance.setupMySQL({}, {
-                instance: {config: {get: getStub, set: setStub, save: saveStub}}
-            }, {skip: skipStub}).then(() => {
-                expect(skipStub.called).to.be.false;
-                expect(listrStub.calledOnce).to.be.true;
-                expect(listrStub.args[0][1]).to.be.false;
-
-                expect(getStub.calledOnce).to.be.true;
-                expect(canConnectStub.calledOnce).to.be.true;
-                expect(createUserStub.calledOnce).to.be.true;
-                expect(grantPermissionsStub.calledOnce).to.be.true;
-                expect(setStub.calledTwice).to.be.true;
-                expect(setStub.calledWithExactly('database.connection.user', 'testuser')).to.be.true;
-                expect(setStub.calledWithExactly('database.connection.password', 'testpass')).to.be.true;
-                expect(saveStub.calledOnce).to.be.true;
-                expect(endStub.calledOnce).to.be.true;
-            });
-        });
+        const listr = sinon.stub();
+        const config = configStub();
+        const dbConfig = {host: 'localhost', user: 'root', password: 'password'};
+        config.get.returns(dbConfig);
+
+        const inst = new MysqlExtension({listr}, {}, {}, '/some/dir');
+        const context = {instance: {config}};
+        inst.setupMySQL(context);
+
+        expect(config.get.calledOnce).to.be.true;
+        expect(config.get.calledWithExactly('database.connection')).to.be.true;
+        expect(listr.calledOnce).to.be.true;
+        const [tasks, ctx] = listr.args[0];
+        expect(tasks).to.have.length(4);
+        expect(ctx).to.be.false;
+
+        const canConnect = sinon.stub(inst, 'canConnect');
+        expect(tasks[0].title).to.equal('Connecting to database');
+        tasks[0].task();
+        expect(canConnect.calledOnce).to.be.true;
+        expect(canConnect.calledWithExactly(context, dbConfig)).to.be.true;
+
+        const createUser = sinon.stub(inst, 'createUser');
+        expect(tasks[1].title).to.equal('Creating new MySQL user');
+        tasks[1].task();
+        expect(createUser.calledOnce).to.be.true;
+        expect(createUser.calledWithExactly(context, dbConfig)).to.be.true;
+
+        const grantPermissions = sinon.stub(inst, 'grantPermissions');
+        expect(tasks[2].title).to.equal('Granting new user permissions');
+        tasks[2].task();
+        expect(grantPermissions.calledOnce).to.be.true;
+        expect(grantPermissions.calledWithExactly(context, dbConfig)).to.be.true;
+
+        const end = sinon.stub();
+        inst.connection = {end};
+        context.mysql = {username: 'new', password: 'new'};
+        expect(tasks[3].title).to.equal('Saving new config');
+        tasks[3].task();
+        expect(config.set.calledTwice).to.be.true;
+        expect(config.set.calledWithExactly('database.connection.user', 'new')).to.be.true;
+        expect(config.set.calledWithExactly('database.connection.password', 'new')).to.be.true;
+        expect(config.save.calledOnce).to.be.true;
+        expect(end.calledOnce).to.be.true;
     });
 
     describe('canConnect', function () {