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 072a3bea4a3b8bd59d8a193dbb6511c1f7963362
Author: Alex Heneveld <[email protected]>
AuthorDate: Fri Nov 4 09:51:32 2022 +0000

    tweak 'let' arguments esp behaviour with yaml/json
---
 .../brooklyn/camp/brooklyn/WorkflowYamlTest.java   |   4 +-
 .../core/resolve/jackson/BeanWithTypeUtils.java    |   5 +-
 .../workflow/steps/SetVariableWorkflowStep.java    | 168 +++++++++++++--------
 .../core/workflow/steps/SshWorkflowStep.java       |   1 +
 .../workflow/WorkflowInputOutputExtensionTest.java |  97 +++++++++---
 5 files changed, 189 insertions(+), 86 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 d5cc35c3b5..945e472355 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
@@ -206,7 +206,7 @@ public class WorkflowYamlTest extends AbstractYamlTest {
                 "            ---",
                 "            foo: bar",
                 "            v: ${v}",
-                "        - let trimmed map x = ${out}",
+                "        - let yaml map x = ${out}",
                 "        - return ${x}",
                 "");
 
@@ -259,7 +259,7 @@ public class WorkflowYamlTest extends AbstractYamlTest {
                 "            ---",
                 "            foo: bar",
                 "            v: ${v}",
-                "        - let trimmed map x = ${out}",
+                "        - let yaml map x = ${out}",
                 "        - set-sensor myWorkflowSensor = ${x}",
                 "");
 
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
 
b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
index cdc47f3561..aa26be9c1a 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
@@ -99,7 +99,10 @@ public class BeanWithTypeUtils {
     public static YAMLMapper newSimpleYamlMapper() {
         // for use with json maps (no special type resolution, even the field 
"type" is ignored);
         // do not split lines as that makes output harder to read
-        return 
YAMLMapper.builder().build().enable(YAMLGenerator.Feature.MINIMIZE_QUOTES).disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER).disable(YAMLGenerator.Feature.SPLIT_LINES);
+        return 
YAMLMapper.builder().build().enable(YAMLGenerator.Feature.MINIMIZE_QUOTES)
+                .enable(YAMLGenerator.Feature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS) 
 //otherwise "1" becomes 1
+                .disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER)
+                .disable(YAMLGenerator.Feature.SPLIT_LINES);
     }
 
     public static boolean isPureJson(Object o) {
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 b83f1767e3..c43bcbe577 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
@@ -24,6 +24,7 @@ import com.google.common.reflect.TypeToken;
 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.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.collections.CollectionMerger;
@@ -33,6 +34,7 @@ import org.apache.brooklyn.util.collections.MutableSet;
 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.javalang.Boxing;
 import org.apache.brooklyn.util.text.QuotedStringTokenizer;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.yaml.Yamls;
@@ -43,6 +45,7 @@ import java.util.*;
 import java.util.function.BiConsumer;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 public class SetVariableWorkflowStep extends WorkflowStepDefinition {
@@ -50,22 +53,24 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
     private static final Logger log = 
LoggerFactory.getLogger(SetVariableWorkflowStep.class);
 
     public static final String SHORTHAND =
-            "[ ?${trim} \"trim \" ] [ ?${trimmed} \"trimmed \" ] " +
-            "[ ?${merge} \"merge \" [ ?${merge_deep} \"deep \" ] [ ?${clean} 
\"clean \" ] ] " +
+            "[ ?${merge} \"merge \" [ ?${merge_deep} \"deep \" ] ] " +
+            "[ ?${trim} \"trim \" ] " +
+            "[ ?${yaml} \"yaml \" ] " +
+            "[ ?${json} \"json \" ] " +
             "[ ?${wait} \"wait \" ] " +
             "[ [ ${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 static final ConfigKey<Boolean> TRIM = 
ConfigKeys.newConfigKey(Boolean.class, "trim");
-    public static final ConfigKey<Boolean> TRIMMED = 
ConfigKeys.newConfigKey(Boolean.class, "trimmed");
-
-    public static final ConfigKey<Boolean> WAIT = 
ConfigKeys.newConfigKey(Boolean.class, "wait");
     public static final ConfigKey<Boolean> MERGE = 
ConfigKeys.newConfigKey(Boolean.class, "merge");
-    public static final ConfigKey<Boolean> CLEAN = 
ConfigKeys.newConfigKey(Boolean.class, "clean");
     public static final ConfigKey<Boolean> MERGE_DEEP = 
ConfigKeys.newConfigKey(Boolean.class, "merge_deep");
 
+    public static final ConfigKey<Boolean> TRIM = 
ConfigKeys.newConfigKey(Boolean.class, "trim");
+    public static final ConfigKey<Boolean> YAML = 
ConfigKeys.newConfigKey(Boolean.class, "yaml");
+    public static final ConfigKey<Boolean> JSON = 
ConfigKeys.newConfigKey(Boolean.class, "json");
+    public static final ConfigKey<Boolean> WAIT = 
ConfigKeys.newConfigKey(Boolean.class, "wait");
+
     @Override
     public void populateFromShorthand(String expression) {
         populateFromShorthandTemplate(SHORTHAND, expression, true);
@@ -88,24 +93,11 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
         if (variable ==null) throw new IllegalArgumentException("Variable name 
is required");
         String name = context.resolve(variable.name, String.class);
         if (Strings.isBlank(name)) throw new 
IllegalArgumentException("Variable name is required");
-        String specialCoercionMode = null;
-        TypeToken<?> type;
-        if ("yaml".equalsIgnoreCase(variable.type) || 
"json".equalsIgnoreCase(variable.type)) {
-            type = TypeToken.of(String.class);
-            specialCoercionMode = variable.type.toLowerCase();
-        } else {
-            type = context.lookupType(variable.type, () -> null);
-        }
+        TypeToken<?> type = context.lookupType(variable.type, () -> null);
 
         Object unresolvedValue = input.get(VALUE.getName());
 
-        Object resolvedValue = new SetVariableEvaluation(context, type==null ? 
TypeToken.of(Object.class) : type, unresolvedValue,
-                Boolean.TRUE.equals(context.getInput(TRIM)) || 
Boolean.TRUE.equals(context.getInput(TRIMMED)),
-                Boolean.TRUE.equals(context.getInput(WAIT)),
-                Boolean.TRUE.equals(context.getInput(MERGE_DEEP)) ? 
LetMergeMode.DEEP : Boolean.TRUE.equals(context.getInput(MERGE)) ? 
LetMergeMode.SHALLOW : LetMergeMode.NONE,
-                Boolean.TRUE.equals(context.getInput(CLEAN)),
-                specialCoercionMode,
-                type!=null ).evaluate();
+        Object resolvedValue = new SetVariableEvaluation(context, type, 
unresolvedValue).evaluate();
 
         Object oldValue;
 
@@ -139,22 +131,52 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
         private final boolean trim;
         private final boolean wait;
         private final LetMergeMode merge;
-        private final boolean clean;
-        private final String specialCoercionMode;
+        private String specialCoercionMode;
         private final boolean typeSpecified;
         private QuotedStringTokenizer qst;
 
 
-        public SetVariableEvaluation(WorkflowStepInstanceExecutionContext 
context, TypeToken<T> type, Object unresolvedValue, boolean trim, boolean wait, 
LetMergeMode merge, boolean clean, String specialCoercionMode, boolean 
typeSpecified) {
+        public SetVariableEvaluation(WorkflowStepInstanceExecutionContext 
context, TypeToken<T> type, Object unresolvedValue) {
             this.context = context;
-            this.type = type;
             this.unresolvedValue = unresolvedValue;
-            this.trim = trim;
-            this.wait = wait;
-            this.merge = merge;
-            this.clean = clean;
-            this.specialCoercionMode = specialCoercionMode;
-            this.typeSpecified = typeSpecified;
+            this.merge = Boolean.TRUE.equals(context.getInput(MERGE_DEEP)) ? 
LetMergeMode.DEEP : Boolean.TRUE.equals(context.getInput(MERGE)) ? 
LetMergeMode.SHALLOW : LetMergeMode.NONE;
+            this.trim = Boolean.TRUE.equals(context.getInput(TRIM));
+            this.wait = Boolean.TRUE.equals(context.getInput(WAIT));
+            if (Boolean.TRUE.equals(context.getInput(YAML))) 
this.specialCoercionMode = "yaml";
+            if (Boolean.TRUE.equals(context.getInput(JSON))) {
+                if (this.specialCoercionMode!=null) throw new 
IllegalArgumentException("Cannot specify both JSON and YAML");
+                this.specialCoercionMode = "json";
+            }
+            if (specialCoercionMode!=null) {
+                if (this.merge != LetMergeMode.NONE) throw new 
IllegalArgumentException("Cannot specify both JSON and YAML");
+            }
+
+            if (type!=null) {
+                this.type = type;
+                this.typeSpecified = true;
+            } else {
+                this.type = (TypeToken) TypeToken.of(Object.class);
+                this.typeSpecified = false;
+            }
+        }
+
+        public boolean unquotedStartsWith(String s, char c) {
+            if (s==null) return false;
+            s = s.trim();
+            s = Strings.removeFromStart(s, "\"");
+            s = Strings.removeFromStart(s, "\'");
+            s = s.trim();
+            return (s.startsWith(""+c));
+        }
+
+        public boolean trimmedEndsWithQuote(String s) {
+            if (s==null) return false;
+            s = s.trim();
+            return s.endsWith("\"") || s.endsWith("\'");
+        }
+
+        public Function<String,TypeToken<?>> ifUnquotedStartsWithThen(char c, 
Class<?> clazz) {
+            return s -> (!unquotedStartsWith(s, c)) ? null : 
TypeToken.of(clazz);
         }
 
         public T evaluate() {
@@ -164,7 +186,7 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
                 try {
                     if (Map.class.isAssignableFrom(type.getRawType())) {
                         Map holder = type.getRawType().isInterface() ? 
MutableMap.of() : (Map) type.getRawType().newInstance();
-                        mergeInto(result, (term,value) -> {
+                        mergeInto(result, ifUnquotedStartsWithThen('{', 
Map.class), (term,value) -> {
                             Map xm;
                             if (value instanceof Map) xm = (Map)value;
                             else if (value==null) {
@@ -177,7 +199,7 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
                                 xm = 
CollectionMerger.builder().build().merge(holder, xm);
                             }
                             xm.forEach((k,v) -> {
-                                if (clean && (k==null || v==null)) return;
+                                if (trim && (k==null || v==null)) return;
                                 holder.put(k, v);
                             });
                         });
@@ -185,7 +207,7 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
                     } else if 
(Collection.class.isAssignableFrom(type.getRawType())) {
                         if (merge==LetMergeMode.DEEP) throw new 
IllegalArgumentException("Merge deep only supported for map");
                         Collection holder = type.getRawType().isInterface() ? 
Set.class.isAssignableFrom(type.getRawType()) ? MutableSet.of() : 
MutableList.of() : (Collection) type.getRawType().newInstance();
-                        mergeInto(result, (term,value) -> {
+                        mergeInto(result, ifUnquotedStartsWithThen('[', 
List.class), (term,value) -> {
                             Collection xm;
                             if (value instanceof Collection) xm = 
(Collection)value;
                             else if (value==null) {
@@ -194,12 +216,12 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
                             } else {
                                 xm = MutableList.of(value);
                             }
-                            if (clean) xm = (List) xm.stream().filter(i -> 
i!=null).collect(Collectors.toList());
+                            if (trim) xm = (List) xm.stream().filter(i -> 
i!=null).collect(Collectors.toList());
                             holder.addAll(xm);
                         });
                         return (T) holder;
                     } else {
-                        throw new IllegalArgumentException("Type 'map' or 
'list' must be specified for 'let merge'");
+                        throw new IllegalArgumentException("Type 'map' or 
'list' or 'set' must be specified for 'let merge'");
                     }
                 } catch (Exception e) {
                     throw Exceptions.propagate(e);
@@ -209,32 +231,46 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
             Object resultCoerced;
             TypeToken<? extends Object> typeIntermediate = 
Strings.isNonBlank(specialCoercionMode) ? TypeToken.of(Object.class) : type;
             if (result instanceof String) {
+                if (trim && result instanceof String) result = ((String) 
result).trim();
                 result = process((String) result);
-                if (trim && result instanceof String) {
-                    if (typeSpecified) {
-                        result = 
Yamls.lastDocumentFunction().apply((String)result);
-                    } else {
-                        result = ((String) result).trim();
-                    }
-                }
                 resultCoerced = 
context.getWorkflowExectionContext().resolveCoercingOnly(result, 
typeIntermediate);
-
             } else {
                 resultCoerced = wait ? context.resolveWaiting(result, 
typeIntermediate) : context.resolve(result, typeIntermediate);
             }
+            if (trim && resultCoerced instanceof String) resultCoerced = 
((String) resultCoerced).trim();
+
             if (Strings.isNonBlank(specialCoercionMode)) {
-                // convert result to yaml or json string
-                ObjectMapper mapper;
-                if ("yaml".equals(specialCoercionMode)) mapper = 
BeanWithTypeUtils.newYamlMapper(context.getManagementContext(), false, null, 
true);
-                else if ("json".equals(specialCoercionMode)) mapper = 
BeanWithTypeUtils.newMapper(context.getManagementContext(), false, null, true);
-                else throw new IllegalArgumentException("Unknown special 
coercion mode '"+specialCoercionMode+"'");
                 try {
-                    resultCoerced = mapper.writeValueAsString(resultCoerced);
-                    // remvoe doc start marker (now no longer included from 
jackson normally but maybe from others)
-                    resultCoerced = 
Strings.removeFromStart((String)resultCoerced, "---");
-                    resultCoerced = Strings.trimStart((String)resultCoerced);
-                    // for convenience don't make multiline unless we have to
-                    if 
(!Strings.isMultiLine(Strings.trim((String)resultCoerced))) resultCoerced = 
Strings.trim((String)resultCoerced);
+                    ObjectMapper mapper;
+                    if ("yaml".equals(specialCoercionMode))
+                        mapper = 
BeanWithTypeUtils.newYamlMapper(context.getManagementContext(), false, null, 
true);
+                    else if ("json".equals(specialCoercionMode))
+                        mapper = 
BeanWithTypeUtils.newMapper(context.getManagementContext(), false, null, true);
+                    else
+                        throw new IllegalArgumentException("Unknown special 
coercion mode '" + specialCoercionMode + "'");
+
+                    if (typeSpecified || 
!Boxing.isPrimitiveOrBoxedObject(resultCoerced)) {
+                        boolean wantsStringifiedOutput = typeSpecified && 
String.class.equals(type.getRawType());
+                        if ("yaml".equals(specialCoercionMode) && 
resultCoerced instanceof String && !wantsStringifiedOutput) {
+                            resultCoerced = 
Yamls.lastDocumentFunction().apply((String) resultCoerced);
+                        }
+                        if (!(resultCoerced instanceof String) || 
wantsStringifiedOutput) {
+                            resultCoerced = 
mapper.writeValueAsString(resultCoerced);
+                        }
+                        if (!wantsStringifiedOutput) {
+                            resultCoerced = mapper.readValue((String) 
resultCoerced, BrooklynJacksonType.asTypeReference(type));
+                        }
+                    }
+                    if (resultCoerced instanceof String) {
+                        boolean doTrim = trim;
+                        if (!doTrim) {
+                            String rst = ((String) resultCoerced).trim();
+                            if (rst.endsWith("\"") || rst.endsWith("'")) 
doTrim = true;
+                            else if (rst.endsWith("+") || rst.endsWith("|")) 
doTrim = false;  //yaml trailing whitespace signifier
+                            else doTrim = !Strings.isMultiLine(rst);     
//lastly trim if not multiline
+                        }
+                        if (doTrim) resultCoerced = ((String) 
resultCoerced).trim();
+                    }
                 } catch (JsonProcessingException e) {
                     throw Exceptions.propagate(e);
                 }
@@ -243,13 +279,13 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
             return (T) resultCoerced;
         }
 
-        public void mergeInto(Object input, BiConsumer<String,Object> 
holderAdd) {
+        public void mergeInto(Object input, Function<String,TypeToken<?>> 
explicitType, BiConsumer<String,Object> holderAdd) {
             if (input instanceof String) {
                 qst = 
QuotedStringTokenizer.builder().includeQuotes(true).includeDelimiters(true).keepInternalQuotes(true).failOnOpenQuote(false).build((String)
 input);
-                List<String> wordsByQuote = qst.remainderAsList();
+                List<String> wordsByQuote = qst.remainderRaw();
                 wordsByQuote.forEach(word -> {
                     if (Strings.isBlank(word)) return;
-                    Maybe<Object> wordResolved = 
processMaybe(MutableList.of(word));
+                    Maybe<Object> wordResolved = 
processMaybe(MutableList.of(word), explicitType);
                     if (trim && wordResolved.isAbsent()) return;
                     holderAdd.accept(word, wordResolved.get());
                 });
@@ -265,10 +301,10 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
             qst = 
QuotedStringTokenizer.builder().includeQuotes(true).includeDelimiters(true).keepInternalQuotes(true).failOnOpenQuote(false).build(input);
             List<String> wordsByQuote = qst.remainderAsList();
             // then look for operators etc
-            return process(wordsByQuote);
+            return process(wordsByQuote, null);
         }
 
-        Object process(List<String> w) {
+        Object process(List<String> w, Function<String,TypeToken<?>> 
explicitType) {
             // if no tokens, treat as null
             if (w.isEmpty()) return null;
 
@@ -317,12 +353,12 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
         }
 
         Object handleNullish(List<String> lhs, List<String> rhs) {
-            return processMaybe(lhs).or(() -> process(rhs));
+            return processMaybe(lhs, null).or(() -> process(rhs, null));
         }
 
-        Maybe<Object> processMaybe(List<String> lhs) {
+        Maybe<Object> processMaybe(List<String> lhs, 
Function<String,TypeToken<?>> explicitType) {
             try {
-                Object result = process(lhs);
+                Object result = process(lhs, explicitType);
                 if (result!=null) return Maybe.of(result);
                 return Maybe.absent("null");
 
@@ -357,8 +393,8 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
         }
 
         Object applyMathOperator(String op, List<String> lhs0, List<String> 
rhs0, BiFunction<Integer,Integer,Number> ifInt, 
BiFunction<Double,Double,Number> ifDouble) {
-            Object lhs = process(lhs0);
-            Object rhs = process(rhs0);
+            Object lhs = process(lhs0, null);
+            Object rhs = process(rhs0, null);
 
             Maybe<Integer> lhsI = asInteger(lhs);
             Maybe<Integer> rhsI = asInteger(rhs);
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
index d5c7c8a56a..5f4bab3fe1 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/SshWorkflowStep.java
@@ -45,6 +45,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
 
     public static final ConfigKey<String> ENDPOINT = 
ConfigKeys.newStringConfigKey("endpoint");
     public static final ConfigKey<String> COMMAND = 
ConfigKeys.newStringConfigKey("command");
+    //TODO public static final ConfigKey<String> COMMAND_URL = 
ConfigKeys.newStringConfigKey("command_url");
     public static final ConfigKey<Map<String,Object>> ENV = new 
MapConfigKey.Builder(Object.class, "env").build();
     public static final ConfigKey<DslPredicates.DslPredicate<Integer>> 
EXIT_CODE = ConfigKeys.newConfigKey(new 
TypeToken<DslPredicates.DslPredicate<Integer>>() {}, "exit_code");
     public static final ConfigKey<Integer> OUTPUT_MAX_SIZE = 
ConfigKeys.newIntegerConfigKey("output_max_size", "Maximum size for stdout and 
stderr, or -1 for no limit", 100000);
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 4b8c532373..bee53967f3 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
@@ -24,6 +24,7 @@ import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.entity.EntityAsserts;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypePlanTransformer;
+import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
 import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
@@ -37,6 +38,8 @@ 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.core.config.ConfigBag;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.text.StringEscapes;
 import org.apache.brooklyn.util.text.Strings;
 import org.testng.Assert;
 import org.testng.annotations.Test;
@@ -296,7 +299,7 @@ public class WorkflowInputOutputExtensionTest extends 
BrooklynMgmtUnitTestSuppor
     public void testLetTrimMergeAllowsNull() throws Exception {
         Object output = invokeWorkflowStepsWithLogging(MutableList.of(
                 "let list a = [ 1 ]",
-                "let trim merge list x = ${a} ${b}",
+                "let merge trim list x = ${a} ${b}",
                 "return ${x}"));
         Asserts.assertEquals(output, MutableList.of(1));
     }
@@ -305,7 +308,7 @@ public class WorkflowInputOutputExtensionTest extends 
BrooklynMgmtUnitTestSuppor
     public void testLetMergeCleanRemovesNull() throws Exception {
         Object output = invokeWorkflowStepsWithLogging(MutableList.of(
                 MutableMap.of("step", "let a", "value", MutableList.of(2, 
null)),
-                "let merge clean list x = ${a}",
+                "let merge trim list x = ${a}",
                 "return ${x}"));
         Asserts.assertEquals(output, MutableList.of(2));
     }
@@ -350,26 +353,86 @@ public class WorkflowInputOutputExtensionTest extends 
BrooklynMgmtUnitTestSuppor
         Object output = invokeWorkflowStepsWithLogging(MutableList.of(
                 "let person_spaces = \" Anna \"",
                 "let string person = ${person_spaces}",
-                "let trimmed person_tidied = ${person_spaces}",
+                "let trim person_tidied = ${person_spaces}",
                 "log PERSON: ${person}",
                 "log PERSON_TIDIED: ${person_tidied}"));
         Asserts.assertEquals(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("PERSON:")).findAny().get(), "PERSON:  Anna ");
         Asserts.assertEquals(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("PERSON_TIDIED:")).findAny().get(), "PERSON_TIDIED: Anna");
     }
 
+    private void assertLogMessageFor(String prefix, String value) {
+        Asserts.assertEquals(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith(prefix+": ")).findAny().get(),
+                prefix+": "+value);
+    }
+
+    private Object let(String prefix, Object expression) throws Exception {
+        return invokeWorkflowStepsWithLogging(MutableList.of(
+                MutableMap.<String,Object>of("step", "let x1", "value",
+                        expression instanceof String
+                                ? 
StringEscapes.JavaStringEscapes.wrapJavaString((String)expression)
+                                : expression instanceof Maybe
+                                ? ((Maybe)expression).get()
+                                : expression),
+                "let "+prefix+" x2 = ${x1}", "return ${x2}"));
+    }
+
+    private void checkLet(String prefix, Object expression, Object 
expectedResult) throws Exception {
+        Object out = let(prefix, expression);
+        Asserts.assertEquals(out, expectedResult);
+    }
+
+    private void checkLetBidi(String prefix, Object expression, String 
expectedResult) throws Exception {
+        Object out = let(prefix, expression);
+
+        Object backIn = BeanWithTypeUtils.newYamlMapper(null, false, null, 
false).readValue((String) out, Object.class);
+        Asserts.assertEquals(backIn, expression);
+
+        Asserts.assertEquals(out, expectedResult, "YAML encoding (for 
whitespace) different to expected; interesting, but not a real fault");
+    }
+
     @Test
-    public void testLetTrimYaml() throws Exception {
-        Object output = invokeWorkflowStepsWithLogging(MutableList.of(
-                "let person_yaml = \"ignore\n---\n name: Anna \"",
-                "let trimmed string person_s = ${person_yaml}",
-                "let trimmed person_s2 = ${person_s}",
-                "let trimmed map person_m = ${person_yaml}",
-                "log PERSON_S: ${person_s}",
-                "log PERSON_S2: ${person_s2}",
-                "return ${person_m}"));
-        Asserts.assertEquals(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("PERSON_S:")).findAny().get(), "PERSON_S:  name: Anna ");
-        Asserts.assertEquals(lastLogWatcher.getMessages().stream().filter(s -> 
s.startsWith("PERSON_S2:")).findAny().get(), "PERSON_S2: name: Anna");
-        Asserts.assertEquals(output, MutableMap.of("name", "Anna"));
+    public void testLetYaml() throws Exception {
+        // null not permitted as return type; would be nice to support but not 
vital
+//        checkLetBidi("yaml string", null, "null");
+
+        checkLet("yaml", 2, 2);
+        checkLet("yaml", Maybe.of("2"), 2);
+        checkLet("yaml", "2", 2);  //wrapped by our test, unwrapped once by 
shorthand, again by step
+        checkLet("yaml", "\"2\"", "2");  //wrapped by our test, unwrapped once 
by shorthand, again by step
+        checkLetBidi("yaml string", 2, "2");
+        checkLetBidi("yaml string", "2", "\"2\"");
+        // for these especially the exact yaml is not significant, but 
included for interest
+        checkLetBidi("yaml string", "\"2\"", "'\"2\"'");
+        checkLetBidi("yaml string", " ", "' '");
+        checkLetBidi("yaml string", "\n", "|2+\n\n");  // 2 is totally 
unnecessary but returned
+        checkLetBidi("yaml string", " \n ", "\" \\n \"");   // double quotes 
used if spaces and newlines significant
+
+        checkLet("trim yaml", "ignore \n---\n x: 1 \n", MutableMap.of("x", 1));
+        checkLetBidi("yaml string", "ignore \n---\n x: 1 \n", "\"ignore 
\\n---\\n x: 1 \\n\"");
+        checkLet("trim yaml string", "ignore \n---\n x: 1 \n", "\"ignore 
\\n---\\n x: 1\"");
+        checkLet("trim string", "ignore \n---\n x: 1 \n", "ignore \n---\n x: 
1");
+
+        checkLetBidi("yaml string", MutableMap.of("a", 1), "a: 1");
+        checkLetBidi("yaml string", MutableMap.of("a", MutableList.of(null), 
"x", "1"), "a:\n- null\nx: \"1\"");
+        checkLet("yaml", MutableMap.of("x", 1), MutableMap.of("x", 1));
+    }
+
+    @Test
+    public void testLetJson() throws Exception {
+        checkLet("json", 2, 2);
+        checkLet("json", Maybe.of("2"), 2);
+        checkLet("json", "2", 2);
+        checkLet("json", "\"2\"", "2");
+        checkLetBidi("json string", 2, "2");
+        checkLetBidi("json string", "2", "\"2\"");
+        checkLetBidi("json string", "\"2\"", "\"\\\"2\\\"\"");
+        checkLetBidi("json string", " ", "\" \"");
+        checkLetBidi("json string", "\n", "\"\\n\"");
+        checkLetBidi("json string", " \n ", "\" \\n \"");
+
+        checkLetBidi("json string", MutableMap.of("a", 1), "{\"a\":1}");
+        checkLetBidi("json string", MutableMap.of("a", MutableList.of(null), 
"x", "1"), "{\"a\":[null],\"x\":\"1\"}");
+        checkLet("json", MutableMap.of("x", 1), MutableMap.of("x", 1));
     }
 
     @Test
@@ -377,8 +440,8 @@ public class WorkflowInputOutputExtensionTest extends 
BrooklynMgmtUnitTestSuppor
         Object output = invokeWorkflowStepsWithLogging(MutableList.of(
                 "let map x = { i: [ 1, 'A' ] }",
                 "let map result = {}",
-                "let yaml result.y = ${x}",
-                "let json result.j = ${x}",
+                "let yaml string result.y = ${x}",
+                "let json string result.j = ${x}",
                 "return ${result}"));
         Asserts.assertEquals(output, MutableMap.of("y", "i:\n- 1\n- A\n", "j", 
"{\"i\":[1,\"A\"]}"));
     }

Reply via email to