From d8a4d62045008d1d63bf0c9e67b695e28ef81601 Mon Sep 17 00:00:00 2001 From: Mateusz Zan Date: Wed, 17 Jun 2026 01:46:34 +0100 Subject: [PATCH] fix: coerce list-of-custom-type effector inputs supplied as YAML Invoking an effector whose parameter is a list of a registered (TOSCA) data type failed to coerce the supplied value. Two fixes: - Parse YAML block-list strings ("- item" lines) into a list of elements for non-string element types, instead of collapsing the whole block into a single string (CommonAdaptorTypeCoercions). - Run effector parameter coercion in the entity's execution context so registered bean types can resolve their management context, instead of failing with "... without a management context" (Effectors). Co-Authored-By: Claude Opus 4.8 --- .../brooklyn/core/effector/Effectors.java | 26 ++++++++++++++++--- .../util/core/internal/TypeCoercionsTest.java | 21 +++++++++++++++ .../coerce/CommonAdaptorTypeCoercions.java | 22 +++++++++++++--- 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/effector/Effectors.java b/core/src/main/java/org/apache/brooklyn/core/effector/Effectors.java index fce5306c5b..b57e988df5 100644 --- a/core/src/main/java/org/apache/brooklyn/core/effector/Effectors.java +++ b/core/src/main/java/org/apache/brooklyn/core/effector/Effectors.java @@ -183,7 +183,7 @@ public static TaskAdaptable invocationPossiblySubWorkflow(Entity entity, if (eff2 != eff) { if (eff2 instanceof EffectorWithBody) { log.debug("Replacing invocation of {} on {} with {} which is the impl defined at that entity", new Object[] { eff, entity, eff2 }); - return ((EffectorWithBody)eff2).getBody().newTask(entity, eff2, getConfigBagWithParametersCoerced(eff2, parameters, false)); + return ((EffectorWithBody)eff2).getBody().newTask(entity, eff2, getConfigBagWithParametersCoerced(entity, eff2, parameters, false)); } else { log.warn("Effector {} defined on {} has no body; invoking caller-supplied {} instead", new Object[] { eff2, entity, eff }); } @@ -194,9 +194,9 @@ public static TaskAdaptable invocationPossiblySubWorkflow(Entity entity, if (eff instanceof EffectorWithBody) { if (eff instanceof WorkflowEffector.WorkflowEffectorAndBody) { - return (TaskAdaptable) ((WorkflowEffector.WorkflowEffectorAndBody) eff).getBody().newSubWorkflowTask(entity, eff, getConfigBagWithParametersCoerced(eff, parameters, false), parent, parentWorkflowInitializer); + return (TaskAdaptable) ((WorkflowEffector.WorkflowEffectorAndBody) eff).getBody().newSubWorkflowTask(entity, eff, getConfigBagWithParametersCoerced(entity, eff, parameters, false), parent, parentWorkflowInitializer); } else { - return ((EffectorWithBody) eff).getBody().newTask(entity, eff, getConfigBagWithParametersCoerced(eff, parameters, false)); + return ((EffectorWithBody) eff).getBody().newTask(entity, eff, getConfigBagWithParametersCoerced(entity, eff, parameters, false)); } } @@ -204,6 +204,26 @@ public static TaskAdaptable invocationPossiblySubWorkflow(Entity entity, } public static ConfigBag getConfigBagWithParametersCoerced(Effector eff, @Nullable Map map, boolean requireParameter) { + return getConfigBagWithParametersCoerced(null, eff, map, requireParameter); + } + + public static ConfigBag getConfigBagWithParametersCoerced(@Nullable Entity entity, Effector eff, @Nullable Map map, boolean requireParameter) { + // Coercing a parameter whose declared type is a registered type (e.g. a TOSCA data type) needs a + // management context, which the bean coercer obtains from the current task's context entity. When the + // effector is invoked outside the entity's task (e.g. from REST) there is no such context, and the + // coercion fails with "... without a management context". Run the coercion in the entity's execution + // context so the context entity (and hence the management context) is available. + if (entity != null && BrooklynTaskTags.getContextEntity(Tasks.current()) == null) { + return ((EntityInternal) entity).getExecutionContext().get( + Tasks.builder().dynamic(false) + .displayName("Coercing parameters for effector " + eff.getName()) + .body(() -> coerceParametersIntoConfigBag(eff, map, requireParameter)) + .build()); + } + return coerceParametersIntoConfigBag(eff, map, requireParameter); + } + + private static ConfigBag coerceParametersIntoConfigBag(Effector eff, @Nullable Map map, boolean requireParameter) { ConfigBag bag = ConfigBag.newInstance(); if (map!=null) { map.forEach( (ko,v) -> { diff --git a/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java b/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java index 65a40db56d..d321c14611 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java @@ -360,6 +360,27 @@ public void testYamlMapsDontGoTooFarWhenWantingListOfString() { assertEquals(s, ImmutableList.of("a: 1", "b : 2")); } + @SuppressWarnings("serial") + @Test + public void testYamlBlockListCoercionToListOfMaps() { + // YAML block list of maps (e.g. an effector param of type list supplied as block + // YAML) must parse to a list of maps rather than collapsing the whole block into a single element. + List s = TypeCoercions.coerce( + "- name: element1\n password: p1\n a_list:\n - sub1\n - sub2\n- name: element2\n password: p2", + new TypeToken>>() {}); + assertEquals(s, ImmutableList.of( + MutableMap.of("name", "element1", "password", "p1", "a_list", ImmutableList.of("sub1", "sub2")), + MutableMap.of("name", "element2", "password", "p2"))); + + // single block-list entry + s = TypeCoercions.coerce("- name: only\n password: p", new TypeToken>>() {}); + assertEquals(s, ImmutableList.of(MutableMap.of("name", "only", "password", "p"))); + + // flow/bracket form still works for list of maps + s = TypeCoercions.coerce("[ a: 1, b: 2 ]", new TypeToken>>() {}); + assertEquals(s, ImmutableList.of(MutableMap.of("a", 1), MutableMap.of("b", 2))); + } + @SuppressWarnings("serial") @Test public void testYamlBlockListCoercionToStringList() { diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java index 563fe125ba..2d51d7150e 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java @@ -461,9 +461,25 @@ public Maybe tryCoerce(Object input, TypeToken type) { result = JavaStringEscapes.unwrapJsonishListStringIfPossible(inputS); } } else { - // any other type, use YAMLish parse - resultM = JavaStringEscapes.tryUnwrapJsonishList(inputS); - result = (Collection) resultM.orNull(); + // for a list of non-strings: if input uses YAML block list syntax ("- item" lines), + // parse it directly with YAML, keeping maps/collections so the element coercer can + // recurse into them. Wrapping in brackets (as the jsonish parser does) kills the block + // sequence markers and collapses the whole block into a single string element. + if (inputS.trim().startsWith("- ")) { + try { + Object yamlDoc = Iterables.getOnlyElement(Yamls.parseAll(inputS)); + if (yamlDoc instanceof List) { + result = (Collection) yamlDoc; + } + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + } + } + if (result == null) { + // any other type, use YAMLish parse + resultM = JavaStringEscapes.tryUnwrapJsonishList(inputS); + result = (Collection) resultM.orNull(); + } } if (result==null) { if (resultM!=null) return Maybe.Absent.castAbsent(resultM);