You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2022/11/04 17:13:43 UTC

[brooklyn-server] branch master updated (c8fe395117 -> 855f00b366)

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

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


    from c8fe395117 Merge pull request #1369 from ahgittin/workflow-tidy
     new 072a3bea4a tweak 'let' arguments esp behaviour with yaml/json
     new 5026b145da fix edge case where types won't coerce as supertypes aren't set
     new 51ea8b0cb3 add load workflow step
     new 855f00b366 Merge branch 'workflow-tidy'

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../brooklyn/camp/brooklyn/AbstractYamlTest.java   |   8 +-
 .../camp/brooklyn/AddChildrenEffectorYamlTest.java |   5 +-
 .../camp/brooklyn/CustomTypeConfigYamlTest.java    |   5 +-
 .../brooklyn/CustomTypeInitializerYamlTest.java    |  77 +++++-----
 .../brooklyn/camp/brooklyn/WorkflowYamlTest.java   |  21 ++-
 .../core/resolve/jackson/BeanWithTypeUtils.java    |   5 +-
 .../core/resolve/jackson/BrooklynJacksonType.java  |   7 +
 .../core/workflow/WorkflowExecutionContext.java    |   2 +-
 .../core/workflow/WorkflowStepResolution.java      |   2 +-
 .../core/workflow/steps/LoadWorkflowStep.java      | 105 +++++++++++++
 .../core/workflow/steps/SetSensorWorkflowStep.java |   2 +-
 .../workflow/steps/SetVariableWorkflowStep.java    | 168 +++++++++++++--------
 .../core/workflow/steps/SshWorkflowStep.java       |   1 +
 .../apache/brooklyn/util/core/ResourceUtils.java   |  11 ++
 .../resolve/jackson/BrooklynJacksonTypeTest.java   |   5 +-
 ...klynRegisteredTypeJacksonSerializationTest.java |  16 +-
 .../core/test/BrooklynAppUnitTestSupport.java      |  17 +++
 .../brooklyn/core/workflow/WorkflowBasicTest.java  |   7 +-
 .../workflow/WorkflowInputOutputExtensionTest.java | 138 ++++++++++++++---
 karaf/init/src/main/resources/catalog.bom          |   7 +-
 .../org/apache/brooklyn/util/stream/Streams.java   |  14 +-
 21 files changed, 459 insertions(+), 164 deletions(-)
 create mode 100644 core/src/main/java/org/apache/brooklyn/core/workflow/steps/LoadWorkflowStep.java


[brooklyn-server] 02/04: fix edge case where types won't coerce as supertypes aren't set

Posted by he...@apache.org.
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 5026b145da941652985114ba12dc97661b7e08c8
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Nov 4 16:16:12 2022 +0000

    fix edge case where types won't coerce as supertypes aren't set
---
 .../brooklyn/camp/brooklyn/AbstractYamlTest.java   |  8 ++-
 .../camp/brooklyn/AddChildrenEffectorYamlTest.java |  5 +-
 .../camp/brooklyn/CustomTypeConfigYamlTest.java    |  5 +-
 .../brooklyn/CustomTypeInitializerYamlTest.java    | 77 +++++++++++-----------
 .../brooklyn/camp/brooklyn/WorkflowYamlTest.java   | 17 +++--
 .../core/resolve/jackson/BrooklynJacksonType.java  |  7 ++
 .../core/workflow/WorkflowExecutionContext.java    |  2 +-
 .../core/workflow/WorkflowStepResolution.java      |  2 +-
 .../workflow/steps/SetVariableWorkflowStep.java    |  4 +-
 .../resolve/jackson/BrooklynJacksonTypeTest.java   |  5 +-
 ...klynRegisteredTypeJacksonSerializationTest.java | 16 ++---
 .../core/test/BrooklynAppUnitTestSupport.java      | 17 +++++
 .../brooklyn/core/workflow/WorkflowBasicTest.java  |  6 +-
 .../workflow/WorkflowInputOutputExtensionTest.java | 32 +++++++--
 14 files changed, 126 insertions(+), 77 deletions(-)

diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
index baeeb4a4ad..8141099ef9 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java
@@ -20,7 +20,6 @@ package org.apache.brooklyn.camp.brooklyn;
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.Reader;
 import java.util.Collection;
 import java.util.Map;
@@ -47,6 +46,7 @@ import org.apache.brooklyn.core.mgmt.EntityManagementUtils;
 import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult;
 import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
 import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests.Builder;
 import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
@@ -246,7 +246,11 @@ public abstract class AbstractYamlTest {
     protected <T extends Application> EntitySpec<T> createAppEntitySpec(String... yaml) {
         return (EntitySpec<T>) EntityManagementUtils.createEntitySpecForApplication(mgmt(), CampTypePlanTransformer.FORMAT, joinLines(yaml));
     }
-    
+
+    protected RegisteredType addBean(String name, String version, RegisteredType.TypeImplementationPlan plan) {
+        return BrooklynAppUnitTestSupport.addRegisteredTypeBean(mgmt(), name, version, plan);
+    }
+
     protected void addCatalogItems(Iterable<String> catalogYaml) {
         addCatalogItems(joinLines(catalogYaml));
     }
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AddChildrenEffectorYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AddChildrenEffectorYamlTest.java
index 476c374a8e..2bbb49f2ba 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AddChildrenEffectorYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AddChildrenEffectorYamlTest.java
@@ -215,13 +215,12 @@ public class AddChildrenEffectorYamlTest extends AbstractYamlTest {
 
     @Test
     public void testAddChildrenWithServicesBlock_RegisteredType() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(
-                RegisteredTypes.bean("add-child", "1",
+        addBean("add-child", "1",
                 new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
                         "type: "+AddChildrenEffector.class.getName()+"\n" +
                         "brooklyn.config:\n" +
                         "  name: add\n"+
-                        "  blueprint_yaml: OVERRIDE")), false);
+                        "  blueprint_yaml: OVERRIDE"));
         testAddChildrenWithServicesBlock(() ->
                 makeAppAndAddChild(Strings.lines(
                         "  - type: add-child",
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java
index ae2d3aa443..a840a74d1a 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlTest.java
@@ -216,10 +216,8 @@ public class CustomTypeConfigYamlTest extends AbstractYamlTest {
     }
 
     private void registerCustomType() {
-        RegisteredType bean = RegisteredTypes.bean("custom-type", "1",
+        addBean("custom-type", "1",
                 new BasicTypeImplementationPlan(JavaClassNameTypePlanTransformer.FORMAT, TestingCustomType.class.getName()));
-        bean = RegisteredTypes.addSuperType(bean, TestingCustomType.class);
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(bean, false);
     }
 
     @Test
@@ -336,6 +334,7 @@ public class CustomTypeConfigYamlTest extends AbstractYamlTest {
                 "      type: " + CustomTypeConfigYamlTest.TestingCustomType.class.getName(),
                 "      x: foo2",
                 "  - id: custom-type",
+                "    format: bean-with-type",
                 "    item:",
                 "      type: custom-type-0",
                 "      y: bar");
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeInitializerYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeInitializerYamlTest.java
index 4eb70c38e2..2acfac34a7 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeInitializerYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeInitializerYamlTest.java
@@ -18,11 +18,7 @@
  */
 package org.apache.brooklyn.camp.brooklyn;
 
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonCreator.Mode;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import com.fasterxml.jackson.annotation.JsonSetter;
-import com.google.common.base.Predicates;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntityInitializer;
 import org.apache.brooklyn.api.entity.EntityLocal;
@@ -31,10 +27,8 @@ import org.apache.brooklyn.core.entity.EntityInitializers.AddTags;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypePlanTransformer;
 import org.apache.brooklyn.core.resolve.jackson.BrooklynRegisteredTypeJacksonSerializationTest.SampleBean;
 import org.apache.brooklyn.core.test.entity.TestEntity;
-import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
 import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
 import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer;
-import org.apache.brooklyn.core.typereg.RegisteredTypes;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.core.config.ConfigBag;
@@ -153,8 +147,8 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCustomInitializerTypeSpecifiedAsRegisteredType() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
-                new BasicTypeImplementationPlan(JavaClassNameTypePlanTransformer.FORMAT, CustomTypeInitializerYamlTest.TestingCustomInitializerType.class.getName())), false);
+        addBean("custom-type", "1",
+                new BasicTypeImplementationPlan(JavaClassNameTypePlanTransformer.FORMAT, CustomTypeInitializerYamlTest.TestingCustomInitializerType.class.getName()));
 
         Entity testEntity = deployWithTestingCustomInitializerType("custom-type");
         assertInitializerRanAndXY(testEntity, null, null);
@@ -162,8 +156,8 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCustomInitializerTypeSpecifiedAsRegisteredTypeWithArgUsingField() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
-                new BasicTypeImplementationPlan(JavaClassNameTypePlanTransformer.FORMAT, CustomTypeInitializerYamlTest.TestingCustomInitializerType.class.getName())), false);
+        addBean("custom-type", "1",
+                new BasicTypeImplementationPlan(JavaClassNameTypePlanTransformer.FORMAT, CustomTypeInitializerYamlTest.TestingCustomInitializerType.class.getName()));
 
         Entity testEntity = deployWithTestingCustomInitializerType("custom-type",
                 "    x: foo");
@@ -172,8 +166,8 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCustomInitializerTypeSpecifiedAsRegisteredTypeWithArgUsingConfigBag() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
-                new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT, "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeAcceptingConfigBag.class.getName())), false);
+        addBean("custom-type", "1",
+                new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT, "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeAcceptingConfigBag.class.getName()));
 
         Entity testEntity = deployWithTestingCustomInitializerType("custom-type",
                 "    brooklyn.config:",
@@ -183,8 +177,8 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCustomInitializerTypeSpecifiedAsRegisteredTypeWithArgUsingConfigBagAndField() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
-                new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT, "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeAcceptingConfigBag.class.getName())), false);
+        addBean("custom-type", "1",
+                new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT, "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeAcceptingConfigBag.class.getName()));
 
         Entity testEntity = deployWithTestingCustomInitializerType("custom-type",
                 "    y: bar",
@@ -195,10 +189,10 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCustomInitializerTypeSpecifiedAsRegisteredTypeWithArgUsingInheritedFieldAndLocalConfigBag() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
+        addBean("custom-type", "1",
                 new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
                         "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeAcceptingConfigBag.class.getName()+"\n" +
-                        "y: bar")), false);
+                        "y: bar"));
 
         Entity testEntity = deployWithTestingCustomInitializerType("custom-type",
                 "    brooklyn.config:",
@@ -208,11 +202,11 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCustomInitializerTypeSpecifiedAsRegisteredTypeWithArgUsingInheritedConfigBagAndLocalField() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
+        addBean("custom-type", "1",
                 new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
                         "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeAcceptingConfigBag.class.getName()+"\n" +
                         "brooklyn.config:\n" +
-                        "  x: foo")), false);
+                        "  x: foo"));
 
         Entity testEntity = deployWithTestingCustomInitializerType("custom-type",
                 "    y: bar");
@@ -235,11 +229,13 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testOldStyleCustomInitializerTypeSpecifiedAsRegisteredTypeFails() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
-                new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
-                        "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeOldStyleAcceptingConfigBagOnly.class.getName())), false);
-
-        Asserts.assertFailsWith(() ->deployWithTestingCustomInitializerType("custom-type"),
+        Asserts.assertFailsWith(() -> {
+                    addBean("custom-type", "1",
+                            new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
+                                    "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeOldStyleAcceptingConfigBagOnly.class.getName()));
+                    deployWithTestingCustomInitializerType("custom-type");
+                    return null;
+                },
                 e -> Asserts.expectedFailureContainsIgnoreCase(e, TestingCustomInitializerTypeOldStyleAcceptingConfigBagOnly.class.getName(), "no creators", "cannot construct"));
     }
 
@@ -280,9 +276,9 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCustomInitializerFinalPatternTakingFields() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
+        addBean("custom-type", "1",
                 new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
-                        "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeFinal.class.getName())), false);
+                        "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeFinal.class.getName()));
 
         Entity entity = deployWithTestingCustomInitializerType("custom-type",
                 "    z: hi");
@@ -299,10 +295,10 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCustomInitializerFinalPatternOverridingFields() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
+        addBean("custom-type", "1",
                 new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
                         "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeFinal.class.getName()+"\n" +
-                                "z: hi1")), false);
+                                "z: hi1"));
 
         Entity entity = deployWithTestingCustomInitializerType("custom-type",
                 "    z: hi2");
@@ -311,27 +307,30 @@ public class CustomTypeInitializerYamlTest extends AbstractYamlTest {
 
     @Test
     public void testCustomInitializerFinalPatternUsingConfigBagWithTypeNotSupported() throws Exception {
-        // we're allowed to add
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("custom-type", "1",
-                new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
-                        "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeFinal.class.getName()+"\n" +
-                        "brooklyn.config:\n" +
-                        "  z: hi1")), false);
         // but deployment fails as we don't have a json setter; cannot use final var pattern with bean-with-type
-        Asserts.assertFailsWith(() -> deployWithTestingCustomInitializerType("custom-type"),
-            e -> Asserts.expectedFailureContainsIgnoreCase(e, "unrecognized field", "brooklyn.config", "hi1"));
+        Asserts.assertFailsWith(() -> {
+                    // we're allowed to add
+                    addBean("custom-type", "1",
+                            new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
+                                    "type: "+CustomTypeInitializerYamlTest.TestingCustomInitializerTypeFinal.class.getName()+"\n" +
+                                            "brooklyn.config:\n" +
+                                            "  z: hi1"));
+                    deployWithTestingCustomInitializerType("custom-type");
+                    return null;
+                },
+            e -> Asserts.expectedFailureContainsIgnoreCase(e, "unrecognized field", "brooklyn.config"));
     }
 
     // also test the stock initializer
 
     @Test
     public void testAddTags() throws Exception {
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("add-tags", "1",
+        addBean("add-tags", "1",
                 new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
-                        "type: "+ AddTags.class.getName())), false);
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(RegisteredTypes.bean("sample-bean", "1",
+                        "type: "+ AddTags.class.getName()));
+        addBean("sample-bean", "1",
                 new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT,
-                        "type: "+ SampleBean.class.getName())), false);
+                        "type: "+ SampleBean.class.getName()));
         Entity ent = deployWithTestingCustomInitializerType("add-tags",
                 "    tags:",
                 "    - t1",
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 945e472355..6025815a66 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
@@ -30,6 +30,7 @@ import org.apache.brooklyn.api.location.LocationSpec;
 import org.apache.brooklyn.api.location.MachineLocation;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.Task;
+import org.apache.brooklyn.api.objs.BrooklynObject;
 import org.apache.brooklyn.api.policy.Policy;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.api.typereg.RegisteredType;
@@ -42,6 +43,7 @@ import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
 import org.apache.brooklyn.core.entity.trait.Startable;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.sensor.Sensors;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
 import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
 import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer;
@@ -82,19 +84,16 @@ public class WorkflowYamlTest extends AbstractYamlTest {
 
     @SuppressWarnings("deprecation")
     static RegisteredType addRegisteredTypeBean(ManagementContext mgmt, String symName, Class<?> clazz) {
-        RegisteredType rt = RegisteredTypes.bean(symName, VERSION,
+        return BrooklynAppUnitTestSupport.addRegisteredTypeBean(mgmt, symName, VERSION,
                 new BasicTypeImplementationPlan(JavaClassNameTypePlanTransformer.FORMAT, clazz.getName()));
-        ((BasicBrooklynTypeRegistry)mgmt.getTypeRegistry()).addToLocalUnpersistedTypeRegistry(rt, false);
-        return rt;
     }
 
-    static RegisteredType addRegisteredTypeSpec(ManagementContext mgmt, String symName, Class<?> clazz) {
+    static RegisteredType addRegisteredTypeSpec(ManagementContext mgmt, String symName, Class<?> clazz, Class<? extends BrooklynObject> superClazz) {
         RegisteredType rt = RegisteredTypes.spec(symName, VERSION,
                 new BasicTypeImplementationPlan(JavaClassNameTypePlanTransformer.FORMAT, clazz.getName()));
-        RegisteredTypes.addSuperType(rt, Policy.class);
-
-        ((BasicBrooklynTypeRegistry)mgmt.getTypeRegistry()).addToLocalUnpersistedTypeRegistry(rt, false);
-        return rt;
+        RegisteredTypes.addSuperType(rt, superClazz);
+        mgmt.getCatalog().validateType(rt, null, false);
+        return mgmt.getTypeRegistry().get(rt.getSymbolicName(), rt.getVersion());
     }
 
     public static void addWorkflowTypes(ManagementContext mgmt) {
@@ -105,7 +104,7 @@ public class WorkflowYamlTest extends AbstractYamlTest {
 
         addRegisteredTypeBean(mgmt, "workflow-effector", WorkflowEffector.class);
         addRegisteredTypeBean(mgmt, "workflow-sensor", WorkflowSensor.class);
-        addRegisteredTypeSpec(mgmt, "workflow-policy", WorkflowPolicy.class);
+        addRegisteredTypeSpec(mgmt, "workflow-policy", WorkflowPolicy.class, Policy.class);
     }
 
     @BeforeMethod(alwaysRun = true)
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonType.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonType.java
index 02d0db717b..e184d3a2f9 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonType.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonType.java
@@ -144,6 +144,13 @@ public class BrooklynJacksonType extends SimpleType implements TypeVariable<Gene
         for (Object x : t.getSuperTypes()) {
             if (x instanceof Class) return (Class<?>) x;
         }
+
+        // we should throw; but there is one place we need to assume Object, and that is if we are
+        // adding multiple things to catalog, we assume parent type is object first even if it cannot be resolved.
+        // see CustomTypeConfigYamlTest.TestRegisteredType_InheritFromPeer
+
+        // throw new IllegalStateException("Accessing concrete type of unresolved registered type "+t);
+
         return Object.class;
     }
 
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 87d7e5f830..7c5ace49f0 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
@@ -507,7 +507,7 @@ public class WorkflowExecutionContext {
     public TypeToken<?> lookupType(String typeName, Supplier<TypeToken<?>> ifUnset) {
         if (Strings.isBlank(typeName)) return ifUnset.get();
         BrooklynClassLoadingContext loader = getEntity() != null ? RegisteredTypes.getClassLoadingContext(getEntity()) : null;
-        return new BrooklynTypeNameResolution.BrooklynTypeNameResolver("", loader, true, true).getTypeToken(typeName);
+        return new BrooklynTypeNameResolution.BrooklynTypeNameResolver("workflow", loader, true, true).getTypeToken(typeName);
     }
 
     /** as {@link #resolve(Object, TypeToken)} but without type coercion */
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java
index 24cdb4af1b..635396a793 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java
@@ -94,7 +94,7 @@ public class WorkflowStepResolution {
 
         try {
             Object def0 = defM !=null ? defM : def;
-            def = BeanWithTypeUtils.convert(mgmt, def0, TypeToken.of(WorkflowStepDefinition.class), true, loader, false);
+            def = BeanWithTypeUtils.convert(mgmt, def0, TypeToken.of(WorkflowStepDefinition.class), true, loader, true);
 
             if (def instanceof WorkflowStepDefinition.WorkflowStepDefinitionWithSpecialDeserialization) {
                 def = ((WorkflowStepDefinition.WorkflowStepDefinitionWithSpecialDeserialization)def).applySpecialDefinition(mgmt, def0, typeBestGuess, (WorkflowStepDefinition.WorkflowStepDefinitionWithSpecialDeserialization) def);
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 c43bcbe577..302d11384e 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
@@ -243,9 +243,9 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
                 try {
                     ObjectMapper mapper;
                     if ("yaml".equals(specialCoercionMode))
-                        mapper = BeanWithTypeUtils.newYamlMapper(context.getManagementContext(), false, null, true);
+                        mapper = BeanWithTypeUtils.newYamlMapper(context.getManagementContext(), true, null, true);
                     else if ("json".equals(specialCoercionMode))
-                        mapper = BeanWithTypeUtils.newMapper(context.getManagementContext(), false, null, true);
+                        mapper = BeanWithTypeUtils.newMapper(context.getManagementContext(), true, null, true);
                     else
                         throw new IllegalArgumentException("Unknown special coercion mode '" + specialCoercionMode + "'");
 
diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonTypeTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonTypeTest.java
index 74e511407d..d2f2f9de68 100644
--- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonTypeTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynJacksonTypeTest.java
@@ -52,7 +52,10 @@ public class BrooklynJacksonTypeTest {
     public static TypeToken<?> parseTestTypes(String s1) {
         return BrooklynTypeNameResolution.parseTypeToken(s1, s2 -> {
             if ("foo".equals(s2))
-                return Maybe.of(BrooklynJacksonType.asTypeToken(RegisteredTypes.bean("foo", "1", new BasicTypeImplementationPlan("x", null))));
+                return Maybe.of(BrooklynJacksonType.asTypeToken(
+                        RegisteredTypes.addSuperType(
+                            RegisteredTypes.bean("foo", "1", new BasicTypeImplementationPlan("x", null)), Object.class
+                        )));
             if ("iterable".equals(s2)) return Maybe.of(TypeToken.of(Iterable.class));
             return BrooklynTypeNameResolution.getTypeTokenForBuiltInTypeName(s2);
         }).get();
diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java
index dbfa7bd538..30ea7877af 100644
--- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java
@@ -24,6 +24,7 @@ import org.apache.brooklyn.api.entity.EntityInitializer;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.config.ConfigKeys;
 import org.apache.brooklyn.core.sensor.StaticSensor;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
 import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
 import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
@@ -76,9 +77,8 @@ public class BrooklynRegisteredTypeJacksonSerializationTest extends BrooklynMgmt
 
     @Test
     public void testDeserializeAlias() throws Exception {
-        RegisteredType rt = RegisteredTypes.bean("sample", "1",
+        BrooklynAppUnitTestSupport.addRegisteredTypeBean(mgmt,"sample", "1",
                 new BasicTypeImplementationPlan(JavaClassNameTypePlanTransformer.FORMAT, SampleBean.class.getName()));
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(rt, false);
 
         Object impl = deser("{\"type\":\"sample\",\"x\":\"hello\"}");
         Asserts.assertInstanceOf(impl, SampleBean.class);
@@ -87,13 +87,12 @@ public class BrooklynRegisteredTypeJacksonSerializationTest extends BrooklynMgmt
 
     @Test
     public void testSimpleBeanRegisteredType() throws Exception {
-        RegisteredType rt = RegisteredTypes.bean("sample-hello", "1",
+        RegisteredType rt = BrooklynAppUnitTestSupport.addRegisteredTypeBean(mgmt, "sample-hello", "1",
                 new BasicTypeImplementationPlan(BeanWithTypeUtils.FORMAT,
                         "type: " + SampleBean.class.getName() + "\n" +
-                        "x: hello\n" +
-                        "y: hi"
-        ));
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(rt, false);
+                                "x: hello\n" +
+                                "y: hi"
+                ));
         Object impl = mgmt().getTypeRegistry().create(rt, null, null);
         Asserts.assertInstanceOf(impl, SampleBean.class);
         Asserts.assertEquals(((SampleBean)impl).x, "hello");
@@ -114,11 +113,10 @@ public class BrooklynRegisteredTypeJacksonSerializationTest extends BrooklynMgmt
 
     @Test
     public void testExtendedListBeanRegisteredType() throws Exception {
-        RegisteredType rt = RegisteredTypes.bean("list-extended", "1",
+        RegisteredType rt = BrooklynAppUnitTestSupport.addRegisteredTypeBean(mgmt, "list-extended", "1",
                 new BasicTypeImplementationPlan(BeanWithTypeUtils.FORMAT,
                         "type: " + ListExtended.class.getName()
                 ));
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(rt, false);
 
         Object impl = mgmt().getTypeRegistry().create(rt, null, null);
         Asserts.assertInstanceOf(impl, ListExtended.class);
diff --git a/core/src/test/java/org/apache/brooklyn/core/test/BrooklynAppUnitTestSupport.java b/core/src/test/java/org/apache/brooklyn/core/test/BrooklynAppUnitTestSupport.java
index 3d02fe7f49..27514a2d7f 100644
--- a/core/src/test/java/org/apache/brooklyn/core/test/BrooklynAppUnitTestSupport.java
+++ b/core/src/test/java/org/apache/brooklyn/core/test/BrooklynAppUnitTestSupport.java
@@ -19,10 +19,19 @@
 package org.apache.brooklyn.core.test;
 
 import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.core.entity.BrooklynConfigKeys;
 import org.apache.brooklyn.core.test.entity.TestApplication;
+import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
+import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer;
+import org.apache.brooklyn.core.typereg.RegisteredTypes;
+import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.testng.annotations.BeforeMethod;
 
+import java.util.Collection;
+
 /**
  * To be extended by unit/integration tests.
  * <p>
@@ -52,4 +61,12 @@ public class BrooklynAppUnitTestSupport extends BrooklynMgmtUnitTestSupport {
         app = mgmt.getEntityManager().createEntity(appSpec);
     }
 
+    @SuppressWarnings("deprecation")
+    public static RegisteredType addRegisteredTypeBean(ManagementContext mgmt, String symName, String version, RegisteredType.TypeImplementationPlan plan) {
+        RegisteredType rt = RegisteredTypes.bean(symName, version, plan);
+        Collection<Throwable> errors = mgmt.getCatalog().validateType(rt, null, true);
+        if (!errors.isEmpty()) Asserts.fail(Exceptions.propagate("Failed to validate", errors));
+        // the resolved type is added, not necessarily type above which will be unresolved
+        return mgmt.getTypeRegistry().get(symName, version);
+    }
 }
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
index 182b9722ba..8de3bf29b9 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
@@ -34,6 +34,7 @@ import org.apache.brooklyn.core.entity.Dumper;
 import org.apache.brooklyn.core.entity.EntityAsserts;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 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.BasicBrooklynTypeRegistry;
 import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
@@ -60,16 +61,15 @@ import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+
 public class WorkflowBasicTest extends BrooklynMgmtUnitTestSupport {
 
     static final String VERSION = "0.1.0-SNAPSHOT";
 
     @SuppressWarnings("deprecation")
     public static RegisteredType addRegisteredTypeBean(ManagementContext mgmt, String symName, Class<?> clazz) {
-        RegisteredType rt = RegisteredTypes.bean(symName, VERSION,
+        return BrooklynAppUnitTestSupport.addRegisteredTypeBean(mgmt, symName, VERSION,
                 new BasicTypeImplementationPlan(JavaClassNameTypePlanTransformer.FORMAT, clazz.getName()));
-        ((BasicBrooklynTypeRegistry)mgmt.getTypeRegistry()).addToLocalUnpersistedTypeRegistry(rt, false);
-        return rt;
     }
 
     protected void loadTypes() {
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 bee53967f3..33ebd0242a 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
@@ -26,9 +26,10 @@ 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.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
-import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
 import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
+import org.apache.brooklyn.core.typereg.JavaClassNameTypePlanTransformer;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
 import org.apache.brooklyn.core.workflow.steps.LogWorkflowStep;
 import org.apache.brooklyn.entity.stock.BasicApplication;
@@ -38,6 +39,7 @@ 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.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.text.StringEscapes;
 import org.apache.brooklyn.util.text.Strings;
@@ -45,6 +47,7 @@ import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
@@ -118,9 +121,9 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor
     }
 
     public RegisteredType addBeanWithType(String typeName, String version, String plan) {
-        RegisteredType type = RegisteredTypes.bean(typeName, version, new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT, plan));
-        ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(type, false);
-        return type;
+        loadTypes();
+        return BrooklynAppUnitTestSupport.addRegisteredTypeBean(mgmt, typeName, version,
+                new BasicTypeImplementationPlan(BeanWithTypePlanTransformer.FORMAT, plan));
     }
 
     ClassLogWatcher lastLogWatcher;
@@ -435,6 +438,27 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor
         checkLet("json", MutableMap.of("x", 1), MutableMap.of("x", 1));
     }
 
+    public static class MockObject {
+        String id;
+        String name;
+    }
+
+    @Test
+    public void testLetYamlCoercion() throws Exception {
+        addBeanWithType("mock-object", "1", "type: " + MockObject.class.getName());
+        Object result = invokeWorkflowStepsWithLogging(MutableList.of(
+                MutableMap.<String, Object>of("step", "let x1", "value",
+                        "ignore\n" +
+                        "---\n" +
+                        "  id: x\n" +
+                        "  name: foo"),
+                "let yaml mock-object result = ${x1}",
+                "return ${result}"));
+        Asserts.assertThat(result, r -> r instanceof MockObject);
+        Asserts.assertEquals(((MockObject)result).id, "x");
+        Asserts.assertEquals(((MockObject)result).name, "foo");
+    }
+
     @Test
     public void testOutputYamlAndJson() throws Exception {
         Object output = invokeWorkflowStepsWithLogging(MutableList.of(


[brooklyn-server] 01/04: tweak 'let' arguments esp behaviour with yaml/json

Posted by he...@apache.org.
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 <al...@cloudsoft.io>
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\"]}"));
     }


[brooklyn-server] 04/04: Merge branch 'workflow-tidy'

Posted by he...@apache.org.
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 855f00b366263f9b1b1edde6481493f165acbd1e
Merge: c8fe395117 51ea8b0cb3
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Nov 4 17:13:38 2022 +0000

    Merge branch 'workflow-tidy'

 .../brooklyn/camp/brooklyn/AbstractYamlTest.java   |   8 +-
 .../camp/brooklyn/AddChildrenEffectorYamlTest.java |   5 +-
 .../camp/brooklyn/CustomTypeConfigYamlTest.java    |   5 +-
 .../brooklyn/CustomTypeInitializerYamlTest.java    |  77 +++++-----
 .../brooklyn/camp/brooklyn/WorkflowYamlTest.java   |  21 ++-
 .../core/resolve/jackson/BeanWithTypeUtils.java    |   5 +-
 .../core/resolve/jackson/BrooklynJacksonType.java  |   7 +
 .../core/workflow/WorkflowExecutionContext.java    |   2 +-
 .../core/workflow/WorkflowStepResolution.java      |   2 +-
 .../core/workflow/steps/LoadWorkflowStep.java      | 105 +++++++++++++
 .../core/workflow/steps/SetSensorWorkflowStep.java |   2 +-
 .../workflow/steps/SetVariableWorkflowStep.java    | 168 +++++++++++++--------
 .../core/workflow/steps/SshWorkflowStep.java       |   1 +
 .../apache/brooklyn/util/core/ResourceUtils.java   |  11 ++
 .../resolve/jackson/BrooklynJacksonTypeTest.java   |   5 +-
 ...klynRegisteredTypeJacksonSerializationTest.java |  16 +-
 .../core/test/BrooklynAppUnitTestSupport.java      |  17 +++
 .../brooklyn/core/workflow/WorkflowBasicTest.java  |   7 +-
 .../workflow/WorkflowInputOutputExtensionTest.java | 138 ++++++++++++++---
 karaf/init/src/main/resources/catalog.bom          |   7 +-
 .../org/apache/brooklyn/util/stream/Streams.java   |  14 +-
 21 files changed, 459 insertions(+), 164 deletions(-)


[brooklyn-server] 03/04: add load workflow step

Posted by he...@apache.org.
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 51ea8b0cb3f8c4bbaba04406e5f143352edc6e78
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Nov 4 16:28:42 2022 +0000

    add load workflow step
---
 .../core/workflow/steps/LoadWorkflowStep.java      | 105 +++++++++++++++++++++
 .../core/workflow/steps/SetSensorWorkflowStep.java |   2 +-
 .../apache/brooklyn/util/core/ResourceUtils.java   |  11 +++
 .../brooklyn/core/workflow/WorkflowBasicTest.java  |   1 +
 .../workflow/WorkflowInputOutputExtensionTest.java |   9 ++
 karaf/init/src/main/resources/catalog.bom          |   7 +-
 .../org/apache/brooklyn/util/stream/Streams.java   |  14 ++-
 7 files changed, 146 insertions(+), 3 deletions(-)

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
new file mode 100644
index 0000000000..73ccd2afb8
--- /dev/null
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/LoadWorkflowStep.java
@@ -0,0 +1,105 @@
+/*
+ * 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.core.workflow.steps;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+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;
+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.ResourceUtils;
+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;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.nio.charset.Charset;
+import java.util.*;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+public class LoadWorkflowStep extends WorkflowStepDefinition {
+
+    private static final Logger log = LoggerFactory.getLogger(LoadWorkflowStep.class);
+
+    public static final String SHORTHAND =
+            "[ \"charset\" ${charset} ] [ ${variable.type} ] ${variable.name} [ \"=\" ${url...} ]";
+
+    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");
+
+    @Override
+    public void populateFromShorthand(String expression) {
+        populateFromShorthandTemplate(SHORTHAND, expression, true);
+    }
+
+    @Override
+    public void validateStep() {
+        super.validateStep();
+        if (!input.containsKey(VARIABLE.getName())) {
+            throw new IllegalArgumentException("Variable name is required");
+        }
+        if (!input.containsKey(URL.getName())) {
+            throw new IllegalArgumentException("url is required");
+        }
+    }
+
+    @Override
+    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);
+        if (Strings.isBlank(name)) throw new IllegalArgumentException("Variable name is required");
+        TypeToken<?> type = context.lookupType(variable.type, () -> TypeToken.of(String.class));
+
+        Object url = context.getInput(URL);
+
+        ResourceUtils r = ResourceUtils.create(context.getEntity());
+
+        String csName = context.getInput(CHARSET);
+        String data;
+        if (Strings.isNonBlank(csName)) {
+            data = r.getResourceAsString("" + url, Charset.forName(csName));
+        } else {
+            data = r.getResourceAsString("" + url);
+        }
+        Object resolvedValue = context.resolve(data, type);
+
+        Object oldValue = context.getWorkflowExectionContext().getWorkflowScratchVariables().put(name, resolvedValue);
+
+        if (oldValue!=null) context.noteOtherMetadata("Previous value", oldValue);
+        return context.getPreviousStepOutput();
+    }
+}
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 f22cc4f517..36154b7f7b 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
@@ -52,8 +52,8 @@ public class SetSensorWorkflowStep extends WorkflowStepDefinition {
         String sensorName = context.resolve(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));
-        Object resolvedValue = context.getInput(VALUE.getName(), type);
         Entity entity = sensor.entity;
+        Object resolvedValue = context.getInput(VALUE.getName(), type);
         if (entity==null) entity = context.getEntity();
         AttributeSensor<Object> s = (AttributeSensor<Object>) Sensors.newSensor(type, sensorName);
         Object oldValue = entity.sensors().set( s, resolvedValue);
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/ResourceUtils.java b/core/src/main/java/org/apache/brooklyn/util/core/ResourceUtils.java
index 1191b05125..2ec9da8b6e 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/ResourceUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/ResourceUtils.java
@@ -30,6 +30,7 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLDecoder;
+import java.nio.charset.Charset;
 import java.util.List;
 import java.util.NoSuchElementException;
 import java.util.function.Supplier;
@@ -538,6 +539,16 @@ public class ResourceUtils {
         }
     }
 
+    /** takes {@link #getResourceFromUrl(String)} and reads fully, into a string */
+    public String getResourceAsString(String url, Charset charset) {
+        try {
+            return Streams.readFullyStringAndClose(getResourceFromUrl(url), charset);
+        } catch (Exception e) {
+            log.debug("ResourceUtils got error reading "+url+(context==null?"":" "+context)+" (rethrowing): "+e);
+            throw Throwables.propagate(e);
+        }
+    }
+
     /** @see #checkUrlExists(String, String) */
     public String checkUrlExists(String url) {
         return checkUrlExists(url, null);
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
index 8de3bf29b9..9788b088b4 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowBasicTest.java
@@ -85,6 +85,7 @@ public class WorkflowBasicTest extends BrooklynMgmtUnitTestSupport {
         addRegisteredTypeBean(mgmt, "set-sensor", SetSensorWorkflowStep.class);
         addRegisteredTypeBean(mgmt, "clear-sensor", ClearSensorWorkflowStep.class);
         addRegisteredTypeBean(mgmt, "let", SetVariableWorkflowStep.class);
+        addRegisteredTypeBean(mgmt, "load", LoadWorkflowStep.class);
         addRegisteredTypeBean(mgmt, "set-workflow-variable", SetVariableWorkflowStep.class);
         addRegisteredTypeBean(mgmt, "clear-workflow-variable", ClearVariableWorkflowStep.class);
         addRegisteredTypeBean(mgmt, "wait", WaitWorkflowStep.class);
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 33ebd0242a..3f335b089d 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
@@ -491,4 +491,13 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor
             "1", "3",  "4", "12"
         ));
     }
+
+    @Test
+    public void testLoadData() throws Exception {
+        Object output = invokeWorkflowStepsWithLogging(MutableList.of(
+                "load x = classpath://hello-world.txt",
+                "return ${x}"));
+        Asserts.assertStringContains((String)output, "The file hello-world.war contains its source code.");
+    }
+
 }
diff --git a/karaf/init/src/main/resources/catalog.bom b/karaf/init/src/main/resources/catalog.bom
index bd9472ea1d..b5cbfbfbdd 100644
--- a/karaf/init/src/main/resources/catalog.bom
+++ b/karaf/init/src/main/resources/catalog.bom
@@ -19,7 +19,7 @@ brooklyn.catalog:
   version: "1.1.0-SNAPSHOT" # BROOKLYN_VERSION
 
   items:
-  # provides aliases for the most common items
+  # provides aliases for the most common items
   # (used to import the other bundles but now this is done by dist feature catalog-core)
   - id: server
     iconUrl: classpath://brooklyn/icons/server.svg
@@ -129,6 +129,11 @@ brooklyn.catalog:
     itemType: bean
     item:
       type: org.apache.brooklyn.core.workflow.steps.WaitWorkflowStep
+  - id: load
+    format: java-type-name
+    itemType: bean
+    item:
+      type: org.apache.brooklyn.core.workflow.steps.LoadWorkflowStep
   - id: return
     format: java-type-name
     itemType: bean
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/stream/Streams.java b/utils/common/src/main/java/org/apache/brooklyn/util/stream/Streams.java
index a14f8b2feb..c6456f2e8c 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/stream/Streams.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/stream/Streams.java
@@ -86,6 +86,7 @@ public class Streams {
     }
     
     public static Reader reader(InputStream stream, Charset charset) {
+        if (charset==null) return new InputStreamReader(stream);
         return new InputStreamReader(stream, charset);
     }
 
@@ -128,8 +129,19 @@ public class Streams {
     }
 
     public static String readFullyStringAndClose(InputStream is) {
+        return readFullyStringAndClose(is, null);
+    }
+
+    /**
+     * Consider using {@link #readFullyStringAndClose(InputStream)} instead.
+     */
+    public static String readFullyString(InputStream is, Charset charset) {
+        return readFully(reader(is, charset));
+    }
+
+    public static String readFullyStringAndClose(InputStream is, Charset charset) {
         try {
-            return readFullyString(is);
+            return readFullyString(is, charset);
         } finally {
             Streams.closeQuietly(is);
         }