From c3674e1293ffebe85f13751ecbad4cbb693ba833 Mon Sep 17 00:00:00 2001
From: Daniel Schep <dschep@gmail.com>
Date: Wed, 21 Aug 2019 10:22:05 -0400
Subject: [PATCH] safer & more explicit implementation of
 variableResolvesrs(also renamed)

---
 docs/providers/aws/guide/plugins.md | 14 ++++++++------
 lib/classes/PluginManager.js        | 27 ++++++++++++++++++++++-----
 lib/classes/PluginManager.test.js   |  4 ++--
 lib/classes/Variables.js            |  4 ++--
 4 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/docs/providers/aws/guide/plugins.md b/docs/providers/aws/guide/plugins.md
index dd752f535..f1d748fd6 100644
--- a/docs/providers/aws/guide/plugins.md
+++ b/docs/providers/aws/guide/plugins.md
@@ -213,13 +213,15 @@ class EchoTestVarPlugin {
     getDependentEchoTestValue(src) {
       return src.slice(5);
     }
-    // if a variable type depends on profile/stage/region/credentials, to avoid infinite loops in
-    // trying to resolve variables that depend on themselves, specify as such by setting a
-    // dependendServiceName property on the variable getter
-    getDependentEchoTestValue.dependendServiceName = 'StateDependentEcho'
-    this.variableGetters = {
+    this.variableResolvers = {
       echo: this.getEchoTestValue,
-      echoStageDependent: getDependentEchoTestValue
+      // if a variable type depends on profile/stage/region/credentials, to avoid infinite loops in
+      // trying to resolve variables that depend on themselves, specify as such by setting a
+      // dependendServiceName property on the variable getter
+      echoStageDependent: : {
+        resolver: this.getDependentEchoTestValue,
+        serviceName: 'echo that isnt prepopulated',
+        isDisabledAtPrepopulation: true
     };
   }
 }
diff --git a/lib/classes/PluginManager.js b/lib/classes/PluginManager.js
index 86a64e440..abc280fa4 100644
--- a/lib/classes/PluginManager.js
+++ b/lib/classes/PluginManager.js
@@ -311,19 +311,36 @@ class PluginManager {
   }
 
   loadVariableGetters(pluginInstance) {
-    for (const [variablePrefix, variableGetter] of _.entries(
-      pluginInstance.variableGetters || {}
+    for (const [variablePrefix, resolverOrOptions] of _.entries(
+      pluginInstance.variableResolvers || {}
     )) {
-      this.serverless.variables.customVariableResolverFuncs[variablePrefix] = variableGetter.bind(
+      let options = resolverOrOptions;
+      if (typeof resolverOrOptions === 'function') {
+        options = {
+          resolver: resolverOrOptions,
+        };
+      }
+      this.serverless.variables.customVariableResolverFuncs[variablePrefix] = options.resolver.bind(
         this.serverless.variables
       );
+      if (
+        this.serverless.variables.variableResolvers
+          .filter(([, , custom]) => custom)
+          .map(([, funcName]) => funcName)
+          .includes(variablePrefix)
+      ) {
+        throw new Error(
+          `Plugin ${pluginInstance.constructor.name} is trying to add support for $\{${variablePrefix}:} variables but another plugin already added it`
+        );
+      }
       this.serverless.variables.variableResolvers.push([
         new RegExp(`^${variablePrefix}:`),
         variablePrefix,
+        true,
       ]);
-      if (variableGetter.dependentServiceName) {
+      if (options.isDisabledAtPrepopulation) {
         this.serverless.variables.dependentServices.push({
-          name: variableGetter.dependentServiceName,
+          name: options.serviceName,
           method: variablePrefix,
           custom: true,
         });
diff --git a/lib/classes/PluginManager.test.js b/lib/classes/PluginManager.test.js
index a7e9305ae..b225f5499 100644
--- a/lib/classes/PluginManager.test.js
+++ b/lib/classes/PluginManager.test.js
@@ -1045,7 +1045,7 @@ describe('PluginManager', () => {
           return Promise.resolve('testVariable');
         }
         constructor() {
-          this.variableGetters = {
+          this.variableResolvers = {
             test: this.getTestVariable,
           };
         }
@@ -1058,7 +1058,7 @@ describe('PluginManager', () => {
       pluginManager.loadVariableGetters(pluginInstance);
 
       expect(pluginManager.serverless.variables.variableResolvers).to.deep.equal([
-        [/^test:/, 'test'],
+        [/^test:/, 'test', true],
       ]);
       expect(
         Object.keys(pluginManager.serverless.variables.customVariableResolverFuncs)
diff --git a/lib/classes/Variables.js b/lib/classes/Variables.js
index 209b8c878..dd9cde342 100644
--- a/lib/classes/Variables.js
+++ b/lib/classes/Variables.js
@@ -552,9 +552,9 @@ class Variables {
     if (this.tracker.contains(variableString)) {
       ret = this.tracker.get(variableString, propertyString);
     } else {
-      for (const [regex, getter] of this.variableResolvers) {
+      for (const [regex, getter, custom] of this.variableResolvers) {
         if (variableString.match(regex)) {
-          if (this[getter]) {
+          if (!custom) {
             ret = this[getter].bind(this)(variableString);
           } else {
             ret = this.customVariableResolverFuncs[getter](variableString);