Skip to content

Commit

Permalink
cleanup: convert cliConfig usage to get/set usage
Browse files Browse the repository at this point in the history
refs #783
- replaces scattered usage of instance.cliConfig.x methods with standard getters/setters
  • Loading branch information
acburdine committed Sep 4, 2018
1 parent 5618880 commit 77188ef
Show file tree
Hide file tree
Showing 18 changed files with 290 additions and 445 deletions.
2 changes: 1 addition & 1 deletion extensions/systemd/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class SystemdExtension extends Extension {

const serviceFilename = `ghost_${instance.name}.service`;

if (instance.cliConfig.get('extension.systemd', false) || fs.existsSync(path.join('/lib/systemd/system', serviceFilename))) {
if (fs.existsSync(path.join('/lib/systemd/system', serviceFilename))) {
this.ui.log('Systemd service has already been set up. Skipping Systemd setup');
return task.skip();
}
Expand Down
35 changes: 3 additions & 32 deletions extensions/systemd/test/extension-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,29 +72,6 @@ describe('Unit: Systemd > Extension', function () {
expect(skipStub.calledOnce).to.be.true;
});

it('skips stage if "extension.systemd" property in cliConfig is true', function () {
const uidStub = sinon.stub().returns(true);

const SystemdExtension = proxyquire(modulePath, {
'./get-uid': uidStub
});

const logStub = sinon.stub();
const skipStub = sinon.stub();
const getStub = sinon.stub().returns(true);
const testInstance = new SystemdExtension({log: logStub}, {}, {}, path.join(__dirname, '..'));
const instance = {dir: '/some/dir', cliConfig: {get: getStub}};

testInstance._setup({}, {instance: instance}, {skip: skipStub});
expect(uidStub.calledOnce).to.be.true;
expect(uidStub.calledWithExactly('/some/dir')).to.be.true;
expect(getStub.calledOnce).to.be.true;
expect(getStub.calledWithExactly('extension.systemd', false)).to.be.true;
expect(logStub.calledOnce).to.be.true;
expect(logStub.args[0][0]).to.match(/Systemd service has already been set up/);
expect(skipStub.calledOnce).to.be.true;
});

it('skips stage if systemd file already exists', function () {
const uidStub = sinon.stub().returns(true);
const existsStub = sinon.stub().returns(true);
Expand All @@ -106,14 +83,12 @@ describe('Unit: Systemd > Extension', function () {

const logStub = sinon.stub();
const skipStub = sinon.stub();
const getStub = sinon.stub().returns(false);
const testInstance = new SystemdExtension({log: logStub}, {}, {}, path.join(__dirname, '..'));
const instance = {dir: '/some/dir', cliConfig: {get: getStub}, name: 'test'};
const instance = {dir: '/some/dir', name: 'test'};

testInstance._setup({}, {instance: instance}, {skip: skipStub});
expect(uidStub.calledOnce).to.be.true;
expect(uidStub.calledWithExactly('/some/dir')).to.be.true;
expect(getStub.calledOnce).to.be.true;
expect(existsStub.calledOnce).to.be.true;
expect(existsStub.calledWithExactly('/lib/systemd/system/ghost_test.service')).to.be.true;
expect(logStub.calledOnce).to.be.true;
Expand All @@ -134,15 +109,13 @@ describe('Unit: Systemd > Extension', function () {
const logStub = sinon.stub();
const sudoStub = sinon.stub().resolves();
const skipStub = sinon.stub();
const getStub = sinon.stub().returns(false);
const testInstance = new SystemdExtension({log: logStub, sudo: sudoStub}, {}, {}, path.join(__dirname, '..'));
const instance = {dir: '/some/dir', cliConfig: {get: getStub}, name: 'test'};
const instance = {dir: '/some/dir', name: 'test'};
const templateStub = sinon.stub(testInstance, 'template').resolves();

return testInstance._setup({}, {instance: instance}, {skip: skipStub}).then(() => {
expect(uidStub.calledOnce).to.be.true;
expect(uidStub.calledWithExactly('/some/dir')).to.be.true;
expect(getStub.calledOnce).to.be.true;
expect(existsStub.calledOnce).to.be.true;
expect(readFileSyncStub.calledOnce).to.be.true;
expect(templateStub.calledOnce).to.be.true;
Expand All @@ -167,10 +140,9 @@ describe('Unit: Systemd > Extension', function () {
const logStub = sinon.stub();
const sudoStub = sinon.stub().rejects({stderr: 'something went wrong'});
const skipStub = sinon.stub();
const getStub = sinon.stub().returns(false);
const testInstance = new SystemdExtension({log: logStub, sudo: sudoStub}, {}, {}, path.join(__dirname, '..'));
const templateStub = sinon.stub(testInstance, 'template').resolves();
const instance = {dir: '/some/dir', cliConfig: {get: getStub}, name: 'test'};
const instance = {dir: '/some/dir', name: 'test'};

return testInstance._setup({}, {instance: instance}, {skip: skipStub}).then(() => {
expect(false, 'Promise should have rejected').to.be.true;
Expand All @@ -180,7 +152,6 @@ describe('Unit: Systemd > Extension', function () {
expect(error.options.stderr).to.be.equal('something went wrong');
expect(uidStub.calledOnce).to.be.true;
expect(uidStub.calledWithExactly('/some/dir')).to.be.true;
expect(getStub.calledOnce).to.be.true;
expect(existsStub.calledOnce).to.be.true;
expect(readFileSyncStub.calledOnce).to.be.true;
expect(templateStub.calledOnce).to.be.true;
Expand Down
10 changes: 4 additions & 6 deletions lib/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,10 @@ class InstallCommand extends Command {
link(ctx) {
symlinkSync(ctx.installPath, path.join(process.cwd(), 'current'));

// Make sure we save the current cli version to the config
// also - this ensures the config exists so the config command
// doesn't throw errors
this.system.getInstance().cliConfig
.set('cli-version', this.system.cliVersion)
.set('active-version', ctx.version).save();
const instance = this.system.getInstance();

instance.version = ctx.version;
instance.cliVersion = this.system.cliVersion;
}
}

Expand Down
4 changes: 2 additions & 2 deletions lib/commands/migrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class MigrateCommand extends Command {
'Checking for available migrations'
).then((extensionMigrations) => {
const neededMigrations = parseNeededMigrations(
instance.cliConfig.get('cli-version'),
instance.cliVersion,
this.system.cliVersion,
extensionMigrations
);
Expand All @@ -28,7 +28,7 @@ class MigrateCommand extends Command {
return this.ui.listr(neededMigrations, {instance: instance});
}).then(() => {
// Update the cli version in the cli config file
instance.cliConfig.set('cli-version', this.system.cliVersion).save();
instance.cliVersion = this.system.cliVersion;
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/commands/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class SetupCommand extends Command {
title: 'Running database migrations',
task: migrator.migrate,
// CASE: We are about to install Ghost 2.0. We moved the execution of knex-migrator into Ghost.
enabled: () => semver.major(instance.cliConfig.get('active-version')) < 2
enabled: () => semver.major(instance.version) < 2
});
}

Expand Down
24 changes: 12 additions & 12 deletions lib/commands/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ class UpdateCommand extends Command {
const instance = this.system.getInstance();

// If installed with a version < 1.0
if (semver.lt(instance.cliConfig.get('cli-version'), '1.0.0')) {
if (semver.lt(instance.cliVersion, '1.0.0')) {
this.ui.log(
`Ghost was installed with Ghost-CLI v${instance.cliConfig.get('cli-version')}, which is a pre-release version.\n` +
`Ghost was installed with Ghost-CLI v${instance.cliVersion}, which is a pre-release version.\n` +
'Your Ghost install is using out-of-date configuration & requires manual changes.\n' +
`Please visit ${chalk.blue.underline('https://docs.ghost.org/v1/docs/how-to-upgrade-ghost#section-upgrading-ghost-cli')}\n` +
'for instructions on how to upgrade your instance.\n',
Expand All @@ -39,19 +39,19 @@ class UpdateCommand extends Command {
const context = {
instance,
force,
activeVersion: instance.cliConfig.get('active-version'),
activeVersion: instance.version,
version,
zip,
v1
};

if (argv.rollback) {
if (!instance.cliConfig.get('previous-version')) {
if (!instance.previousVersion) {
return Promise.reject(new Error('No previous version found'));
}

context.rollback = true;
context.version = instance.cliConfig.get('previous-version');
context.version = instance.previousVersion;
context.installPath = path.join(process.cwd(), 'versions', context.version);
}

Expand All @@ -72,7 +72,7 @@ class UpdateCommand extends Command {
task: majorUpdate,
// CASE: Skip if you are already on ^2 or you update from v1 to v1.
enabled: () => {
if (semver.major(instance.cliConfig.get('active-version')) === 2 ||
if (semver.major(instance.version) === 2 ||
semver.major(context.version) === 1) {
return false;
}
Expand All @@ -98,7 +98,7 @@ class UpdateCommand extends Command {
// If you are already on v2 or you update from v1 to v2, then skip the task.
// We compare the major versions, otherwise pre-releases won't match.
enabled: () => {
if (semver.major(instance.cliConfig.get('active-version')) === 2 ||
if (semver.major(instance.version) === 2 ||
semver.major(context.version) === 2) {
return false;
}
Expand Down Expand Up @@ -219,7 +219,7 @@ class UpdateCommand extends Command {
}

rollbackFromFail(error, newVer, force = false) {
const oldVer = this.system.getInstance().cliConfig.get('previous-version');
const oldVer = this.system.getInstance().previousVersion;
const question = `Unable to upgrade Ghost from v${oldVer} to v${newVer}. Would you like to revert back to v${oldVer}?`;

this.ui.error(error, this.system);
Expand All @@ -244,14 +244,14 @@ class UpdateCommand extends Command {
});
}

link(context) {
link({instance, installPath, version, rollback}) {
const symlinkSync = require('symlink-or-copy').sync;

fs.removeSync(path.join(process.cwd(), 'current'));
symlinkSync(context.installPath, path.join(process.cwd(), 'current'));
symlinkSync(installPath, path.join(process.cwd(), 'current'));

context.instance.cliConfig.set('previous-version', context.rollback ? null : context.instance.cliConfig.get('active-version'))
.set('active-version', context.version).save();
instance.previousVersion = rollback ? null : instance.version;
instance.version = version;
}
}

Expand Down
4 changes: 2 additions & 2 deletions lib/commands/version.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ class VersionCommand extends Command {

const instance = this.system.getInstance();
// This will be false if we're not in a Ghost instance folder
if (instance.cliConfig.has('active-version')) {
if (instance.version) {
const dir = chalk.gray(`(at ${instance.dir.replace(os.homedir(), '~')})`);
this.ui.log(`Ghost version: ${chalk.cyan(instance.cliConfig.get('active-version'))} ${dir}`);
this.ui.log(`Ghost version: ${chalk.cyan(instance.version)} ${dir}`);
}
}
}
Expand Down
81 changes: 56 additions & 25 deletions lib/instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,6 @@ const Config = require('./utils/config');
* @class System
*/
class Instance {
/**
* Local instance config (.ghost-cli)
* Contains some instance-specific variables
*
* @property
* @type Config
* @public
*/
get cliConfig() {
if (!this._cliConfig) {
this._cliConfig = new Config(path.join(this.dir, '.ghost-cli'));
}

return this._cliConfig;
}

/**
* Process name (makes the instance identifiable on the system)
*
Expand All @@ -34,10 +18,10 @@ class Instance {
* @public
*/
get name() {
return this.cliConfig.get('name') || this.config.get('pname');
return this._cliConfig.get('name') || this.config.get('pname');
}
set name(value) {
this.cliConfig.set('name', value).save();
this._cliConfig.set('name', value).save();
return true;
}

Expand Down Expand Up @@ -78,6 +62,51 @@ class Instance {
return this._process;
}

/**
* Instance version
*
* @property version
* @type string
* @public
*/
get version() {
return this._cliConfig.get('active-version', null);
}
set version(value) {
this._cliConfig.set('active-version', value).save();
return true;
}

/**
* Version of Ghost-CLI the instance was created/migrated with
*
* @property cliVersion
* @type string
* @public
*/
get cliVersion() {
return this._cliConfig.get('cli-version', null);
}
set cliVersion(value) {
this._cliConfig.set('cli-version', value).save();
return true;
}

/**
* Previously running version of the instance
*
* @property previousVersion
* @type string
* @public
*/
get previousVersion() {
return this._cliConfig.get('previous-version', null);
}
set previousVersion(value) {
this._cliConfig.set('previous-version', value).save();
return true;
}

/**
* Constructs the instance
*
Expand All @@ -89,6 +118,8 @@ class Instance {
this.ui = ui;
this.system = system;
this.dir = dir;

this._cliConfig = new Config(path.join(this.dir, '.ghost-cli'));
}

/**
Expand All @@ -104,11 +135,11 @@ class Instance {
running(environment) {
if (environment !== undefined) {
// setter
this.cliConfig.set('running', environment).save();
this._cliConfig.set('running', environment).save();
return Promise.resolve(true);
}

if (!this.cliConfig.has('running')) {
if (!this._cliConfig.has('running')) {
const currentEnvironment = this.system.development;

const envIsRunning = (environment) => {
Expand All @@ -119,7 +150,7 @@ class Instance {
this.system.setEnvironment(environment === 'development');
return Promise.resolve(this.process.isRunning(this.dir)).then((running) => {
if (running) {
this.cliConfig.set('running', environment).save();
this._cliConfig.set('running', environment).save();
}

return running;
Expand Down Expand Up @@ -148,7 +179,7 @@ class Instance {

return Promise.resolve(this.process.isRunning(this.dir)).then((running) => {
if (!running) {
this.cliConfig.set('running', null).save();
this._cliConfig.set('running', null).save();
}

return running;
Expand Down Expand Up @@ -187,7 +218,7 @@ class Instance {
* @public
*/
loadRunningEnvironment(setNodeEnv) {
const env = this.cliConfig.get('running');
const env = this._cliConfig.get('running');

if (!env) {
throw new Error('This instance is not running.');
Expand All @@ -210,7 +241,7 @@ class Instance {
return {
name: this.name,
dir: this.dir.replace(os.homedir(), '~'),
version: this.cliConfig.get('active-version'),
version: this.version,
running: false
};
}
Expand All @@ -219,7 +250,7 @@ class Instance {
name: this.name,
dir: this.dir.replace(os.homedir(), '~'),
running: true,
version: this.cliConfig.get('active-version'),
version: this.version,
mode: this.system.environment,
url: this.config.get('url'),
port: this.config.get('server.port'),
Expand Down
2 changes: 1 addition & 1 deletion lib/process-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class ProcessManager {
stopOnError: true,
port: this.instance.config.get('server.port'),
host: this.instance.config.get('server.host', 'localhost'),
useNetServer: semver.major(this.instance.cliConfig.get('active-version')) === 2
useNetServer: semver.major(this.instance.version) === 2
}, options || {});

return portPolling(options).catch((err) => {
Expand Down

0 comments on commit 77188ef

Please sign in to comment.