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 4daf9406a7332aa4ff0760b7fa6d5d2624200a75
Author: Alex Heneveld <[email protected]>
AuthorDate: Tue Mar 21 15:56:18 2023 +0000

    add `interpolation_mode` and `interpolation_errors` to `let` and `load`
---
 core/pom.xml                                       |   1 +
 .../workflow/WorkflowExpressionResolution.java     |  29 +++--
 .../core/workflow/WorkflowStepDefinition.java      |   5 +-
 .../WorkflowStepInstanceExecutionContext.java      |   5 +
 .../workflow/steps/variables/LoadWorkflowStep.java |   5 +-
 .../steps/variables/SetVariableWorkflowStep.java   |  54 ++++++--
 .../brooklyn/util/core/text/TemplateProcessor.java |  42 ++++++
 .../workflow/WorkflowInputOutputExtensionTest.java | 141 ++++++++++++++++++++-
 .../document-with-interpolated-expression.txt      |   1 +
 9 files changed, 257 insertions(+), 26 deletions(-)

diff --git a/core/pom.xml b/core/pom.xml
index 2eeef7af18..8cbfa5c021 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -325,6 +325,7 @@
                     <excludes combine.children="append">
                         
<exclude>**/src/test/resources/org/apache/brooklyn/util/core/osgi/test/bundlemaker/nomanifest/**</exclude>
                         
<exclude>**/src/test/resources/org/apache/brooklyn/util/core/osgi/test/bundlemaker/withmanifest/**</exclude>
+                        
<exclude>**/src/test/resources/document-with-interpolated-expression.txt</exclude>
                     </excludes>
                 </configuration>
             </plugin>
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 eede7a77c6..64cbf7d2a2 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
@@ -28,7 +28,10 @@ import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 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.*;
+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.ThreadLocalStack;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import 
org.apache.brooklyn.util.core.predicates.ResolutionFailureTreatedAsAbsent;
 import org.apache.brooklyn.util.core.task.DeferredSupplier;
@@ -43,7 +46,6 @@ import org.slf4j.LoggerFactory;
 
 import java.time.Instant;
 import java.util.*;
-import java.util.concurrent.Callable;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
@@ -69,17 +71,23 @@ public class WorkflowExpressionResolution {
     private final boolean allowWaiting;
     private final boolean useWrappedValue;
     private final WorkflowExpressionStage stage;
+    private final TemplateProcessor.InterpolationErrorMode errorMode;
 
     public WorkflowExpressionResolution(WorkflowExecutionContext context, 
WorkflowExpressionStage stage, boolean allowWaiting, boolean 
wrapExpressionValues) {
+        this(context, stage, allowWaiting, wrapExpressionValues, 
TemplateProcessor.InterpolationErrorMode.FAIL);
+    }
+    public WorkflowExpressionResolution(WorkflowExecutionContext context, 
WorkflowExpressionStage stage, boolean allowWaiting, boolean 
wrapExpressionValues, TemplateProcessor.InterpolationErrorMode errorMode) {
         this.context = context;
         this.stage = stage;
         this.allowWaiting = allowWaiting;
         this.useWrappedValue = wrapExpressionValues;
+        this.errorMode = errorMode;
     }
 
     TemplateModel ifNoMatches() {
-        // this causes the execution to fail. any other behaviour is hard with 
freemarker.
-        // recommendation is to use freemarker attempts/escapes to recover.
+        // fail here - any other behaviour is hard with freemarker (exceptions 
intercepted etc).
+        // error handling is done by 'process' method below, and by ?? 
notation handling in let,
+        // or if needed freemarker attempts/escapes to recover could be used 
(not currently used much)
         return null;
     }
 
@@ -439,21 +447,22 @@ public class WorkflowExpressionResolution {
             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("->")));
+                throw new WorkflowVariableRecursiveReference("Recursive 
reference: " + RESOLVE_STACK.getAll(false).stream().map(p -> "" + 
p.object).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 (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);
             if (expression instanceof Map) return 
processTemplateExpressionMap((Map) expression);
-            if (expression instanceof Collection) return 
processTemplateExpressionCollection((Collection) expression);
+            if (expression instanceof Collection)
+                return processTemplateExpressionCollection((Collection) 
expression);
             if (expression == null || 
Boxing.isPrimitiveOrBoxedObject(expression)) return expression;
             // otherwise resolve DSL
             return resolveDsl(expression);
 
         } finally {
-            if (entry!=null) RESOLVE_STACK.pop(entry);
+            if (entry != null) RESOLVE_STACK.pop(entry);
         }
     }
 
@@ -502,7 +511,7 @@ public class WorkflowExpressionResolution {
 
         boolean ourWait = interruptSetIfNeededToPreventWaiting();
         try {
-            result = TemplateProcessor.processTemplateContents("workflow", 
expression, model, true, false);
+            result = TemplateProcessor.processTemplateContents("workflow", 
expression, model, true, false, errorMode);
         } catch (Exception e) {
             Exception e2 = e;
             if (!allowWaiting && Exceptions.isCausedByInterruptInAnyThread(e)) 
{
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 9c192497a4..c6ea01cabe 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
@@ -144,11 +144,12 @@ public abstract class WorkflowStepDefinition {
     protected void populateFromShorthandTemplate(String template, String 
value) {
         populateFromShorthandTemplate(template, value, false, true);
     }
-    protected void populateFromShorthandTemplate(String template, String 
value, boolean finalMatchRaw, boolean failOnMismatch) {
+    protected Maybe<Map<String, Object>> populateFromShorthandTemplate(String 
template, String value, boolean finalMatchRaw, boolean failOnMismatch) {
         Maybe<Map<String, Object>> result = new 
ShorthandProcessor(template).withFinalMatchRaw(finalMatchRaw).withFailOnMismatch(failOnMismatch).process(value);
         if (result.isAbsent()) throw new IllegalArgumentException("Invalid 
shorthand expression: '"+value+"'", Maybe.Absent.getException(result));
 
-        input.putAll((Map) CollectionMerger.builder().build().merge(input, 
result.get()));
+        input.putAll((Map<? extends String, ?>) 
CollectionMerger.builder().build().merge(input, result.get()));
+        return result;
     }
 
     final Task<?> newTask(WorkflowStepInstanceExecutionContext context) {
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 ed710f0e0b..8958904944 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
@@ -111,6 +111,11 @@ public class WorkflowStepInstanceExecutionContext {
     public <T> T getInput(ConfigKey<T> key) {
         return getInput(key.getName(), key.getTypeToken());
     }
+    public <T> T getInputOrDefault(ConfigKey<T> key) {
+        T result = getInput(key.getName(), key.getTypeToken());
+        if (result==null && !input.containsKey(key.getName())) result = 
key.getDefaultValue();
+        return result;
+    }
     /** Returns the resolved value of the given key, converting to the type of 
the key if the key is known */
     public Object getInput(String key) {
         ConfigKey<?> keyTyped = ConfigUtilsInternal.findConfigKeys(getClass(), 
null).get(key);
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/LoadWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/LoadWorkflowStep.java
index 3238045ac3..1a60f5a10c 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/LoadWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/LoadWorkflowStep.java
@@ -27,6 +27,7 @@ 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.ResourceUtils;
+import org.apache.brooklyn.util.core.text.TemplateProcessor;
 import org.apache.brooklyn.util.text.ByteSizeStrings;
 import org.apache.brooklyn.util.text.Strings;
 import org.slf4j.Logger;
@@ -45,6 +46,8 @@ public class LoadWorkflowStep extends WorkflowStepDefinition {
     public static final ConfigKey<TypedValueToSet> VARIABLE = 
ConfigKeys.newConfigKey(TypedValueToSet.class, "variable");
     public static final ConfigKey<Object> URL = 
ConfigKeys.newConfigKey(Object.class, "url");
     public static final ConfigKey<String> CHARSET = 
ConfigKeys.newStringConfigKey("charset");
+    public static final ConfigKey<SetVariableWorkflowStep.InterpolationMode> 
INTERPOLATION_MODE = 
ConfigKeys.newConfigKeyWithDefault(SetVariableWorkflowStep.INTERPOLATION_MODE, 
SetVariableWorkflowStep.InterpolationMode.DISABLED);
+    public static final ConfigKey<TemplateProcessor.InterpolationErrorMode> 
INTERPOLATION_ERRORS = 
ConfigKeys.newConfigKeyWithDefault(SetVariableWorkflowStep.INTERPOLATION_ERRORS,
 TemplateProcessor.InterpolationErrorMode.IGNORE);
 
     @Override
     public void populateFromShorthand(String expression) {
@@ -83,7 +86,7 @@ public class LoadWorkflowStep extends WorkflowStepDefinition {
             data = r.getResourceAsString("" + url);
         }
 
-        Object resolvedValue = 
context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_OUTPUT,
 data, type);
+        Object resolvedValue = new 
SetVariableWorkflowStep.ConfigurableInterpolationEvaluation(context, type, 
data, context.getInputOrDefault(INTERPOLATION_MODE), 
context.getInputOrDefault(INTERPOLATION_ERRORS)).evaluate();
 
         
context.getWorkflowExectionContext().getWorkflowScratchVariables().put(name, 
resolvedValue);
 
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java
index 729a16e130..44e4969fbc 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java
@@ -28,6 +28,7 @@ import 
org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.collections.*;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
+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.text.QuotedStringTokenizer;
@@ -46,26 +47,41 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
     private static final Logger log = 
LoggerFactory.getLogger(SetVariableWorkflowStep.class);
 
     public static final String SHORTHAND =
-
             "[ [ ${variable.type} ] ${variable.name} [ \"=\" ${value...} ] ]";
 
     public static final ConfigKey<TypedValueToSet> VARIABLE = 
ConfigKeys.newConfigKey(TypedValueToSet.class, "variable");
     public static final ConfigKey<Object> VALUE = 
ConfigKeys.newConfigKey(Object.class, "value");
 
+    public enum InterpolationMode {
+        WORDS,
+        DISABLED,
+        FULL,
+    }
+    public static final ConfigKey<InterpolationMode> INTERPOLATION_MODE = 
ConfigKeys.newConfigKey(InterpolationMode.class, "interpolation_mode",
+            "Whether interpolation runs on the full value (not touching 
quotes; the default in most places), " +
+                    "on words (if unquoted, unquoting others; the default for 
'let var = value' shorthand), " +
+                    "or is disabled (not applied at all)");
+    public static final ConfigKey<TemplateProcessor.InterpolationErrorMode> 
INTERPOLATION_ERRORS = 
ConfigKeys.newConfigKey(TemplateProcessor.InterpolationErrorMode.class, 
"interpolation_errors",
+            "Whether unresolvable interpolated expressions fail and return an 
error (the default for 'let'), " +
+                    "ignore the expression leaving it in place (the default 
for 'load'), " +
+                    "or replace the expression with a blank string");
 
     @Override
     public void populateFromShorthand(String expression) {
-        populateFromShorthandTemplate(SHORTHAND, expression, true, true);
+        Maybe<Map<String, Object>> newInput = 
populateFromShorthandTemplate(SHORTHAND, expression, true, true);
+        if (newInput.isPresentAndNonNull() && 
newInput.get().get(VALUE.getName())!=null && 
input.get(INTERPOLATION_MODE.getName())==null) {
+            setInput(INTERPOLATION_MODE, InterpolationMode.WORDS);
+        }
     }
 
     @Override
     public void validateStep(@Nullable ManagementContext mgmt, @Nullable 
WorkflowExecutionContext workflow) {
         super.validateStep(mgmt, workflow);
 
-        if (!input.containsKey(VARIABLE.getName())) {
+        if (input.get(VARIABLE.getName())==null) {
             throw new IllegalArgumentException("Variable name is required");
         }
-        if (!input.containsKey(VALUE.getName())) {
+        if (input.get(VALUE.getName())==null) {
             throw new IllegalArgumentException("Value is required");
         }
     }
@@ -80,7 +96,7 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
 
         Object unresolvedValue = input.get(VALUE.getName());
 
-        Object resolvedValue = new SetVariableEvaluation(context, type, 
unresolvedValue).evaluate();
+        Object resolvedValue = new 
ConfigurableInterpolationEvaluation(context, type, unresolvedValue, 
context.getInputOrDefault(INTERPOLATION_MODE), 
context.getInputOrDefault(INTERPOLATION_ERRORS)).evaluate();
 
         Object oldValue = setWorkflowScratchVariableDotSeparated(context, 
name, resolvedValue);
         context.noteOtherMetadata("Value set", resolvedValue);
@@ -113,15 +129,22 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
 
     private enum LetMergeMode { NONE, SHALLOW, DEEP }
 
-    public static class SetVariableEvaluation<T> {
+    public static class ConfigurableInterpolationEvaluation<T> {
         protected final WorkflowStepInstanceExecutionContext context;
         protected final TypeToken<T> type;
         protected final Object unresolvedValue;
+        protected final InterpolationMode interpolationMode;
+        protected final TemplateProcessor.InterpolationErrorMode errorMode;
 
-        public SetVariableEvaluation(WorkflowStepInstanceExecutionContext 
context, TypeToken<T> type, Object unresolvedValue) {
+        public 
ConfigurableInterpolationEvaluation(WorkflowStepInstanceExecutionContext 
context, TypeToken<T> type, Object unresolvedValue) {
+            this(context, type, unresolvedValue, null, null);
+        }
+        public 
ConfigurableInterpolationEvaluation(WorkflowStepInstanceExecutionContext 
context, TypeToken<T> type, Object unresolvedValue, InterpolationMode 
interpolationMode, TemplateProcessor.InterpolationErrorMode errorMode) {
             this.context = context;
             this.unresolvedValue = unresolvedValue;
             this.type = type;
+            this.interpolationMode = interpolationMode;
+            this.errorMode = errorMode;
         }
 
         public boolean unquotedStartsWith(String s, char c) {
@@ -148,16 +171,27 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
 
             Object resultCoerced;
             TypeToken<? extends Object> typeIntermediate = type==null ? 
TypeToken.of(Object.class) : type;
-            if (result instanceof String) {
+
+            if (interpolationMode==InterpolationMode.DISABLED) {
+                resultCoerced = 
context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 result, typeIntermediate);
+
+            } else if (result instanceof String && 
interpolationMode==InterpolationMode.WORDS) {
                 result = process((String) result);
                 resultCoerced = 
context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 result, typeIntermediate);
+
             } else {
-                resultCoerced = 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 result, typeIntermediate);
+                // full, or null the default
+                resultCoerced = resolveSubPart(result, typeIntermediate);
             }
 
             return (T) resultCoerced;
         }
 
+        <T> T resolveSubPart(Object v, TypeToken<T> type) {
+            return new 
WorkflowExpressionResolution(context.getWorkflowExectionContext(), 
WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, false, 
false, errorMode)
+                    .resolveWithTemplates(v, type);
+        }
+
         Object process(String input) {
             if (Strings.isBlank(input)) return input;
 
@@ -201,7 +235,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 
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
 t, target);
+                return resolveSubPart(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/util/core/text/TemplateProcessor.java 
b/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java
index 3673db3390..511867b4f4 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
@@ -840,6 +840,9 @@ public class TemplateProcessor {
     }
 
     public static Object processTemplateContents(String context, String 
templateContents, final TemplateHashModel substitutions, boolean 
allowSingleVariableObject, boolean logErrors) {
+        return processTemplateContents(context, templateContents, 
substitutions, allowSingleVariableObject, logErrors, 
InterpolationErrorMode.FAIL);
+    }
+    public static Object processTemplateContents(String context, String 
templateContents, final TemplateHashModel substitutions, boolean 
allowSingleVariableObject, boolean logErrors, InterpolationErrorMode errorMode) 
{
         try {
             Configuration cfg = new 
Configuration(Configuration.DEFAULT_INCOMPATIBLE_IMPROVEMENTS);
             cfg.setLogTemplateExceptions(logErrors);
@@ -885,6 +888,7 @@ public class TemplateProcessor {
             // TODO could expose CAMP '$brooklyn:' style dsl, based on 
template.createProcessingEnvironment
             ByteArrayOutputStream baos = new ByteArrayOutputStream();
             Writer out = new OutputStreamWriter(baos);
+            template.setTemplateExceptionHandler(new 
ForgivingFreemarkerTemplateExceptionHandler(errorMode));
             template.process(substitutions, out);
             out.flush();
 
@@ -903,4 +907,42 @@ public class TemplateProcessor {
         }
     }
 
+    public enum InterpolationErrorMode {
+        FAIL,
+        BLANK,
+        IGNORE,
+    }
+
+    InterpolationErrorMode interpolationErrorMode;
+
+    public void setInterpolationErrorMode(InterpolationErrorMode 
interpolationErrorMode) {
+        this.interpolationErrorMode = interpolationErrorMode;
+    }
+
+    public static class ForgivingFreemarkerTemplateExceptionHandler implements 
TemplateExceptionHandler {
+        private final InterpolationErrorMode errorMode;
+        public 
ForgivingFreemarkerTemplateExceptionHandler(InterpolationErrorMode errorMode) {
+            this.errorMode = errorMode;
+        }
+        public void handleTemplateException(TemplateException te, Environment 
env, Writer out) throws TemplateException {
+            if (errorMode==null || errorMode==InterpolationErrorMode.FAIL) 
throw te;
+
+            if (errorMode==InterpolationErrorMode.BLANK) return;
+            if (errorMode==InterpolationErrorMode.IGNORE) {
+                try {
+                    // below won't work for complex expressions but those are 
discouraged anyways
+                    out.write("${" + te.getBlamedExpressionString() + "}");
+
+                    // this would work better, if we want to access private 
fields
+                    //te.getFTLInstructionStack();
+//                TemplateElement els[] = env.instructionStack;
+//                env.instructionStackSize - 1
+                } catch (IOException e) {
+                    throw Exceptions.propagate(e);
+                }
+                return;
+            }
+        }
+    }
+
 }
diff --git 
a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java
 
b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java
index ecd28d2f31..2a7b48bd3e 100644
--- 
a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java
+++ 
b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java
@@ -31,8 +31,8 @@ import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
 import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
-import org.apache.brooklyn.core.workflow.steps.flow.LogWorkflowStep;
 import org.apache.brooklyn.core.workflow.steps.appmodel.SetSensorWorkflowStep;
+import org.apache.brooklyn.core.workflow.steps.flow.LogWorkflowStep;
 import org.apache.brooklyn.entity.stock.BasicApplication;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.test.ClassLogWatcher;
@@ -48,6 +48,7 @@ import org.testng.annotations.Test;
 import java.time.Instant;
 import java.util.List;
 import java.util.Map;
+import java.util.function.BiConsumer;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -274,6 +275,138 @@ public class WorkflowInputOutputExtensionTest extends 
BrooklynMgmtUnitTestSuppor
         Asserts.assertEquals(output, 0);
     }
 
+    @Test
+    public void testLetInterpolationMode() {
+        Consumer<Map<String,Object>> invoke = input -> { try {
+            invokeWorkflowStepsWithLogging(MutableList.of("let person = Anna",
+                    input,
+                    "log NOTE: ${x}"));
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        } };
+        BiFunction<Map<String,Object>,String,String> assertLetGives = 
(input,expected) -> {
+            invoke.accept(input);
+            String note = 
Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("NOTE:")).findAny().get(), "NOTE: ");
+            Asserts.assertEquals(note, expected);
+            return note;
+        };
+
+        // disabled
+        assertLetGives.apply(MutableMap.of("step", "let x = \"${person}\"", 
"interpolation_mode", "disabled"), "\"${person}\"");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", 
"interpolation_mode", "disabled"), "${person}");
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", 
"\"${person}\"", "interpolation_mode", "disabled"), "\"${person}\"");
+
+        // words forced
+        assertLetGives.apply(MutableMap.of("step", "let x = \"${person}\"", 
"interpolation_mode", "words"), "${person}");
+        assertLetGives.apply(MutableMap.of("step", "let x = \"Anna\"", 
"interpolation_mode", "words"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", 
"interpolation_mode", "words"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", 
"\"${person}\"", "interpolation_mode", "words"), "${person}");
+
+        // full forced
+        assertLetGives.apply(MutableMap.of("step", "let x = \"${person}\"", 
"interpolation_mode", "full"), "\"Anna\"");
+        assertLetGives.apply(MutableMap.of("step", "let x = \"Anna\"", 
"interpolation_mode", "full"), "\"Anna\"");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", 
"interpolation_mode", "full"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", 
"\"${person}\"", "interpolation_mode", "full"), "\"Anna\"");
+
+        // defaults - words for shorthand
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}"), 
"Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = \"${person}\""), 
"${person}");
+        assertLetGives.apply(MutableMap.of("step", "let x = \"Anna\""), 
"Anna");
+
+        // defaults - full for separate value
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", 
"${person}"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", 
"\"${person}\""), "\"Anna\"");
+    }
+
+    @Test
+    public void testLetInterpolationErrorMode() {
+        Consumer<Map<String,Object>> invoke = input -> { try {
+            invokeWorkflowStepsWithLogging(MutableList.of("let person = Anna",
+                    input,
+                    "log NOTE: ${x}"));
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        } };
+        BiFunction<Map<String,Object>,String,String> assertLetGives = 
(input,expected) -> {
+            invoke.accept(input);
+            String note = 
Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("NOTE:")).findAny().get(), "NOTE: ");
+            Asserts.assertEquals(note, expected);
+            return note;
+        };
+
+        // does nothing if value found
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}"), 
"Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", 
"interpolation_errors", "fail"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", 
"interpolation_errors", "blank"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", 
"interpolation_errors", "ignore"), "Anna");
+
+        // but if value not found
+        Asserts.assertFailsWith(() -> 
assertLetGives.apply(MutableMap.of("step", "let x = ${unknown_person}"), 
"not-used"),
+                e -> Asserts.expectedFailureContains(e, "unknown_person"));
+        Asserts.assertFailsWith(() -> 
assertLetGives.apply(MutableMap.of("step", "let x = ${unknown_person}", 
"interpolation_errors", "fail"), "not-used"),
+            e -> Asserts.expectedFailureContains(e, "unknown_person"));
+        assertLetGives.apply(MutableMap.of("step", "let x = 
${unknown_person}", "interpolation_errors", "blank"), "");
+        assertLetGives.apply(MutableMap.of("step", "let x = 
${unknown_person}", "interpolation_errors", "ignore"), "${unknown_person}");
+    }
+
+    @Test
+    public void testLoadInterpolationMode() {
+        BiConsumer<String,Map<String,Object>> invoke = (otherVarName, input) 
-> { try {
+            invokeWorkflowStepsWithLogging(MutableList.of(
+                    "let person = Anna",
+                    "let "+otherVarName+" = Other",
+                    MutableMap.<String,Object>of("step", "load x = 
classpath://document-with-interpolated-expression.txt").add(input),
+                    "log NOTE: ${x}"));
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        } };
+        BiFunction<Map<String,Object>,String,String> 
assertLoadWithOtherVarGives = (input,expected) -> {
+            invoke.accept("other", input);
+            String note = 
Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("NOTE:")).findAny().get(), "NOTE: ");
+            Asserts.assertEquals(note, expected);
+            return note;
+        };
+        BiFunction<Map<String,Object>,String,String> 
assertLoadWithoutOtherVarGives = (input,expected) -> {
+            invoke.accept("ignored", input);
+            String note = 
Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("NOTE:")).findAny().get(), "NOTE: ");
+            Asserts.assertEquals(note, expected);
+            return note;
+        };
+
+        assertLoadWithOtherVarGives.apply(null, "This is a test. Person is 
${person} and other is ${other}.");
+        assertLoadWithOtherVarGives.apply(MutableMap.of("interpolation_mode", 
"disabled"), "This is a test. Person is ${person} and other is ${other}.");
+        assertLoadWithOtherVarGives.apply(MutableMap.of("interpolation_mode", 
"full"), "This is a test. Person is Anna and other is Other.");
+
+        assertLoadWithoutOtherVarGives.apply(null, "This is a test. Person is 
${person} and other is ${other}.");
+        
assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", 
"disabled"), "This is a test. Person is ${person} and other is ${other}.");
+        
assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", 
"full"), "This is a test. Person is Anna and other is ${other}.");
+
+
+        
assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_error", 
"fail"), "This is a test. Person is ${person} and other is ${other}.");
+        
assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", 
"disabled", "interpolation_errors", "fail"), "This is a test. Person is 
${person} and other is ${other}.");
+        
assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", 
"full"), "This is a test. Person is Anna and other is ${other}.");
+        
assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", 
"full", "interpolation_errors", "ignore"), "This is a test. Person is Anna and 
other is ${other}.");
+        
assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", 
"full", "interpolation_errors", "blank"), "This is a test. Person is Anna and 
other is .");
+        Asserts.assertFailsWith(() -> 
assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", 
"full", "interpolation_error", "fail"), "not-used"),
+                e -> Asserts.expectedFailureContains(e, "other"));
+    }
+
+    @Test
+    public void testInterpolationNotRecursiveSoDisablingWorks() throws 
Exception {
+        // outwith inputs, interpolation is not recursive. for inputs, it is 
permitted to be.
+        Object result = invokeWorkflowStepsWithLogging(MutableList.of("let 
person1 = Anna",
+                MutableMap.of("step", "let person2 = ${person1}", 
"interpolation_mode", "disabled"),
+                MutableMap.of("step", "log P2: ${person2}"),
+                MutableMap.of("step", "let person3 = ${person2}"),
+                MutableMap.of("step", "log P3: ${person3}"),
+                "return ${person3}"));
+        Asserts.assertEquals(result, "${person1}");
+        String p2 = 
Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("P2:")).findAny().get(), "P2: ");
+        Asserts.assertEquals(p2, "${person1}");
+        String p3 = 
Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("P3:")).findAny().get(), "P3: ");
+        Asserts.assertEquals(p3, "${person1}");
+    }
+
     @Test
     public void testLetQuoteVar() {
         Consumer<String> invoke = input -> { try {
@@ -362,8 +495,10 @@ public class WorkflowInputOutputExtensionTest extends 
BrooklynMgmtUnitTestSuppor
     }
 
     private Object transform(String filter, Object expression) throws 
Exception {
-        return invokeWorkflowStepsWithLogging(MutableList.of(
-                MutableMap.<String,Object>of("step", "let x1", "value", 
expression),
+        boolean inline = expression instanceof String && 
Strings.isNonEmpty((String)expression) && 
((String)expression).trim().equals(expression);
+        Map<String,Object> letStep = MutableMap.<String,Object>of("step", "let 
x1" + (inline ? " = "+expression : ""));
+        if (!inline) letStep.put("value", expression);
+        return invokeWorkflowStepsWithLogging(MutableList.of(letStep,
                 "transform x2 = ${x1} | "+filter, "return ${x2}"));
     }
 
diff --git a/core/src/test/resources/document-with-interpolated-expression.txt 
b/core/src/test/resources/document-with-interpolated-expression.txt
new file mode 100644
index 0000000000..418013d90a
--- /dev/null
+++ b/core/src/test/resources/document-with-interpolated-expression.txt
@@ -0,0 +1 @@
+This is a test. Person is ${person} and other is ${other}.
\ No newline at end of file


Reply via email to