From 7cbcac0bb233ade18eb76ba96d2236c09a8fd4ff Mon Sep 17 00:00:00 2001
From: Shane Handley <sh@ie.com.au>
Date: Sun, 30 Dec 2018 18:33:18 +1100
Subject: [PATCH] Load config earlier in pluginManager and store as instance
 property to prevent duplicate calls to getServerlessConfigFile.

---
 lib/Serverless.js                 |  1 +
 lib/classes/PluginManager.js      | 24 ++++++++--------
 lib/classes/PluginManager.test.js | 48 +++++++++++++++++++------------
 3 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/lib/Serverless.js b/lib/Serverless.js
index 28a8a89dc..c82556782 100644
--- a/lib/Serverless.js
+++ b/lib/Serverless.js
@@ -58,6 +58,7 @@ class Serverless {
     this.processedInput = this.cli.processInput();
 
     // set the options and commands which were processed by the CLI
+    this.pluginManager.loadConfig();
     this.pluginManager.setCliOptions(this.processedInput.options);
     this.pluginManager.setCliCommands(this.processedInput.commands);
 
diff --git a/lib/classes/PluginManager.js b/lib/classes/PluginManager.js
index 9155777a7..9ec7b61b5 100644
--- a/lib/classes/PluginManager.js
+++ b/lib/classes/PluginManager.js
@@ -29,6 +29,7 @@ class TerminateHookChain extends Error {
 class PluginManager {
   constructor(serverless) {
     this.serverless = serverless;
+    this.config = null;
 
     this.cliOptions = {};
     this.cliCommands = [];
@@ -40,6 +41,12 @@ class PluginManager {
     this.deprecatedEvents = {};
   }
 
+  loadConfig() {
+    getServerlessConfigFile(this.serverless.config.servicePath).then((serverlessConfigFile) => {
+      this.config = serverlessConfigFile;
+    });
+  }
+
   setCliOptions(options) {
     this.cliOptions = options;
   }
@@ -436,19 +443,12 @@ class PluginManager {
    */
   validateServerlessConfigDependency(command) {
     if ('configDependent' in command && command.configDependent) {
-      return getServerlessConfigFile().then((serverlessConfigFile) => {
-        if (!serverlessConfigFile) {
-          const error = new this.serverless.classes.Error(
-            'This command can only be run in a Serverless service directory'
-          );
-          return BbPromise.reject(error);
-        }
-
-        return BbPromise.resolve();
-      });
+      if (!this.config) {
+        throw new this.serverless.classes.Error(
+          'This command can only be run in a Serverless service directory'
+        );
+      }
     }
-
-    return BbPromise.resolve();
   }
 
   validateOptions(command) {
diff --git a/lib/classes/PluginManager.test.js b/lib/classes/PluginManager.test.js
index 0bb8aaa89..b41dd2b97 100644
--- a/lib/classes/PluginManager.test.js
+++ b/lib/classes/PluginManager.test.js
@@ -1184,7 +1184,7 @@ describe('PluginManager', () => {
     });
 
     it('should continue loading if the configDependent property is absent', () => {
-      getServerlessConfigFileStub.resolves({ service: 'my-service' });
+      pluginManagerInstance.config = null;
 
       pluginManagerInstance.commands = {
         foo: {},
@@ -1192,11 +1192,11 @@ describe('PluginManager', () => {
 
       const foo = pluginManagerInstance.commands.foo;
 
-      return expect(pluginManagerInstance.validateServerlessConfigDependency(foo)).to.be.fulfilled;
+      expect(pluginManagerInstance.validateServerlessConfigDependency(foo)).to.be.undefined;
     });
 
-    it('should load if the configDependent property has a false value', () => {
-      getServerlessConfigFileStub.resolves('');
+    it('should load if the configDependent property has a false value and config is null', () => {
+      pluginManagerInstance.config = null;
 
       pluginManagerInstance.commands = {
         foo: {
@@ -1206,11 +1206,11 @@ describe('PluginManager', () => {
 
       const foo = pluginManagerInstance.commands.foo;
 
-      return expect(pluginManagerInstance.validateServerlessConfigDependency(foo)).to.be.fillfilled;
+      expect(pluginManagerInstance.validateServerlessConfigDependency(foo)).to.be.undefined;
     });
 
-    it('should load if the configDependent property has a true value, and config is found', () => {
-      getServerlessConfigFileStub.resolves({ service: 'my-service' });
+    it('should throw an error if configDependent has a true value and no config is found', () => {
+      pluginManagerInstance.config = null;
 
       pluginManagerInstance.commands = {
         foo: {
@@ -1220,15 +1220,11 @@ describe('PluginManager', () => {
 
       const foo = pluginManagerInstance.commands.foo;
 
-      return expect(
-        pluginManagerInstance.validateServerlessConfigDependency(foo)
-      ).to.be.fulfilled.then(() => {
-        expect(getServerlessConfigFileStub).to.have.been.calledOnce;
-      });
+      expect(() => { pluginManager.validateServerlessConfigDependency(foo); }).to.throw(Error);
     });
 
-    it('should error if configDependent has a true value and no config is found', () => {
-      getServerlessConfigFileStub.resolves('');
+    it('should throw an error if configDependent has a true value and config was returned as an empty string', () => {
+      pluginManagerInstance.config = '';
 
       pluginManagerInstance.commands = {
         foo: {
@@ -1238,12 +1234,26 @@ describe('PluginManager', () => {
 
       const foo = pluginManagerInstance.commands.foo;
 
-      return expect(
-        pluginManagerInstance.validateServerlessConfigDependency(foo)
-      ).to.be.rejected.then(() => {
-        expect(getServerlessConfigFileStub).to.have.been.calledOnce;
-      });
+      expect(() => { pluginManager.validateServerlessConfigDependency(foo); }).to.throw(Error);
     });
+
+    it('should load if the configDependent property has a true value, and config exists', () => {
+      pluginManagerInstance.config = {
+        servicePath: 'foo'
+      };
+
+      pluginManagerInstance.commands = {
+        foo: {
+          configDependent: true,
+        },
+      };
+
+      const foo = pluginManagerInstance.commands.foo;
+
+      expect(pluginManagerInstance.validateServerlessConfigDependency(foo)).to.be.undefined;
+    });
+
+
   });
 
   describe('#validateOptions()', () => {