fix(instance): clean up config usage around application (#254)

acburdine · web-flow · commit 22c804cf049d · 2017-07-02T15:29:09.000-04:00
diff --git a/extensions/nginx/index.js b/extensions/nginx/index.js
@@ -30,7 +30,6 @@ class NginxExtension extends cli.Extension {
         }
 
         this.instance = this.system.getInstance();
-        this.instance.loadConfig();
 
         cmd.addStage('nginx', this.setupNginx.bind(this));
         cmd.addStage('ssl', this.setupSSL.bind(this));
@@ -219,7 +218,6 @@ class NginxExtension extends cli.Extension {
 
     uninstall() {
         this.instance = this.system.getInstance();
-        this.instance.loadConfig();
 
         if (!this.instance.cliConfig.get('extension.nginx', false)) {
             return;
diff --git a/extensions/systemd/index.js b/extensions/systemd/index.js
@@ -14,7 +14,6 @@ class SystemdExtension extends cli.Extension {
 
     setup(cmd, argv) {
         let instance = this.system.getInstance();
-        instance.loadConfig();
 
         if (!argv.local && instance.config.get('process') === 'systemd') {
             cmd.addStage('systemd', this._setup.bind(this));
diff --git a/lib/commands/config/index.js b/lib/commands/config/index.js
@@ -14,7 +14,6 @@ class ConfigCommand extends Command {
         super(ui, system);
 
         let instance = this.system.getInstance();
-        instance.loadConfig(true);
         this.config = instance.config;
     }
 
diff --git a/lib/commands/log.js b/lib/commands/log.js
@@ -19,7 +19,7 @@ class LogCommand extends Command {
         // relative to that
         process.chdir(instance.dir);
 
-        instance.loadRunningConfig(true);
+        instance.loadRunningEnvironment(true);
 
         // Check if logging file transport is set in config
         if (!includes(instance.config.get('logging.transports', []), 'file')) {
diff --git a/lib/commands/restart.js b/lib/commands/restart.js
@@ -11,7 +11,7 @@ class RestartCommand extends Command {
             return Promise.reject(new Error('Ghost instance is not currently running.'));
         }
 
-        instance.loadRunningConfig(true, true);
+        instance.loadRunningEnvironment(true, true);
 
         return this.runCommand(StopCommand, argv).then(() => this.runCommand(StartCommand, argv));
     }
diff --git a/lib/commands/run.js b/lib/commands/run.js
@@ -13,7 +13,6 @@ class RunCommand extends Command {
         }
 
         let instance = this.system.getInstance();
-        instance.loadConfig(true);
 
         process.env.paths__contentPath = path.join(process.cwd(), 'content');
 
diff --git a/lib/commands/setup.js b/lib/commands/setup.js
@@ -66,7 +66,6 @@ class SetupCommand extends Command {
             title: 'Setting up instance',
             task: () => {
                 let instance = this.system.getInstance();
-                instance.loadConfig();
                 instance.name = argv.pname || url.parse(instance.config.get('url')).hostname.replace(/\./g, '-');
                 this.system.addInstance(instance);
             }
diff --git a/lib/commands/start.js b/lib/commands/start.js
@@ -27,7 +27,7 @@ class StartCommand extends Command {
             return Promise.reject(new Error('Ghost is already running.'));
         }
 
-        instance.loadConfig();
+        instance.checkEnvironment();
 
         return this.ui.listr(startupChecks.concat({
             title: 'Running database migrations',
diff --git a/lib/commands/stop.js b/lib/commands/stop.js
@@ -41,7 +41,7 @@ class StopCommand extends Command {
             return Promise.reject(new errors.SystemError('No running Ghost instance found here.'));
         }
 
-        instance.loadRunningConfig();
+        instance.loadRunningEnvironment(true);
 
         let stop = () => Promise.resolve(instance.process.stop(process.cwd())).then(() => {
             instance.running = null;
diff --git a/lib/commands/uninstall.js b/lib/commands/uninstall.js
@@ -31,14 +31,13 @@ class UninstallCommand extends Command {
                 return Promise.resolve();
             }
 
-            instance.loadRunningConfig(true, true);
+            instance.loadRunningEnvironment(true, true);
 
             // If the instance is currently running we need to make
             // sure it gets stopped
             return this.runCommand(StopCommand);
         }).then(() => {
             this.system.setEnvironment(!fs.existsSync('config.production.json'));
-            instance.loadConfig(true);
 
             return this.ui.run(this.system.hook('uninstall'), 'Removing related configuration');
         }).then(() => this.ui.run(() => {
diff --git a/lib/commands/update.js b/lib/commands/update.js
@@ -35,7 +35,11 @@ class UpdateCommand extends Command {
             context.installPath = path.join(process.cwd(), 'versions', context.version);
         }
 
-        instance.running ? instance.loadRunningConfig(true, true) : instance.loadConfig();
+        if (instance.running) {
+            instance.loadRunningEnvironment(true, true);
+        }
+
+        instance.checkEnvironment();
 
         // TODO: add meaningful update checks after this task
         let tasks = [{
diff --git a/lib/instance.js b/lib/instance.js
@@ -30,11 +30,7 @@ class Instance {
             return false;
         }
 
-        // Ordinarily we'd use this.loadRunningConfig(), but stack overflow
-        // (the error not the programming bible) happens if we do that.
-        let env = this.cliConfig.get('running');
-        this.config = new Config(path.join(this.dir, `config.${env}.json`));
-        this.loadProcess();
+        this.loadRunningEnvironment();
 
         if (!this.process.isRunning(this.dir)) {
             this.cliConfig.set('running', null).save();
@@ -48,47 +44,56 @@ class Instance {
         return true;
     }
 
+    get config() {
+        let currentEnv = this.system.environment;
+
+        if (!this._config || this._config.environment !== currentEnv) {
+            this._config = new Config(path.join(this.dir, `config.${this.system.environment}.json`));
+            this._config.environment = this.system.environment;
+        }
+
+        return this._config;
+    }
+
+    get process() {
+        let name = this.config.get('process', 'local');
+
+        if (!this._process || name !== this._process.name) {
+            let manager = this.system.getProcessManager(name);
+            this._process = new manager.Class(this.ui, this.system, this);
+            this._process.name = name;
+        }
+
+        return this._process;
+    }
+
     constructor(ui, system, dir) {
         this.ui = ui;
         this.system = system;
         this.dir = dir;
     }
 
-    loadConfig(skipEnvCheck) {
-        // If we are starting in production mode but a development config exists and a production config doesn't,
-        // we want to start in development mode anyways.
+    checkEnvironment() {
         if (
-            !skipEnvCheck && this.system.production &&
+            this.system.production &&
             Config.exists(path.join(this.dir, 'config.development.json')) &&
-            !Config.exists(path.join(this.dir, 'config.production.json'))
+            !Config.exists(path.join('config.production.json'))
         ) {
             this.ui.log('Found a development config but not a production config, running in development mode instead', 'yellow');
             this.system.setEnvironment(true, true);
         }
-        this.config = new Config(path.join(this.dir, `config.${this.system.environment}.json`));
-        this.loadProcess();
     }
 
-    loadRunningConfig(setEnv, setNodeEnv) {
-        if (!this.running) {
+    loadRunningEnvironment(setEnv, setNodeEnv) {
+        let env = this.cliConfig.get('running');
+
+        if (!env) {
             throw new Error('This instance is not running.');
         }
 
-        let env = this.cliConfig.get('running');
         if (setEnv) {
             this.system.setEnvironment(env === 'development', setNodeEnv);
         }
-
-        this.config = new Config(path.join(this.dir, `config.${env}.json`));
-        this.loadProcess();
-        return env;
-    }
-
-    loadProcess() {
-        let name = this.config.get('process', 'local');
-        let manager = this.system.getProcessManager(name);
-        this.process = new manager.Class(this.ui, this.system, this);
-        this.processName = manager.name;
     }
 
     summary() {
@@ -101,17 +106,17 @@ class Instance {
             };
         }
 
-        let env = this.loadRunningConfig();
+        this.loadRunningEnvironment(true);
 
         return {
             name: this.name,
             dir: this.dir.replace(os.homedir(), '~'),
             running: true,
             version: this.cliConfig.get('active-version'),
-            mode: env,
+            mode: this.system.environment,
             url: this.config.get('url'),
             port: this.config.get('server.port'),
-            process: this.processName
+            process: this._process.name
         };
     }
 

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,6 @@ class ConfigCommand extends Command {`
`14`	`14`	`super(ui, system);`
`15`	`15`
`16`	`16`	`let instance = this.system.getInstance();`
`17`		`- instance.loadConfig(true);`
`18`	`17`	`this.config = instance.config;`
`19`	`18`	`}`
`20`	`19`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ class RestartCommand extends Command {`
`11`	`11`	`return Promise.reject(new Error('Ghost instance is not currently running.'));`
`12`	`12`	`}`
`13`	`13`
`14`		`- instance.loadRunningConfig(true, true);`
	`14`	`+ instance.loadRunningEnvironment(true, true);`
`15`	`15`
`16`	`16`	`return this.runCommand(StopCommand, argv).then(() => this.runCommand(StartCommand, argv));`
`17`	`17`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,6 @@ class RunCommand extends Command {`
`13`	`13`	`}`
`14`	`14`
`15`	`15`	`let instance = this.system.getInstance();`
`16`		`- instance.loadConfig(true);`
`17`	`16`
`18`	`17`	`process.env.paths__contentPath = path.join(process.cwd(), 'content');`
`19`	`18`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,6 @@ class SetupCommand extends Command {`
`66`	`66`	`title: 'Setting up instance',`
`67`	`67`	`task: () => {`
`68`	`68`	`let instance = this.system.getInstance();`
`69`		`- instance.loadConfig();`
`70`	`69`	`instance.name = argv.pname \|\| url.parse(instance.config.get('url')).hostname.replace(/\./g, '-');`
`71`	`70`	`this.system.addInstance(instance);`
`72`	`71`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ class StartCommand extends Command {`
`27`	`27`	`return Promise.reject(new Error('Ghost is already running.'));`
`28`	`28`	`}`
`29`	`29`
`30`		`- instance.loadConfig();`
	`30`	`+ instance.checkEnvironment();`
`31`	`31`
`32`	`32`	`return this.ui.listr(startupChecks.concat({`
`33`	`33`	`title: 'Running database migrations',`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ class StopCommand extends Command {`
`41`	`41`	`return Promise.reject(new errors.SystemError('No running Ghost instance found here.'));`
`42`	`42`	`}`
`43`	`43`
`44`		`- instance.loadRunningConfig();`
	`44`	`+ instance.loadRunningEnvironment(true);`
`45`	`45`
`46`	`46`	`let stop = () => Promise.resolve(instance.process.stop(process.cwd())).then(() => {`
`47`	`47`	`instance.running = null;`