This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git

commit 12ee696d26dcb18bfb1c487b6708358e67b6f82a
Author: Alex Heneveld <[email protected]>
AuthorDate: Wed Nov 9 12:32:08 2022 +0000

    track stage where var is being resolved so we can handle some special 
recursive references
---
 .../brooklyn/camp/brooklyn/WorkflowYamlTest.java   |   2 -
 .../core/workflow/WorkflowErrorHandling.java       |   2 +-
 .../core/workflow/WorkflowExecutionContext.java    |  44 ++---
 .../workflow/WorkflowExpressionResolution.java     | 217 +++++++++++++++++----
 .../core/workflow/WorkflowStepDefinition.java      |   2 +-
 .../WorkflowStepInstanceExecutionContext.java      |  26 +--
 .../workflow/steps/ClearConfigWorkflowStep.java    |   3 +-
 .../workflow/steps/ClearSensorWorkflowStep.java    |   3 +-
 .../workflow/steps/ClearVariableWorkflowStep.java  |   3 +-
 .../core/workflow/steps/LoadWorkflowStep.java      |   5 +-
 .../core/workflow/steps/SetConfigWorkflowStep.java |   3 +-
 .../core/workflow/steps/SetSensorWorkflowStep.java |   3 +-
 .../workflow/steps/SetVariableWorkflowStep.java    |   9 +-
 .../core/workflow/steps/WaitWorkflowStep.java      |  17 +-
 .../brooklyn/util/core/text/TemplateProcessor.java |  29 +--
 .../util/collections/ThreadLocalStack.java         |  90 +++++++++
 16 files changed, 348 insertions(+), 110 deletions(-)

diff --git 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
index 9cdacacdbb..8ef25ca69e 100644
--- 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
+++ 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
@@ -25,7 +25,6 @@ import com.google.common.collect.Iterables;
 import org.apache.brooklyn.api.effector.Effector;
 import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.Entity;
-import org.apache.brooklyn.api.entity.EntityInitializer;
 import org.apache.brooklyn.api.entity.EntitySpec;
 import org.apache.brooklyn.api.location.LocationSpec;
 import org.apache.brooklyn.api.location.MachineLocation;
@@ -43,7 +42,6 @@ import org.apache.brooklyn.core.entity.trait.Startable;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.TestApplication;
-import org.apache.brooklyn.core.test.entity.TestEntity;
 import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
 import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
index 3ab02f17fe..0a4a102544 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
@@ -126,7 +126,7 @@ public class WorkflowErrorHandling implements 
Callable<WorkflowErrorHandling.Wor
             result.output = DynamicTasks.queue(handlerI).getUnchecked();
 
             if (errorStep.output!=null) {
-                result.output = handlerContext.resolve(errorStep.output, 
Object.class);
+                result.output = 
handlerContext.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_FINISHING_POST_OUTPUT,
 errorStep.output, Object.class);
             }
             result.next = errorStep.getNext();
             log.debug("Completed handler " + potentialTaskName + "; proceeding 
to " + (result.next!=null ? result.next : "default next step"));
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
index 7c5ace49f0..31621cb28a 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
@@ -226,7 +226,7 @@ public class WorkflowExecutionContext {
         WorkflowStepResolution.resolveOnErrorSteps(w.getManagementContext(), 
w.onError);
 
         // some fields need to be resolved at setting time, in the context of 
the workflow
-        
w.setCondition(w.resolveWrapped(paramsDefiningWorkflow.getStringKey(WorkflowCommonConfig.CONDITION.getName()),
 WorkflowCommonConfig.CONDITION.getTypeToken()));
+        
w.setCondition(w.resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage.WORKFLOW_STARTING_POST_INPUT,
 paramsDefiningWorkflow.getStringKey(WorkflowCommonConfig.CONDITION.getName()), 
WorkflowCommonConfig.CONDITION.getTypeToken()));
 
         // finished -- checkpoint noting this has been created but not yet 
started
         w.status = WorkflowStatus.STAGED;
@@ -510,42 +510,42 @@ public class WorkflowExecutionContext {
         return new 
BrooklynTypeNameResolution.BrooklynTypeNameResolver("workflow", loader, true, 
true).getTypeToken(typeName);
     }
 
-    /** as {@link #resolve(Object, TypeToken)} but without type coercion */
-    public Object resolve(String expression) {
-        return resolve(expression, Object.class);
+    /** as {@link 
#resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, 
TypeToken)} but without type coercion */
+    public Object resolve(WorkflowExpressionResolution.WorkflowExpressionStage 
stage, String expression) {
+        return resolve(stage, expression, Object.class);
     }
 
-    /** as {@link #resolve(Object, TypeToken)} */
-    public <T> T resolve(Object expression, Class<T> type) {
-        return resolve(expression, TypeToken.of(type));
+    /** as {@link 
#resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, 
TypeToken)} */
+    public <T> T resolve(WorkflowExpressionResolution.WorkflowExpressionStage 
stage, Object expression, Class<T> type) {
+        return resolve(stage, expression, TypeToken.of(type));
     }
 
     /** resolution of ${interpolation} and $brooklyn:dsl and deferred 
suppliers, followed by type coercion.
      * if the type is a string, null is not permitted, otherwise it is. */
-    public <T> T resolve(Object expression, TypeToken<T> type) {
-        return new WorkflowExpressionResolution(this, false, 
false).resolveWithTemplates(expression, type);
+    public <T> T resolve(WorkflowExpressionResolution.WorkflowExpressionStage 
stage, Object expression, TypeToken<T> type) {
+        return new WorkflowExpressionResolution(this, stage, false, 
false).resolveWithTemplates(expression, type);
     }
 
-    public <T> T resolveCoercingOnly(Object expression, TypeToken<T> type) {
-        return new WorkflowExpressionResolution(this, false, 
false).resolveCoercingOnly(expression, type);
+    public <T> T 
resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage stage, 
Object expression, TypeToken<T> type) {
+        return new WorkflowExpressionResolution(this, stage, false, 
false).resolveCoercingOnly(expression, type);
     }
 
-    /** as {@link #resolve(Object, TypeToken)}, but returning DSL/supplier for 
values (so the indication of their dynamic nature is preserved, even if the 
value returned by it is resolved;
+    /** as {@link 
#resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, 
TypeToken)}, but returning DSL/supplier for values (so the indication of their 
dynamic nature is preserved, even if the value returned by it is resolved;
      * this is needed e.g. for conditions which treat dynamic expressions 
differently to explicit values) */
-    public <T> T resolveWrapped(Object expression, TypeToken<T> type) {
-        return new WorkflowExpressionResolution(this, false, 
true).resolveWithTemplates(expression, type);
+    public <T> T 
resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, 
Object expression, TypeToken<T> type) {
+        return new WorkflowExpressionResolution(this, stage, false, 
true).resolveWithTemplates(expression, type);
     }
 
-    /** as {@link #resolve(Object, TypeToken)}, but waiting on any expressions 
which aren't ready */
-    public <T> T resolveWaiting(Object expression, TypeToken<T> type) {
-        return new WorkflowExpressionResolution(this, true, 
false).resolveWithTemplates(expression, type);
+    /** as {@link 
#resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, 
TypeToken)}, but waiting on any expressions which aren't ready */
+    public <T> T 
resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage stage, 
Object expression, TypeToken<T> type) {
+        return new WorkflowExpressionResolution(this, stage, true, 
false).resolveWithTemplates(expression, type);
     }
 
     /** resolution of ${interpolation} and $brooklyn:dsl and deferred 
suppliers, followed by type coercion */
-    public <T> T resolveConfig(ConfigBag config, ConfigKey<T> key) {
+    public <T> T 
resolveConfig(WorkflowExpressionResolution.WorkflowExpressionStage stage, 
ConfigBag config, ConfigKey<T> key) {
         Object v = config.getStringKey(key.getName());
         if (v==null) return null;
-        return resolve(v, key.getTypeToken());
+        return resolve(stage, v, key.getTypeToken());
     }
 
     public Integer getCurrentStepIndex() {
@@ -776,7 +776,7 @@ public class WorkflowExecutionContext {
                         }
 
                         if (outputDefinition != null) {
-                            output = resolve(outputDefinition, Object.class);
+                            output = 
resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_FINISHING_POST_OUTPUT,
 outputDefinition, Object.class);
                         } else {
                             // (default is the output of the last step)
                             // ((unlike steps, workflow output is not 
available as a default value, but previous step always is, so there is no need 
to do this
@@ -862,7 +862,7 @@ public class WorkflowExecutionContext {
                                 if (result != null) {
                                     errorHandled = true;
 
-                                    if (result.output != null) output = 
resolve(result.output, Object.class);
+                                    if (result.output != null) output = 
resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_FINISHING_POST_OUTPUT,
 result.output, Object.class);
 
                                     String next = 
Strings.isNonBlank(result.next) ? result.next : "end";
                                     moveToNextStep(next, "Handled error in 
workflow around step " + workflowStepReference(currentStepIndex));
@@ -994,7 +994,7 @@ public class WorkflowExecutionContext {
             AtomicReference<String> customNext = new AtomicReference<>();
             Runnable next = () -> 
moveToNextStep(Strings.isNonBlank(customNext.get()) ? customNext.get() : 
step.getNext(), "Completed step "+ workflowStepReference(currentStepIndex));
             Consumer<Object> saveOutput = output -> {
-                if (output!=null) currentStepInstance.output = resolve(output, 
Object.class);
+                if (output!=null) currentStepInstance.output = 
resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_FINISHING_POST_OUTPUT,
 output, Object.class);
             };
 
             try {
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java
index 90aa7b6b05..0f23092745 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java
@@ -26,37 +26,48 @@ import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import 
org.apache.brooklyn.core.resolve.jackson.BrooklynJacksonSerializationUtils;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
-import org.apache.brooklyn.util.collections.Jsonya;
-import org.apache.brooklyn.util.collections.MutableList;
-import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.collections.*;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import 
org.apache.brooklyn.util.core.predicates.ResolutionFailureTreatedAsAbsent;
 import org.apache.brooklyn.util.core.task.DeferredSupplier;
-import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.core.task.Tasks;
-import org.apache.brooklyn.util.core.task.ValueResolver;
 import org.apache.brooklyn.util.core.text.TemplateProcessor;
 import org.apache.brooklyn.util.exceptions.Exceptions;
-import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.javalang.Boxing;
-import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.swing.*;
 import java.util.*;
+import java.util.concurrent.Callable;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 public class WorkflowExpressionResolution {
 
+    public enum WorkflowExpressionStage implements 
Comparable<WorkflowExpressionStage> {
+        WORKFLOW_INPUT,
+        WORKFLOW_STARTING_POST_INPUT,
+        STEP_PRE_INPUT,
+        STEP_INPUT,
+        STEP_RUNNING,
+        STEP_OUTPUT,
+        STEP_FINISHING_POST_OUTPUT,
+        WORKFLOW_OUTPUT;
+
+        public boolean after(WorkflowExpressionStage other) {
+            return compareTo(other) > 0;
+        }
+    }
+
     private static final Logger log = 
LoggerFactory.getLogger(WorkflowExpressionResolution.class);
     private final WorkflowExecutionContext context;
     private final boolean allowWaiting;
     private final boolean useWrappedValue;
+    private final WorkflowExpressionStage stage;
 
-    public WorkflowExpressionResolution(WorkflowExecutionContext context, 
boolean allowWaiting, boolean wrapExpressionValues) {
+    public WorkflowExpressionResolution(WorkflowExecutionContext context, 
WorkflowExpressionStage stage, boolean allowWaiting, boolean 
wrapExpressionValues) {
         this.context = context;
+        this.stage = stage;
         this.allowWaiting = allowWaiting;
         this.useWrappedValue = wrapExpressionValues;
     }
@@ -70,6 +81,8 @@ public class WorkflowExpressionResolution {
     class WorkflowFreemarkerModel implements TemplateHashModel {
         @Override
         public TemplateModel get(String key) throws TemplateModelException {
+            List<Throwable> errors = MutableList.of();
+
             if ("workflow".equals(key)) {
                 return new WorkflowExplicitModel();
             }
@@ -83,33 +96,84 @@ public class WorkflowExpressionResolution {
             Object candidate;
 
             //workflow.current_step.input.somevar
-            WorkflowStepInstanceExecutionContext currentStep = 
context.currentStepInstance;
-            if (currentStep!=null) {
-                if (currentStep.output instanceof Map) {
-                    candidate = ((Map) currentStep.output).get(key);
-                    if (candidate!=null) return 
TemplateProcessor.wrapAsTemplateModel(candidate);
+            if (stage.after(WorkflowExpressionStage.STEP_PRE_INPUT)) {
+                WorkflowStepInstanceExecutionContext currentStep = 
context.currentStepInstance;
+                if (currentStep != null && 
stage.after(WorkflowExpressionStage.STEP_OUTPUT)) {
+                    if (currentStep.output instanceof Map) {
+                        candidate = ((Map) currentStep.output).get(key);
+                        if (candidate != null) return 
TemplateProcessor.wrapAsTemplateModel(candidate);
+                    }
                 }
 
-                candidate = currentStep.getInput(key, Object.class);
-                if (candidate!=null) return 
TemplateProcessor.wrapAsTemplateModel(candidate);
+                try {
+                    candidate = currentStep.getInput(key, Object.class);
+                } catch (Throwable t) {
+                    Exceptions.propagateIfFatal(t);
+                    if (stage==WorkflowExpressionStage.STEP_INPUT && 
WorkflowVariableResolutionStackEntry.isStackForSettingVariable(RESOLVE_STACK.getAll(true),
 key) && Exceptions.getFirstThrowableOfType(t, 
WorkflowVariableRecursiveReference.class)!=null) {
+
+                        // input evaluation can look at local input, and will 
gracefully handle some recursive references.
+                        // this is needed so we can handle things like 
env:=${env} in input, and also {message:="Hi ${name}", name:="Bob"}.
+                        // but there are
+                        // if we have a chain input1:=input2, and input 
input2:=input1 with both defined on step and on workflow
+                        //
+                        // (a) eval of either will give recursive reference 
error and allow retry immediately;
+                        //     then it's a bit weird, inconsistent, step 
input1 will resolve to local input2 which resolves as global input1;
+                        //     but step input2 will resolve to local input1 
which this time will resolve as global input2.
+                        //     and whichever is invoked first will cause both 
to be stored as resolved, so if input2 resolved first then
+                        //     step input1 subsequently returns global input2.
+                        //
+                        // (b) recursive reference error only recoverable at 
the outermost stage,
+                        //     so step input1 = global input2, step input2 = 
global input1,
+                        //     prevents inconsistency but blocks useful 
things, eg log ${message} wrapped with message:="Hi ${name}",
+                        //     then invoked with name: "person who says 
${message}" to refer to a previous step's message,
+                        //     or even name:="Mr ${name}" to refer to an outer 
variable.
+                        //     in this case if name is resolved first then 
message resolves as Hi Mr X, but if message resolved first
+                        //     it only recovers when resolving message which 
would become "Hi X", and if message:="${greeting} ${name}"
+                        //     then it fails to find a local ${greeting}. 
(with strategy (a) these both do what is expected.)
+                        //
+                        // (to handle this we include stage in the stack, 
needed in both cases above)
+                        //
+                        // ideally we would know which vars are from a 
wrapper, but that info is lost when we build up the step
+                        //
+                        // (c) we could just fail fast, disallow the nice 
things we wanted, require explicit
+                        //
+                        // (d) we could fail in edge cases, so the obvious 
cases above work as expected, but anything more sophisticated, eg A calling B 
calling A, will fail
+                        //
+                        // settled on (d) effectively; we allow local 
references, and fail on recursive references, with exceptions.
+                        // the main exception, handled here, is if we are 
setting an input
+                        candidate = null;
+                        errors.add(t);
+                    } else {
+                        throw Exceptions.propagate(t);
+                    }
+                }
+                if (candidate != null) return 
TemplateProcessor.wrapAsTemplateModel(candidate);
             }
+
             //workflow.previous_step.output.somevar
-            Object prevStepOutput = context.getPreviousStepOutput();
-            if (prevStepOutput instanceof Map) {
-                candidate = ((Map)prevStepOutput).get(key);
-                if (candidate!=null) return 
TemplateProcessor.wrapAsTemplateModel(candidate);
+            if (stage.after(WorkflowExpressionStage.WORKFLOW_INPUT)) {
+                Object prevStepOutput = context.getPreviousStepOutput();
+                if (prevStepOutput instanceof Map) {
+                    candidate = ((Map) prevStepOutput).get(key);
+                    if (candidate != null) return 
TemplateProcessor.wrapAsTemplateModel(candidate);
+                }
             }
 
             //workflow.scratch.somevar
-            candidate = context.workflowScratchVariables.get(key);
-            if (candidate!=null) return 
TemplateProcessor.wrapAsTemplateModel(candidate);
+            if (stage.after(WorkflowExpressionStage.WORKFLOW_INPUT)) {
+                candidate = context.workflowScratchVariables.get(key);
+                if (candidate != null) return 
TemplateProcessor.wrapAsTemplateModel(candidate);
+            }
 
             //workflow.input.somevar
             if (context.input.containsKey(key)) {
                 candidate = context.getInput(key);
+                // the subtlety around step input above doesn't apply here as 
workflow inputs are not resolved with freemarker
                 if (candidate != null) return 
TemplateProcessor.wrapAsTemplateModel(candidate);
             }
 
+            if (!errors.isEmpty()) Exceptions.propagate("Errors resolving 
"+key, errors);
+
             return ifNoMatches();
         }
 
@@ -233,21 +297,103 @@ public class WorkflowExpressionResolution {
         }
     }
 
-    static ThreadLocal<Set<Pair<WorkflowExecutionContext,Object>>> 
RESOLVE_STACK = new ThreadLocal<>();
+    static class WorkflowVariableResolutionStackEntry {
+        WorkflowExecutionContext context;
+        WorkflowExpressionStage stage;
+        Object object;
+        String settingVariable;
+
+        public static WorkflowVariableResolutionStackEntry 
of(WorkflowExecutionContext context, WorkflowExpressionStage stage, Object 
expression) {
+            WorkflowVariableResolutionStackEntry result = new 
WorkflowVariableResolutionStackEntry();
+            result.context = context;
+            result.stage = stage;
+            result.object = expression;
+            return result;
+        }
 
-    public Object processTemplateExpression(Object expression) {
-        Set<Pair<WorkflowExecutionContext,Object>> stack = null;
+        public static WorkflowVariableResolutionStackEntry 
setting(WorkflowExecutionContext context, WorkflowExpressionStage stage, String 
settingVariable) {
+            WorkflowVariableResolutionStackEntry result = new 
WorkflowVariableResolutionStackEntry();
+            result.context = context;
+            result.stage = stage;
+            result.settingVariable = settingVariable;
+            return result;
+        }
+
+        public static boolean 
isStackForSettingVariable(Collection<WorkflowVariableResolutionStackEntry> 
stack, String key) {
+            if (stack==null) return true;
+            MutableList<WorkflowVariableResolutionStackEntry> s2 = 
MutableList.copyOf(stack);
+            Collections.reverse(s2);
+            Optional<WorkflowVariableResolutionStackEntry> s = 
s2.stream().filter(si -> si.settingVariable != null).findFirst();
+            if (!s.isPresent()) return false;
+            return s.get().settingVariable.equals(key);
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            WorkflowVariableResolutionStackEntry that = 
(WorkflowVariableResolutionStackEntry) o;
+
+            if (context != null && that.context != null ? 
!Objects.equals(context.getWorkflowId(), that.context.getWorkflowId()) : 
!Objects.equals(context, that.context)) return false;
+            if (stage != that.stage) return false;
+            if (object != null ? !object.equals(that.object) : that.object != 
null) return false;
+            if (settingVariable != null ? 
!settingVariable.equals(that.settingVariable) : that.settingVariable != null) 
return false;
+            return true;
+        }
+
+        @Override
+        public int hashCode() {
+            int result = context != null && context.getWorkflowId()!=null ? 
context.getWorkflowId().hashCode() : 0;
+            result = 31 * result + (stage != null ? stage.hashCode() : 0);
+            result = 31 * result + (object != null ? object.hashCode() : 0);
+            result = 31 * result + (settingVariable != null ? 
settingVariable.hashCode() : 0);
+            return result;
+        }
+    }
+
+    /** method which can be used to indicate that a reference to the variable, 
if it is recursive, is recoverable, because we are in the process of setting 
that variable.
+     * see discussion on usages of 
WorkflowVariableResolutionStackEntry.isStackForSettingVariable */
+    public static <T> T allowingRecursionWhenSetting(WorkflowExecutionContext 
context, WorkflowExpressionStage stage, String variable, Supplier<T> callable) {
+        WorkflowVariableResolutionStackEntry entry = null;
         try {
-            stack = RESOLVE_STACK.get();
-            if (stack==null) {
-                stack = MutableSet.of();
-                RESOLVE_STACK.set(stack);
+            entry = WorkflowVariableResolutionStackEntry.setting(context, 
stage, variable);
+            if (!RESOLVE_STACK.push(entry)) {
+                entry = null;
+                throw new WorkflowVariableRecursiveReference("Recursive 
reference setting "+variable+": "+RESOLVE_STACK.getAll(false).stream().map(p -> 
p.object!=null ? p.object.toString() : 
p.settingVariable).collect(Collectors.joining("->")));
+            }
+
+            return callable.get();
+
+        } finally {
+            if (entry!=null) {
+                RESOLVE_STACK.pop(entry);
             }
-            if (!stack.add(Pair.of(context, expression))) {
-                throw new IllegalStateException("Recursive reference: 
"+stack.stream().map(p -> ""+p.getRight()).collect(Collectors.joining("->")));
+        }
+    }
+
+    static ThreadLocalStack<WorkflowVariableResolutionStackEntry> 
RESOLVE_STACK = new ThreadLocalStack<>(false);
+
+    WorkflowExpressionStage previousStage() {
+        return RESOLVE_STACK.peekPenultimate().map(s -> s.stage).orNull();
+    }
+
+    public static class WorkflowVariableRecursiveReference extends 
IllegalArgumentException {
+        public WorkflowVariableRecursiveReference(String msg) {
+            super(msg);
+        }
+    }
+
+    public Object processTemplateExpression(Object expression) {
+        WorkflowVariableResolutionStackEntry entry = null;
+        try {
+            entry = WorkflowVariableResolutionStackEntry.of(context, stage, 
expression);
+            if (!RESOLVE_STACK.push(entry)) {
+                entry = null;
+                throw new WorkflowVariableRecursiveReference("Recursive 
reference: "+RESOLVE_STACK.getAll(false).stream().map(p -> 
""+p.object).collect(Collectors.joining("->")));
             }
-            if (stack.size()>100) {
-                throw new IllegalStateException("Reference exceeded max depth 
100: "+stack.stream().map(p -> 
""+p.getRight()).collect(Collectors.joining("->")));
+            if (RESOLVE_STACK.size()>100) {
+                throw new WorkflowVariableRecursiveReference("Reference 
exceeded max depth 100: "+RESOLVE_STACK.getAll(false).stream().map(p -> 
""+p.object).collect(Collectors.joining("->")));
             }
 
             if (expression instanceof String) return 
processTemplateExpressionString((String) expression);
@@ -258,8 +404,7 @@ public class WorkflowExpressionResolution {
             return resolveDsl(expression);
 
         } finally {
-            stack.remove(Pair.of(context, expression));
-            if (stack.isEmpty()) RESOLVE_STACK.remove();
+            if (entry!=null) RESOLVE_STACK.pop(entry);
         }
     }
 
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
index 3ef5f39934..16a89024cc 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java
@@ -86,7 +86,7 @@ public abstract class WorkflowStepDefinition {
     @JsonIgnore
     public DslPredicates.DslPredicate 
getConditionResolved(WorkflowStepInstanceExecutionContext context) {
         try {
-            return context.resolveWrapped(condition, 
TypeToken.of(DslPredicates.DslPredicate.class));
+            return 
context.resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 condition, TypeToken.of(DslPredicates.DslPredicate.class));
         } catch (Exception e) {
             throw Exceptions.propagateAnnotated("Unresolveable condition 
("+condition+")", e);
         }
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
index d05e734dda..8a747428ef 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java
@@ -121,10 +121,14 @@ public class WorkflowStepInstanceExecutionContext {
      * (Input is not resolved until first access because some implementations, 
such as 'let', might handle errors in resolution.
      * But once resolved we don't want inconsistent return values.) */
     public <T> T getInput(String key, TypeToken<T> type) {
+        return 
getInput(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, key, 
type);
+    }
+    public <T> T getInput(WorkflowExpressionResolution.WorkflowExpressionStage 
stage, String key, TypeToken<T> type) {
         if (inputResolved.containsKey(key)) return (T)inputResolved.get(key);
 
         Object v = input.get(key);
-        T v2 = context.resolve(v, type);
+        T v2 = 
WorkflowExpressionResolution.allowingRecursionWhenSetting(context, 
WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, key,
+                () -> context.resolve(stage, v, type));
         if (REMEMBER_RESOLVED_INPUT) {
             if (!Objects.equals(v, v2)) {
                 inputResolved.put(key, v2);
@@ -180,22 +184,22 @@ public class WorkflowStepInstanceExecutionContext {
         return context.lookupType(type, ifUnset);
     }
 
-    public Object resolve(String expression) {
-        return context.resolve(expression);
+    public Object resolve(WorkflowExpressionResolution.WorkflowExpressionStage 
stage, String expression) {
+        return context.resolve(stage, expression);
     }
 
-    public <T> T resolve(Object expression, Class<T> type) {
-        return context.resolve(expression, type);
+    public <T> T resolve(WorkflowExpressionResolution.WorkflowExpressionStage 
stage, Object expression, Class<T> type) {
+        return context.resolve(stage, expression, type);
     }
 
-    public <T> T resolve(Object expression, TypeToken<T> type) {
-        return context.resolve(expression, type);
+    public <T> T resolve(WorkflowExpressionResolution.WorkflowExpressionStage 
stage, Object expression, TypeToken<T> type) {
+        return context.resolve(stage, expression, type);
     }
-    public <T> T resolveWrapped(Object expression, TypeToken<T> type) {
-        return context.resolveWrapped(expression, type);
+    public <T> T 
resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, 
Object expression, TypeToken<T> type) {
+        return context.resolveWrapped(stage, expression, type);
     }
-    public <T> T resolveWaiting(Object expression, TypeToken<T> type) {
-        return context.resolveWaiting(expression, type);
+    public <T> T 
resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage stage, 
Object expression, TypeToken<T> type) {
+        return context.resolveWaiting(stage, expression, type);
     }
 
     @JsonIgnore
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearConfigWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearConfigWorkflowStep.java
index 16f3af72b6..b0054ef1fa 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearConfigWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearConfigWorkflowStep.java
@@ -24,6 +24,7 @@ import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.workflow.WorkflowExecutionContext;
+import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
 import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.text.Strings;
@@ -43,7 +44,7 @@ public class ClearConfigWorkflowStep extends 
WorkflowStepDefinition {
     protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
         EntityValueToSet config = context.getInput(CONFIG);
         if (config==null) throw new IllegalArgumentException("Config key name 
is required");
-        String configName = context.resolve(config.name, String.class);
+        String configName = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
 config.name, String.class);
         if (Strings.isBlank(configName)) throw new 
IllegalArgumentException("Config key name is required");
         TypeToken<?> type = context.lookupType(config.type, () -> 
TypeToken.of(Object.class));
         Entity entity = config.entity;
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearSensorWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearSensorWorkflowStep.java
index 9ee3175f00..f7057e880d 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearSensorWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearSensorWorkflowStep.java
@@ -26,6 +26,7 @@ import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.workflow.WorkflowExecutionContext;
+import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
 import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.core.task.Tasks;
@@ -46,7 +47,7 @@ public class ClearSensorWorkflowStep extends 
WorkflowStepDefinition {
     protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
         EntityValueToSet sensor = context.getInput(SENSOR);
         if (sensor==null) throw new IllegalArgumentException("Sensor name is 
required");
-        String sensorName = context.resolve(sensor.name, String.class);
+        String sensorName = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
 sensor.name, String.class);
         if (Strings.isBlank(sensorName)) throw new 
IllegalArgumentException("Sensor name is required");
         TypeToken<?> type = context.lookupType(sensor.type, () -> 
TypeToken.of(Object.class));
         Entity entity = sensor.entity;
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearVariableWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearVariableWorkflowStep.java
index 4f38db8612..29413d8705 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearVariableWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/ClearVariableWorkflowStep.java
@@ -20,6 +20,7 @@ package org.apache.brooklyn.core.workflow.steps;
 
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
 import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.text.Strings;
@@ -39,7 +40,7 @@ public class ClearVariableWorkflowStep extends 
WorkflowStepDefinition {
     protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
         TypedValueToSet variable = context.getInput(VARIABLE);
         if (variable ==null) throw new IllegalArgumentException("Variable name 
is required");
-        String name = context.resolve(variable.name, String.class);
+        String name = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
 variable.name, String.class);
         if (Strings.isBlank(name)) throw new 
IllegalArgumentException("Variable name is required");
         
context.getWorkflowExectionContext().getWorkflowScratchVariables().remove(name);
         return context.getPreviousStepOutput();
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/LoadWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/LoadWorkflowStep.java
index 73ccd2afb8..52d7653441 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/LoadWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/LoadWorkflowStep.java
@@ -25,6 +25,7 @@ import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.resolve.jackson.BrooklynJacksonType;
+import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
 import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.collections.CollectionMerger;
@@ -80,7 +81,7 @@ public class LoadWorkflowStep extends WorkflowStepDefinition {
     protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
         TypedValueToSet variable = context.getInput(VARIABLE);
         if (variable ==null) throw new IllegalArgumentException("Variable name 
is required");
-        String name = context.resolve(variable.name, String.class);
+        String name = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
 variable.name, String.class);
         if (Strings.isBlank(name)) throw new 
IllegalArgumentException("Variable name is required");
         TypeToken<?> type = context.lookupType(variable.type, () -> 
TypeToken.of(String.class));
 
@@ -95,7 +96,7 @@ public class LoadWorkflowStep extends WorkflowStepDefinition {
         } else {
             data = r.getResourceAsString("" + url);
         }
-        Object resolvedValue = context.resolve(data, type);
+        Object resolvedValue = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 data, type);
 
         Object oldValue = 
context.getWorkflowExectionContext().getWorkflowScratchVariables().put(name, 
resolvedValue);
 
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetConfigWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetConfigWorkflowStep.java
index f1d7e74869..eb79cca29d 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetConfigWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetConfigWorkflowStep.java
@@ -24,6 +24,7 @@ import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.workflow.WorkflowExecutionContext;
+import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
 import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.core.task.Tasks;
@@ -45,7 +46,7 @@ public class SetConfigWorkflowStep extends 
WorkflowStepDefinition {
     protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
         EntityValueToSet config = context.getInput(CONFIG);
         if (config ==null) throw new IllegalArgumentException("Config key name 
is required");
-        String configName = context.resolve(config.name, String.class);
+        String configName = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
 config.name, String.class);
         if (Strings.isBlank(configName)) throw new 
IllegalArgumentException("Config key name is required");
         TypeToken<?> type = context.lookupType(config.type, () -> 
TypeToken.of(Object.class));
         Object resolvedValue = context.getInput(VALUE.getName(), type);
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetSensorWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetSensorWorkflowStep.java
index 36154b7f7b..291af43542 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetSensorWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetSensorWorkflowStep.java
@@ -25,6 +25,7 @@ import org.apache.brooklyn.api.sensor.Sensor;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.sensor.Sensors;
+import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
 import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.text.Strings;
@@ -49,7 +50,7 @@ public class SetSensorWorkflowStep extends 
WorkflowStepDefinition {
     protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
         EntityValueToSet sensor = context.getInput(SENSOR);
         if (sensor==null) throw new IllegalArgumentException("Sensor name is 
required");
-        String sensorName = context.resolve(sensor.name, String.class);
+        String sensorName = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
 sensor.name, String.class);
         if (Strings.isBlank(sensorName)) throw new 
IllegalArgumentException("Sensor name is required");
         TypeToken<?> type = context.lookupType(sensor.type, () -> 
TypeToken.of(Object.class));
         Entity entity = sensor.entity;
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetVariableWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetVariableWorkflowStep.java
index 302d11384e..e6b8fd2f2c 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetVariableWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SetVariableWorkflowStep.java
@@ -25,6 +25,7 @@ import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.resolve.jackson.BrooklynJacksonType;
+import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
 import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.collections.CollectionMerger;
@@ -91,7 +92,7 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
     protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
         TypedValueToSet variable = context.getInput(VARIABLE);
         if (variable ==null) throw new IllegalArgumentException("Variable name 
is required");
-        String name = context.resolve(variable.name, String.class);
+        String name = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
 variable.name, String.class);
         if (Strings.isBlank(name)) throw new 
IllegalArgumentException("Variable name is required");
         TypeToken<?> type = context.lookupType(variable.type, () -> null);
 
@@ -233,9 +234,9 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
             if (result instanceof String) {
                 if (trim && result instanceof String) result = ((String) 
result).trim();
                 result = process((String) result);
-                resultCoerced = 
context.getWorkflowExectionContext().resolveCoercingOnly(result, 
typeIntermediate);
+                resultCoerced = 
context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 result, typeIntermediate);
             } else {
-                resultCoerced = wait ? context.resolveWaiting(result, 
typeIntermediate) : context.resolve(result, typeIntermediate);
+                resultCoerced = wait ? 
context.resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 result, typeIntermediate) : 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 result, typeIntermediate);
             }
             if (trim && resultCoerced instanceof String) resultCoerced = 
((String) resultCoerced).trim();
 
@@ -326,7 +327,7 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
             List<Object> objs = w.stream().map(t -> {
                 if (qst.isQuoted(t)) return qst.unwrapIfQuoted(t);
                 TypeToken<?> target = resolveToString ? 
TypeToken.of(String.class) : TypeToken.of(Object.class);
-                return wait ? context.resolveWaiting(t, target) : 
context.resolve(t, target);
+                return wait ? 
context.resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 t, target) : 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 t, target);
             }).collect(Collectors.toList());
             if (!resolveToString) return objs.get(0);
             return 
((List<String>)(List)objs).stream().collect(Collectors.joining());
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/WaitWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/WaitWorkflowStep.java
index 89095a4948..bcf74808c4 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/WaitWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/WaitWorkflowStep.java
@@ -25,23 +25,14 @@ import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.entity.EntityInternal;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
 import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
-import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.core.flags.TypeCoercions;
-import org.apache.brooklyn.util.exceptions.Exceptions;
-import org.apache.brooklyn.util.guava.Maybe;
-import org.apache.brooklyn.util.text.QuotedStringTokenizer;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.List;
-import java.util.Map;
-import java.util.function.BiFunction;
-import java.util.stream.Collectors;
-
 public class WaitWorkflowStep extends WorkflowStepDefinition {
 
     private static final Logger log = 
LoggerFactory.getLogger(WaitWorkflowStep.class);
@@ -65,7 +56,7 @@ public class WaitWorkflowStep extends WorkflowStepDefinition {
         TypeToken<?> type = TypeToken.of(Object.class);
 
         if (variable!=null) {
-            name = context.resolve(variable.name, String.class);
+            name = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
 variable.name, String.class);
             if (Strings.isBlank(name)) throw new 
IllegalArgumentException("Variable name is required");
             type = context.lookupType(variable.type, () -> 
TypeToken.of(Object.class));
         }
@@ -74,10 +65,10 @@ public class WaitWorkflowStep extends 
WorkflowStepDefinition {
 
         Stopwatch sw = Stopwatch.createStarted();
         Object unresolvedValue = input.get(VALUE.getName());
-        Object resolvedValue = context.resolveWaiting(unresolvedValue, type);
+        Object resolvedValue = 
context.resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 unresolvedValue, type);
         if (task) {
             if (resolvedValue instanceof String) {
-                resolvedValue = ((ManagementContextInternal) ((EntityInternal) 
context.getWorkflowExectionContext().getEntity())).getExecutionManager().getTask((String)
 resolvedValue);
+                resolvedValue = ((ManagementContextInternal) 
(context.getWorkflowExectionContext().getEntity())).getExecutionManager().getTask((String)
 resolvedValue);
             }
             if (resolvedValue !=null) {
                 if (resolvedValue instanceof TaskAdaptable) {
diff --git 
a/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java 
b/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java
index ea20ce10c2..6037105d03 100644
--- 
a/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java
+++ 
b/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java
@@ -38,6 +38,7 @@ import org.apache.brooklyn.core.sensor.DependentConfiguration;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.ThreadLocalStack;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.exceptions.RuntimeInterruptedException;
 import org.apache.brooklyn.util.guava.Maybe;
@@ -75,15 +76,17 @@ public class TemplateProcessor {
 
     static BrooklynFreemarkerUnwrappableObjectWrapper BROOKLYN_WRAPPER = new 
BrooklynFreemarkerUnwrappableObjectWrapper();
 
-    static ThreadLocal<Map<TemplateModel,Object>> TEMPLATE_MODEL_UNWRAP_CACHE 
= new ThreadLocal<>();
-    static ThreadLocal<String> TEMPLATE_FILE_WANTING_LEGACY_SYNTAX = new 
ThreadLocal<>();
+    static ThreadLocalStack<Map<TemplateModel,Object>> 
TEMPLATE_MODEL_UNWRAP_CACHE = new ThreadLocalStack<>(true);
+    static ThreadLocalStack<String> TEMPLATE_FILE_WANTING_LEGACY_SYNTAX = new 
ThreadLocalStack<>(true);
 
     static class BrooklynFreemarkerUnwrappableObjectWrapper extends 
BrooklynFreemarkerObjectWrapper {
 
         public Maybe<Object> unwrapMaybe(TemplateModel model) {
-            Map<TemplateModel, Object> unwrappingMap = 
TEMPLATE_MODEL_UNWRAP_CACHE.get();
-            if (unwrappingMap==null) return Maybe.absent("This thread does not 
support unwrapping");
-            if (!unwrappingMap.containsKey(model)) {
+            Maybe<Map<TemplateModel, Object>> unwrappingMapM = 
TEMPLATE_MODEL_UNWRAP_CACHE.peek();
+            if (unwrappingMapM.isAbsent()) {
+                return Maybe.absent("This thread does not support unwrapping");
+            }
+            if (!unwrappingMapM.get().containsKey(model)) {
                 // happens if we return a constant within a model
                 if (model instanceof TemplateScalarModel) {
                     try {
@@ -94,11 +97,11 @@ public class TemplateProcessor {
                 }
                 return Maybe.absent("Type and source of model is unknown: " + 
model);
             }
-            return Maybe.ofAllowingNull(unwrappingMap.get(model));
+            return Maybe.ofAllowingNull(unwrappingMapM.get().get(model));
         }
 
         public TemplateModel rememberWrapperIfSupported(Object o, 
TemplateModel m) {
-            Map<TemplateModel, Object> unwrappingMap = 
TEMPLATE_MODEL_UNWRAP_CACHE.get();
+            Map<TemplateModel, Object> unwrappingMap = 
TEMPLATE_MODEL_UNWRAP_CACHE.peek().orNull();
             if (unwrappingMap!=null) unwrappingMap.put(m, o);
             return m;
         }
@@ -482,11 +485,11 @@ public class TemplateProcessor {
 
         protected EntityAttributeTemplateModel(EntityInternal entity, 
SensorResolutionMode mode) {
             this.entity = entity;
-            if (TEMPLATE_FILE_WANTING_LEGACY_SYNTAX.get()!=null) {
+            if 
(TEMPLATE_FILE_WANTING_LEGACY_SYNTAX.peek().isPresentAndNonNull()) {
                 // in templates, we have only ever supported attribute when 
ready. preserve that for now, but warn of deprecation.
                 if (mode != SensorResolutionMode.ATTRIBUTE_WHEN_READY) {
                     log.warn("Using deprecated legacy attributeWhenReady 
behaviour of ${entity.attribute...} or ${entity.sensor...}. Template should be 
updated to use ${entity.attributeWhenReady...} if that is required: "
-                        + TEMPLATE_FILE_WANTING_LEGACY_SYNTAX.get());
+                        + TEMPLATE_FILE_WANTING_LEGACY_SYNTAX.peek());
                     mode = SensorResolutionMode.ATTRIBUTE_WHEN_READY;
                 }
             }
@@ -763,10 +766,10 @@ public class TemplateProcessor {
     @Deprecated /** since 1.1, used to warn about deprecated use of 
${entity.sensor.value} to have attribute-when-ready behaviour */
     private static Object processTemplateContentsLegacy(String context, String 
templateContents, final TemplateHashModel substitutions, boolean 
allowSingleVariableObject, boolean logErrors) {
         try {
-            TEMPLATE_FILE_WANTING_LEGACY_SYNTAX.set(context);
+            TEMPLATE_FILE_WANTING_LEGACY_SYNTAX.push(context);
             return processTemplateContents(context, templateContents, 
substitutions, allowSingleVariableObject, logErrors);
         } finally {
-            TEMPLATE_FILE_WANTING_LEGACY_SYNTAX.remove();
+            TEMPLATE_FILE_WANTING_LEGACY_SYNTAX.pop();
         }
     }
 
@@ -788,7 +791,7 @@ public class TemplateProcessor {
                 Environment env = 
template.createProcessingEnvironment(substitutions, null);
                 Maybe<Method> evalMethod = 
Reflections.findMethodMaybe(Expression.class, "eval", Environment.class);
                 try {
-                    TEMPLATE_MODEL_UNWRAP_CACHE.set(MutableMap.of());
+                    TEMPLATE_MODEL_UNWRAP_CACHE.push(MutableMap.of());
                     Maybe<Object> model = evalMethod.isAbsent() ? 
Maybe.Absent.castAbsent(evalMethod) : escapedExpression.map(expr -> {
                         try {
                             return Reflections.invokeMethodFromArgs(expr,
@@ -809,7 +812,7 @@ public class TemplateProcessor {
                         log.warn("Unable to access FreeMarker internals to 
resolve " + templateContents + "; will cast argument as string");
                     }
                 } finally {
-                    TEMPLATE_MODEL_UNWRAP_CACHE.remove();
+                    TEMPLATE_MODEL_UNWRAP_CACHE.pop();
                 }
             }
 
diff --git 
a/utils/common/src/main/java/org/apache/brooklyn/util/collections/ThreadLocalStack.java
 
b/utils/common/src/main/java/org/apache/brooklyn/util/collections/ThreadLocalStack.java
new file mode 100644
index 0000000000..d225e1770f
--- /dev/null
+++ 
b/utils/common/src/main/java/org/apache/brooklyn/util/collections/ThreadLocalStack.java
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.util.collections;
+
+import com.google.common.collect.Iterables;
+import org.apache.brooklyn.util.guava.Maybe;
+
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Objects;
+
+public class ThreadLocalStack<T> implements Iterable<T> {
+
+    private final boolean allowDuplicates;
+
+    public ThreadLocalStack(boolean allowsDuplicates) {
+        this.allowDuplicates = allowsDuplicates;
+    }
+
+    final ThreadLocal<Collection<T>> set = new ThreadLocal<>();
+
+    public Collection<T> getAll(boolean forceInitialized) {
+        Collection<T> result = set.get();
+        if (forceInitialized && result==null) {
+            result = allowDuplicates ? MutableList.of() : MutableSet.of();
+            set.set(result);
+        }
+        return result;
+    }
+
+    public T pop() {
+        Collection<T> resultS = getAll(true);
+        T last = Iterables.getLast(resultS);
+        resultS.remove(last);
+        if (resultS.isEmpty()) set.remove();
+        return last;
+    }
+
+    public boolean push(T object) {
+        return getAll(true).add(object);
+    }
+
+    @Override
+    public Iterator<T> iterator() {
+        return null;
+    }
+
+    public Maybe<T> peek() {
+        Collection<T> resultS = getAll(false);
+        if (resultS==null || resultS.isEmpty()) return Maybe.absent("Nothing 
in local stack");
+        return Maybe.of( Iterables.getLast(resultS) );
+    }
+
+    public Maybe<T> peekPenultimate() {
+        Collection<T> resultS = getAll(false);
+        if (resultS==null) return Maybe.absent();
+        int size = resultS.size();
+        if (size<=1) return Maybe.absent();
+        return Maybe.of( Iterables.get(resultS, size-2) );
+    }
+
+    public void pop(T entry) {
+        Maybe<T> popped = peek();
+        if (popped.isAbsent()) throw new IllegalStateException("Nothing to 
pop; cannot pop "+entry);
+        if (!Objects.equals(entry, popped.get())) throw new 
IllegalStateException("Stack mismatch, expected to pop "+entry+" but instead 
would have popped "+popped.get());
+        pop();
+    }
+
+    public int size() {
+        Collection<T> v = getAll(false);
+        if (v==null) return 0;
+        return v.size();
+    }
+}

Reply via email to