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 2b485eaf6de134164e2d6f9b3a814130043ee931 Author: Alex Heneveld <[email protected]> AuthorDate: Sat Nov 12 03:08:52 2022 +0000 clean up json/yaml parse v encoding, and add bash encoding --- .../core/workflow/steps/SetConfigWorkflowStep.java | 2 +- .../core/workflow/steps/SetSensorWorkflowStep.java | 2 +- .../workflow/steps/SetVariableWorkflowStep.java | 107 +++++++++++++---- .../workflow/WorkflowInputOutputExtensionTest.java | 131 ++++++++++++++++----- 4 files changed, 182 insertions(+), 60 deletions(-) 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 eb79cca29d..7f64079915 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 @@ -32,7 +32,7 @@ import org.apache.brooklyn.util.text.Strings; public class SetConfigWorkflowStep extends WorkflowStepDefinition { - public static final String SHORTHAND = "[ ${config.type} ] ${config.name} \"=\" ${value...}"; + public static final String SHORTHAND = "[ ${config.type} ] ${config.name} [ \"=\" ${value...} ]"; public static final ConfigKey<EntityValueToSet> CONFIG = ConfigKeys.newConfigKey(EntityValueToSet.class, "config"); public static final ConfigKey<Object> VALUE = ConfigKeys.newConfigKey(Object.class, "value"); 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 291af43542..e64e8dded7 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 @@ -36,7 +36,7 @@ public class SetSensorWorkflowStep extends WorkflowStepDefinition { private static final Logger LOG = LoggerFactory.getLogger(SetSensorWorkflowStep.class); - public static final String SHORTHAND = "[ ${sensor.type} ] ${sensor.name} \"=\" ${value...}"; + public static final String SHORTHAND = "[ ${sensor.type} ] ${sensor.name} [ \"=\" ${value...} ]"; public static final ConfigKey<EntityValueToSet> SENSOR = ConfigKeys.newConfigKey(EntityValueToSet.class, "sensor"); public static final ConfigKey<Object> VALUE = ConfigKeys.newConfigKey(Object.class, "value"); 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 540b7b133a..9c1df819b1 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 @@ -28,15 +28,13 @@ 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; -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.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.StringEscapes; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.yaml.Yamls; import org.slf4j.Logger; @@ -57,6 +55,9 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition { "[ ?${trim} \"trim \" ] " + "[ ?${yaml} \"yaml \" ] " + "[ ?${json} \"json \" ] " + + "[ ?${bash} \"bash \" ] " + + "[ ?${encode} \"encode \" ] " + + "[ ?${parse} \"parse \" ] " + "[ ?${wait} \"wait \" ] " + "[ [ ${variable.type} ] ${variable.name} [ \"=\" ${value...} ] ]"; @@ -69,6 +70,9 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition { 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> BASH = ConfigKeys.newConfigKey(Boolean.class, "bash"); + public static final ConfigKey<Boolean> ENCODE = ConfigKeys.newConfigKey(Boolean.class, "encode"); + public static final ConfigKey<Boolean> PARSE = ConfigKeys.newConfigKey(Boolean.class, "parse"); public static final ConfigKey<Boolean> WAIT = ConfigKeys.newConfigKey(Boolean.class, "wait"); @Override @@ -132,6 +136,8 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition { private final boolean wait; private final LetMergeMode merge; private String specialCoercionMode; + private boolean encode; + private final boolean parse; private final boolean typeSpecified; // private QuotedStringTokenizer qst; @@ -147,16 +153,34 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition { if (this.specialCoercionMode!=null) throw new IllegalArgumentException("Cannot specify both JSON and YAML"); this.specialCoercionMode = "json"; } + + this.encode = Boolean.TRUE.equals(context.getInput(ENCODE)); + this.parse = Boolean.TRUE.equals(context.getInput(PARSE)); + if (encode && parse) throw new IllegalArgumentException("Cannot specify both encode and parse"); + + if (Boolean.TRUE.equals(context.getInput(BASH))) { + if ("yaml".equals(this.specialCoercionMode)) throw new IllegalArgumentException("Cannot specify YAML for bash encoding"); + this.specialCoercionMode = "bash"; + this.encode = true; + if (parse) throw new IllegalArgumentException("Cannot specify `bash parse`"); + } + if (specialCoercionMode==null) { + if (encode || parse) throw new IllegalArgumentException("Cannot specify encode or parse unless JSON, YAML, or bash is specified"); + } if (specialCoercionMode!=null) { - if (this.merge != LetMergeMode.NONE) throw new IllegalArgumentException("Cannot specify both JSON and YAML"); + if (this.merge != LetMergeMode.NONE) throw new IllegalArgumentException("Cannot specify merge with JSON or YAML"); } - if (type!=null) { + if (encode) { + typeSpecified = type!=null; + if (typeSpecified && !type.getRawType().isAssignableFrom(String.class)) throw new IllegalArgumentException("Cannot encode to anything other than a string"); + this.type = (TypeToken) TypeToken.of(String.class); + } else if (type!=null) { this.type = type; - this.typeSpecified = true; + typeSpecified = true; } else { this.type = (TypeToken) TypeToken.of(Object.class); - this.typeSpecified = false; + typeSpecified = false; } } @@ -237,48 +261,79 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition { } else { 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(); + + boolean doTrim = trim; if (Strings.isNonBlank(specialCoercionMode)) { + boolean bash = "bash".equals(specialCoercionMode); try { ObjectMapper mapper; if ("yaml".equals(specialCoercionMode)) mapper = BeanWithTypeUtils.newYamlMapper(context.getManagementContext(), true, null, true); - else if ("json".equals(specialCoercionMode)) + else if (bash || "json".equals(specialCoercionMode)) mapper = BeanWithTypeUtils.newMapper(context.getManagementContext(), true, 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) { + boolean generated = false; + if (resultCoerced==null) { + // skip processing if value is null + } else if (encode && resultCoerced instanceof String) { + // skip processing when encoding a string (if it contains a yaml document separator) + } else { + if ("yaml".equals(specialCoercionMode) && resultCoerced instanceof String) { 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 (parse || (typeSpecified && !type.getRawType().isAssignableFrom(resultCoerced.getClass())) || + (!Boxing.isPrimitiveOrBoxedObject(resultCoerced) && !(resultCoerced instanceof CharSequence))) { + // in the above cases we might need to stringify it and then we should parse it + // maps and lists are included because their contents might not be json-primitive + if (!(resultCoerced instanceof String)) { + resultCoerced = mapper.writeValueAsString(resultCoerced); + generated = true; + } + if (parse || !String.class.equals(type.getRawType())) { + 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 + + doTrim |= shouldTrim(resultCoerced, generated); + + if (encode) { + if (doTrim && (resultCoerced instanceof String)) resultCoerced = ((String) resultCoerced).trim(); + if (bash) { + // currently always wraps for bash; could make it conditional + resultCoerced = StringEscapes.BashStringEscapes.wrapBash("" + resultCoerced); + } else { + resultCoerced = mapper.writeValueAsString(resultCoerced); } - if (doTrim) resultCoerced = ((String) resultCoerced).trim(); + generated = true; } + + doTrim = trim || shouldTrim(resultCoerced, generated); + } catch (JsonProcessingException e) { throw Exceptions.propagate(e); } } + if (doTrim && resultCoerced instanceof String) resultCoerced = ((String) resultCoerced).trim(); + return (T) resultCoerced; } + private boolean shouldTrim(Object resultCoerced, boolean generated) { + if (generated && resultCoerced instanceof String) { + // automatically trim in some generated cases, to make output nicer to work with + String rst = ((String) resultCoerced).trim(); + if (rst.endsWith("\"") || rst.endsWith("'")) return true; + else if ("yaml".equals(specialCoercionMode) && (rst.endsWith("+") || rst.endsWith("|"))) return false; //yaml trailing whitespace signifier + else return !Strings.isMultiLine(rst); //lastly trim if something was generated and is multiline + } + return false; + } + public void mergeInto(Object input, Function<String,TypeToken<?>> explicitType, BiConsumer<String,Object> holderAdd) { if (input instanceof String) { List<String> wordsByQuote = qst((String) input).remainderRaw(); 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 7496dc568a..26ff66f441 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 @@ -40,7 +40,6 @@ 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.exceptions.Exceptions; -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; @@ -52,7 +51,6 @@ import java.util.List; import java.util.Map; import java.util.function.BiFunction; import java.util.function.Consumer; -import java.util.function.Function; public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSupport { @@ -427,12 +425,7 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor 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), + MutableMap.<String,Object>of("step", "let x1", "value", expression), "let "+prefix+" x2 = ${x1}", "return ${x2}")); } @@ -450,26 +443,45 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor Asserts.assertEquals(out, expectedResult, "YAML encoding (for whitespace) different to expected; interesting, but not a real fault"); } + static String q(String s) { + return StringEscapes.JavaStringEscapes.wrapJavaString(s); + } + @Test 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\""); + checkLet("yaml", "2", "2"); + checkLet("yaml", q("2"), "2"); //unwrapped once by shorthand, again by step + checkLet("yaml", q(q("2")), "\"2\""); //finally we get the string + + checkLet("yaml parse", 2, 2); + checkLet("yaml parse", "2", 2); + checkLet("yaml parse", q("2"), 2); + checkLet("yaml parse", q(q("2")), "2"); + + checkLet("yaml string", 2, "2"); + checkLet("yaml string", "2", "2"); + checkLet("yaml string", q("2"), "2"); + checkLet("yaml string", q(q("2")), "\"2\""); + + checkLet("yaml encode", 2, "2"); + checkLet("yaml encode", "2", "\"2\""); + checkLet("yaml encode", q("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("yaml encode", q(q("2")), "'\"2\"'"); + + checkLetBidi("yaml encode", " ", "' '"); + checkLetBidi("yaml encode", "\n", "|2+\n\n"); // 2 is totally unnecessary but returned + checkLetBidi("yaml encode", " \n ", "\" \\n \""); // double quotes used if spaces and newlines significant + + checkLet("trim yaml", "ignore \n---\n x: 1 \n", "x: 1"); + checkLet("trim yaml parse", "ignore \n---\n x: 1 \n y: 2", MutableMap.of("x", 1, "y", 2)); + checkLet("yaml string", "ignore \n---\n x: 1 \n", " x: 1 \n"); + checkLetBidi("yaml encode", "ignore \n---\n x: 1 \n", "\"ignore \\n---\\n x: 1 \\n\""); + checkLet("trim yaml encode", "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"); @@ -480,21 +492,53 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor @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 \""); + checkLet("json", "2", "2"); + checkLet("json", q("2"), "2"); //unwrapped once by shorthand, again by step + checkLet("json", q(q("2")), "\"2\""); //finally we get the string + + checkLet("json parse", 2, 2); + checkLet("json parse", "2", 2); + checkLet("json parse", q("2"), 2); + checkLet("json parse", q(q("2")), "2"); + + checkLet("json string", 2, "2"); + checkLet("json string", "2", "2"); + checkLet("json string", q("2"), "2"); + checkLet("json string", q(q("2")), "\"2\""); + + checkLet("json encode", 2, "2"); + checkLet("json encode", "2", "\"2\""); + checkLet("json encode", q("2"), "\"2\""); + // for these especially the exact yaml is not significant, but included for interest + checkLet("json encode", q(q("2")), "\"\\\"2\\\"\""); + + checkLet("json string", " ", " "); + checkLet("json string", "\n", "\n"); + checkLet("json string", " \n ", " \n "); + + checkLetBidi("json encode", " ", "\" \""); + checkLetBidi("json encode", "\n", "\"\\n\""); + checkLetBidi("json encode", " \n ", "\" \\n \""); checkLetBidi("json string", MutableMap.of("a", 1), "{\"a\":1}"); + checkLet("json encode", MutableMap.of("a", 1), q("{\"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 + public void testLetBash() throws Exception { + checkLet("bash", 2, q("2")); + checkLet("bash encode", 2, q("2")); + checkLet("bash", "2", q("2")); + checkLet("bash", q("2"), q("2")); //unwrapped once by shorthand, again by step + checkLet("bash", q(q("2")), q("\"2\"")); //finally we get the string + + checkLet("bash", "hello(n $o!)", "\"hello(n \\$o\"'!'\")\""); + checkLet("bash", "two\nlines", "\"two\nlines\""); + checkLet("bash", MutableMap.of("x", 1), q("{\"x\":1}")); + } + public static class MockObject { String id; String name; @@ -522,9 +566,32 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor "let map x = { i: [ 1, 'A' ] }", "let map result = {}", "let yaml string result.y = ${x}", - "let json string result.j = ${x}", + "let json string result.j = ${x}", // will give the stringified map + "let json string result.j2 = ${result.j}", // no change to the string + "let json encode result.e = ${x}", + "let json map result.m0 = ${x}", + "let json map result.m1 = ${result.j}", + "return ${result}")); + String s = "{\"i\":[1,\"A\"]}"; + Object m = MutableMap.of("i", MutableList.of(1, "A")); + Asserts.assertEquals(output, MutableMap.of("y", "i:\n- 1\n- A\n", "j", s, + "j2", s, "e", q(s), + "m0", m, "m1", m)); + + String s0 = "{ i: [ 1, 'A' ] }"; + output = invokeWorkflowStepsWithLogging(MutableList.of( + "let string x = "+ q(s0), // wrap to preserve the make the second thing in list be 'A' (including single quotes) + "let map result = {}", + "let yaml parse result.mp = ${x}", // parsing a string gives a map + "let yaml parse result.mp2 = ${result.mp}", // parsing a map has no effect + "let json result.mo = ${result.mp}", // json referencing a map has no discernable effect (though it serializes and deserializes) + "let json result.so = ${x}", // json referencing a string has no effect + "let json encode result.ss0 = ${x}", // encoding a string gives the double-stringified map, you can parse to get the string back + "let json encode result.ss2 = ${result.mp}", // encoding a map gives a double-stringified result, parse it to get the stringified version + "let json string result.ss1 = ${result.mp}", // specifying string on non-string gives single encoding (again) "return ${result}")); - Asserts.assertEquals(output, MutableMap.of("y", "i:\n- 1\n- A\n", "j", "{\"i\":[1,\"A\"]}")); + Asserts.assertEquals(output, MutableMap.of("mp", m, "mp2", m, "mo", m, + "so", s0, "ss0", q(s0), "ss2", q(s), "ss1", s)); } // test complex object in an expression
