Skip to content

Commit

Permalink
fix(migrate): move setting of content-path to setup
Browse files Browse the repository at this point in the history 
closes TryGhost#708
- remove any need to save the config out of the migrations step
- instead, we set the content path during ghost setup
  • Loading branch information
@acburdine
acburdine committed May 28, 2018
1 parent 9cb99b0 commit e53ce6a
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 35 deletions.
5 changes: 5 additions & 0 deletions lib/commands/setup.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ class SetupCommand extends Command {
ctx.instance = this.system.getInstance();
ctx.instance.name = (argv.pname || url.parse(ctx.instance.config.get('url')).hostname).replace(/\./g, '-');
this.system.addInstance(ctx.instance);

// Ensure we set the content path when we set up the instance
if (!ctx.instance.config.has('paths.contentPath')) {
ctx.instance.config.set('paths.contentPath', path.join(ctx.instance.dir, 'content')).save();
}
}
}];

Expand Down
14 changes: 5 additions & 9 deletions lib/tasks/migrate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,23 @@ const errors = require('../errors');
const ghostUser = require('../utils/use-ghost-user');

module.exports = function runMigrations(context) {
const config = context.instance.config;
const {config, dir} = context.instance;

if (!config.has('paths.contentPath')) {
config.set('paths.contentPath', path.join(context.instance.dir, 'content')).save();
}

const contentDir = path.join(context.instance.dir, 'content');
const currentDir = path.join(context.instance.dir, 'current');
const contentDir = path.join(dir, 'content');
const currentDir = path.join(dir, 'current');
let knexMigratorPromise;

const args = ['--init', '--mgpath', currentDir];

// If we're using sqlite and the ghost user owns the content folder, then
// we should run sudo, otherwise run normally
if (ghostUser.shouldUseGhostUser(contentDir)) {
const knexMigratorPath = path.resolve(context.instance.dir, 'current/node_modules/.bin/knex-migrator-migrate');
const knexMigratorPath = path.resolve(dir, 'current/node_modules/.bin/knex-migrator-migrate');
knexMigratorPromise = context.ui.sudo(`${knexMigratorPath} ${args.join(' ')}`, {sudoArgs: '-E -u ghost'});
} else {
knexMigratorPromise = execa('knex-migrator-migrate', args, {
preferLocal: true,
localDir: path.join(context.instance.dir, 'current')
localDir: path.join(dir, 'current')
});
}

Expand Down
25 changes: 22 additions & 3 deletions test/unit/commands/setup-spec.js
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
'use strict';
const expect = require('chai').expect;
const {expect} = require('chai');
const sinon = require('sinon');
const proxyquire = require('proxyquire').noCallThru();
const Promise = require('bluebird');
const path = require('path');
const configStub = require('../../utils/config-stub');

const modulePath = '../../../lib/commands/setup';
const SetupCommand = proxyquire(modulePath, {os: {platform: () => 'linux'}});
Expand Down Expand Up @@ -223,12 +224,17 @@ describe('Unit: Commands > Setup', function () {
it('Initial stage is setup properly', function () {
const listr = sinon.stub().resolves();
const aIstub = sinon.stub();
const config = configStub();
config.get.withArgs('url').returns('https://ghost.org');
config.has.returns(false);

const system = {
getInstance: () => {
return {
checkEnvironment: () => true,
apples: true,
config: {get: () => 'https://ghost.org'}
config,
dir: '/var/www/ghost'
};
},
addInstance: aIstub,
Expand Down Expand Up @@ -262,18 +268,28 @@ describe('Unit: Commands > Setup', function () {
expect(ctx.instance.name).to.equal('ghost-org');
expect(aIstub.calledOnce).to.be.true;
expect(aIstub.args[0][0]).to.deep.equal(ctx.instance);

expect(config.has.calledOnce).to.be.true;
expect(config.set.calledOnce).to.be.true;
expect(config.set.calledWithExactly('paths.contentPath', '/var/www/ghost/content')).to.be.true;
expect(config.save.calledOnce).to.be.true;
});
});

it('Initial stage is setup properly, sanitizes pname argument', function () {
const listr = sinon.stub().resolves();
const aIstub = sinon.stub();
const config = configStub();
config.get.withArgs('url').returns('https://ghost.org');
config.has.returns(true);

const system = {
getInstance: () => {
return {
checkEnvironment: () => true,
apples: true,
config: {get: () => 'https://ghost.org'}
config,
dir: '/var/www/ghost'
};
},
addInstance: aIstub,
Expand Down Expand Up @@ -308,6 +324,9 @@ describe('Unit: Commands > Setup', function () {
expect(ctx.instance.name).to.equal('example-com');
expect(aIstub.calledOnce).to.be.true;
expect(aIstub.args[0][0]).to.deep.equal(ctx.instance);
expect(config.has.calledOnce).to.be.true;
expect(config.set.called).to.be.false;
expect(config.save.called).to.be.false;
});
});

Expand Down
31 changes: 8 additions & 23 deletions test/unit/tasks/migrate-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,15 @@
const {expect} = require('chai');
const sinon = require('sinon');
const proxyquire = require('proxyquire');
const configStub = require('../../utils/config-stub');

const errors = require('../../../lib/errors');

const migratePath = '../../../lib/tasks/migrate';

function getConfigStub(noContentPath) {
const config = {
get: sinon.stub(),
set: sinon.stub().returnsThis(),
has: sinon.stub(),
save: sinon.stub().returnsThis()
};

config.has.withArgs('paths.contentPath').returns(!noContentPath);
return config;
}

describe('Unit: Tasks > Migrate', function () {
it('runs direct command if useGhostUser returns false', function () {
const config = getConfigStub(true);
const config = configStub();
const execaStub = sinon.stub().resolves();
const useGhostUserStub = sinon.stub().returns(false);

Expand All @@ -37,15 +26,11 @@ describe('Unit: Tasks > Migrate', function () {
expect(useGhostUserStub.args[0][0]).to.equal('/some-dir/content');
expect(execaStub.calledOnce).to.be.true;
expect(sudoStub.called).to.be.false;
expect(config.has.calledOnce).to.be.true;
expect(config.set.calledOnce).to.be.true;
expect(config.set.args[0]).to.deep.equal(['paths.contentPath', '/some-dir/content']);
expect(config.save.called).to.be.true;
});
});

it('runs sudo command if useGhostUser returns true', function () {
const config = getConfigStub();
const config = configStub();
const execaStub = sinon.stub().resolves();
const useGhostUserStub = sinon.stub().returns(true);

Expand All @@ -65,7 +50,7 @@ describe('Unit: Tasks > Migrate', function () {
});

it('throws config error with db host if database not found', function () {
const config = getConfigStub();
const config = configStub();
const execaStub = sinon.stub().returns(Promise.reject({stderr: 'CODE: ENOTFOUND'}));
const useGhostUserStub = sinon.stub().returns(false);

Expand All @@ -83,7 +68,7 @@ describe('Unit: Tasks > Migrate', function () {
});

it('throws config error with db user if access denied error', function () {
const config = getConfigStub();
const config = configStub();
const execaStub = sinon.stub().returns(Promise.reject({stderr: 'CODE: ER_ACCESS_DENIED_ERROR'}));
const useGhostUserStub = sinon.stub().returns(false);

Expand All @@ -101,7 +86,7 @@ describe('Unit: Tasks > Migrate', function () {
});

it('throws system error if sqlite3 error is thrown by knex', function () {
const config = getConfigStub();
const config = configStub();
const execaStub = sinon.stub().returns(Promise.reject({stdout: 'Knex: run\n$ npm install sqlite3 --save\nError:'}));
const useGhostUserStub = sinon.stub().returns(false);

Expand All @@ -123,7 +108,7 @@ describe('Unit: Tasks > Migrate', function () {

process.argv = ['node', 'ghost', 'update'];

const config = getConfigStub();
const config = configStub();
const execaStub = sinon.stub().rejects({stderr: 'YA_GOOFED'});
const useGhostUserStub = sinon.stub().returns(false);

Expand All @@ -149,7 +134,7 @@ describe('Unit: Tasks > Migrate', function () {

process.argv = ['node', 'ghost', 'setup', 'migrate'];

const config = getConfigStub();
const config = configStub();
const execaStub = sinon.stub().rejects({stderr: 'YA_GOOFED'});
const useGhostUserStub = sinon.stub().returns(false);

Expand Down

0 comments on commit e53ce6a

Please sign in to comment.