Skip to content

Commit

Permalink
feat(setup): refactor setup command
Browse files Browse the repository at this point in the history 
closes TryGhost#594, TryGhost#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
  • Loading branch information
@acburdine
acburdine committed Jan 26, 2019
1 parent 13a6160 commit 8da3d1a
Show file tree
Hide file tree
Showing 9 changed files with 991 additions and 1,263 deletions.
44 changes: 21 additions & 23 deletions extensions/mysql/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
167 changes: 82 additions & 85 deletions extensions/mysql/test/extension-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
Loading

0 comments on commit 8da3d1a

Please sign in to comment.