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 ac9430fd0247759ebe190ca882f5ba32cf31cbcb Author: Alex Heneveld <[email protected]> AuthorDate: Fri Apr 14 12:45:26 2023 +0100 fix 2 places where IGNORE for interpolated expressions did not work could return partial result in one and could throw error in other; big test for scope of vars in blueprints --- .../brooklyn/camp/brooklyn/WorkflowYamlTest.java | 152 +++++++++++++++++++++ .../steps/appmodel/AddPolicyWorkflowStep.java | 6 - .../steps/variables/SetVariableWorkflowStep.java | 30 ++-- .../brooklyn/util/core/text/TemplateProcessor.java | 39 +++++- 4 files changed, 204 insertions(+), 23 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 a6cf973ad7..d6991f7b9b 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 @@ -1006,6 +1006,158 @@ public class WorkflowYamlTest extends AbstractYamlTest { Asserts.assertEquals(Iterables.getOnlyElement(entity.getChildren()).getDisplayName(), "Test"); } + @Test + public void testAddEntityWithResolvableAndBogusVarRefs() throws Exception { + /* + * care must be taken when embedding blueprints that use variables; if those variables are available from the workflow scope, + * they are resolved immediately. otherwise they are ignored. + */ + Entity app = createAndStartApplication( + "services:", + "- type: " + BasicEntity.class.getName()); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + WorkflowExecutionContext x = WorkflowBasicTest.runWorkflow(entity, Strings.lines( + "steps:", + " - let v = A", + " - set-config c = A", + " - set-config cp = A", + " - \"set-config map m = { a: 1 }\"", + " - type: add-entity", + " blueprint:", + " type: " + BasicEntity.class.getName(), + " name: Test", + " brooklyn.config:", + " ccv: ${v}", // resolved at add-entity time + " c: B", + " x: B", + " cc: ${entity.config.c}", // resolved at add-entity time + " ccc: ${entity.config.cc}", // not resolvable + " cx: ${entity.config.x}", // not resolvable at add time, remains as var + " cm: ${entity.config.m}", // a map + " brooklyn.policies:", + " -", + " type: workflow-policy", + " brooklyn.config:", + " name: Check if attached", + " triggers:", + " - s", + " steps:", + " - let ccv = ${v}", // resolved at add-entity time + " - let c = ${entity.config.c}", // resolved at add-entity time + " - let cc = ${entity.config.cc}", // resolved when this workflow runs, to parent + " - let x = ${entity.config.x}", // resolved when this workflow runs, to B + " - let cx = ${entity.config.cx}", // resolved when this workflow runs, to expression set in cx + " - let cm = ${entity.config.cm}", // resolved when this workflow runs + " - let cma = ${entity.config.cm.a}", // resolved when this workflow runs + " - let cxa = ${entity.config.cx.a} ?? unset", // resolved when this workflow runs + " - let entity1 = $brooklyn:self()", // resolved at execution time + " - step: let entity2", + " value: $brooklyn:self()", // resolved at deploy time + " - step: set-sensor result", + " value:", + " ccv: ${ccv}", + " c: ${c}", + " cc: ${cc}", + " x: ${x}", + " cx: ${cx}", + " cm: ${cm}", + " cma: ${cma}", + " cxa: ${cxa}", + " entity1: ${entity1.id}", + " entity2: ${entity2.id}", + "" + ), "test"); + x.getTask(false).get().getUnchecked(); + Entity newE = Iterables.getOnlyElement(entity.getChildren()); + newE.sensors().set(Sensors.newStringSensor("s"), "run"); + EntityAsserts.assertAttributeEventually(newE, Sensors.newSensor(Object.class, "result"), v -> v!=null); + EntityAsserts.assertAttributeEquals(newE, Sensors.newSensor(Object.class, "result"), MutableMap.of( + "ccv", "A", "c", "A", "cc", "A", "x", "B", "cx", "${entity.config.x}", "cm", MutableMap.of("a", 1)) + .add(MutableMap.of("cma", 1, "cxa", "unset", + "entity1", newE.getId(), "entity2", entity.getId()))); + } + + @Test + public void testAddPolicyWithWeirdInterpolation() throws Exception { + // this is not a nice pattern, relying on the fact that output is not a string in order to give the right output + Entity app = createAndStartApplication( + "services:", + "- type: " + BasicEntity.class.getName()); + Entity entity = Iterables.getOnlyElement(app.getChildren()); + WorkflowExecutionContext x = WorkflowBasicTest.runWorkflow(entity, Strings.lines( + "steps:", + " - type: no-op", + " output: intended", + " - type: apply-initializer", + " blueprint:", + " type: workflow-effector", + " name: foo", + " steps:", + " - return ${output}", + " - type: add-entity", + " blueprint:", + " type: "+BasicEntity.class.getName(), + " - let child = ${output.entity}", + " - type: add-policy", + " entity: ${child}", + " interpolation_mode: disabled", + " blueprint:", + " type: workflow-policy", + " triggers: [ good_interpolation ]", + " brooklyn.config:", + " steps:", + " - type: workflow", + " steps:", + " - step: invoke-effector", + " entity: $brooklyn:parent()", + " effector: foo", + " - step: set-sensor result1 = ${output}", + " - type: add-policy", + " entity: ${child}", + " blueprint:", + " type: workflow-policy", + " triggers: [ bad_interpolation_but_working_because_outer_output_is_not_a_string ]", + " brooklyn.config:", + " steps:", + " - type: workflow", + " steps:", + " - step: invoke-effector", + " entity: $brooklyn:self()", // note we refer to parent + " effector: foo", + " - step: set-sensor result2 = ${output}", + " - type: no-op", + " output: probably_unintended", + " - type: add-policy", + " entity: ${child}", + " blueprint:", + " type: workflow-policy", + " triggers: [ bad_interpolation_returning_unintended ]", + " brooklyn.config:", + " steps:", + " - type: workflow", + " steps:", + " - step: invoke-effector", + " entity: $brooklyn:self()", + " effector: foo", + " - step: set-sensor result3 = ${output}", + "" + ), "test"); + x.getTask(false).get().getUnchecked(); + Entity newE = Iterables.getOnlyElement(entity.getChildren()); + + newE.sensors().set(Sensors.newStringSensor("good_interpolation"), "run"); + EntityAsserts.assertAttributeEventually(newE, Sensors.newSensor(Object.class, "result1"), v -> v!=null); + EntityAsserts.assertAttributeEquals(newE, Sensors.newSensor(Object.class, "result1"), "intended"); + + newE.sensors().set(Sensors.newStringSensor("bad_interpolation_but_working_because_outer_output_is_not_a_string"), "run"); + EntityAsserts.assertAttributeEventually(newE, Sensors.newSensor(Object.class, "result2"), v -> v!=null); + EntityAsserts.assertAttributeEquals(newE, Sensors.newSensor(Object.class, "result2"), "intended"); + + newE.sensors().set(Sensors.newStringSensor("bad_interpolation_returning_unintended"), "run"); + EntityAsserts.assertAttributeEventually(newE, Sensors.newSensor(Object.class, "result3"), v -> v!=null); + EntityAsserts.assertAttributeEquals(newE, Sensors.newSensor(Object.class, "result3"), "probably_unintended"); + } + @Test public void testSubWorkflowOnEntities() throws Exception { Application app = createAndStartApplication( diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/AddPolicyWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/AddPolicyWorkflowStep.java index 8995389c25..546e58ccaf 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/AddPolicyWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/AddPolicyWorkflowStep.java @@ -18,10 +18,8 @@ */ package org.apache.brooklyn.core.workflow.steps.appmodel; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.google.common.collect.Iterables; import org.apache.brooklyn.api.entity.Entity; -import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.objs.BrooklynObject; @@ -31,7 +29,6 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.EntityAdjuncts; import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils; -import org.apache.brooklyn.core.resolve.jackson.JsonPassThroughDeserializer; import org.apache.brooklyn.core.workflow.WorkflowExecutionContext; import org.apache.brooklyn.core.workflow.WorkflowStepDefinition; import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext; @@ -40,7 +37,6 @@ import org.apache.brooklyn.util.core.flags.FlagUtils; 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.text.StringEscapes; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.yaml.Yamls; import org.slf4j.Logger; @@ -54,8 +50,6 @@ public class AddPolicyWorkflowStep extends WorkflowStepDefinition implements Has public static final String SHORTHAND = "[ ${type} ] [ \" at \" ${entity} ] [ \" unique-tag \" ${uniqueTag} ]"; - public static final ConfigKey<Object> BLUEPRINT = ConfigKeys.newConfigKey(Object.class, "blueprint"); - public static final ConfigKey<String> TYPE = ConfigKeys.newStringConfigKey("type"); public static final ConfigKey<String> UNIQUE_TAG = ConfigKeys.newStringConfigKey("uniqueTag"); public static final ConfigKey<Object> ENTITY = ConfigKeys.newConfigKey(Object.class, "entity"); diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java index c0f6cccc0b..15b10528ae 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java @@ -167,24 +167,28 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition { } public T evaluate() { - Object result = unresolvedValue; + try { + Object result = unresolvedValue; - Object resultCoerced; - TypeToken<? extends Object> typeIntermediate = type==null ? TypeToken.of(Object.class) : type; + Object resultCoerced; + TypeToken<? extends Object> typeIntermediate = type == null ? TypeToken.of(Object.class) : type; - if (interpolationMode==InterpolationMode.DISABLED) { - resultCoerced = context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, result, typeIntermediate); + if (interpolationMode == InterpolationMode.DISABLED) { + resultCoerced = context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, result, typeIntermediate); - } else if (result instanceof String && interpolationMode==InterpolationMode.WORDS) { - result = process((String) result); - resultCoerced = context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, result, typeIntermediate); + } else if (result instanceof String && interpolationMode == InterpolationMode.WORDS) { + result = process((String) result); + resultCoerced = context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, result, typeIntermediate); - } else { - // full, or null the default - resultCoerced = resolveSubPart(result, typeIntermediate); - } + } else { + // full, or null the default + resultCoerced = resolveSubPart(result, typeIntermediate); + } - return (T) resultCoerced; + return (T) resultCoerced; + } catch (Exception e) { + throw e; + } } <T> T resolveSubPart(Object v, TypeToken<T> type) { diff --git a/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java b/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java index 511867b4f4..c24f23c414 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java @@ -25,6 +25,8 @@ import com.google.common.io.Files; import freemarker.cache.StringTemplateLoader; import freemarker.core.Environment; import freemarker.core.Expression; +import freemarker.core.TemplateElement; +import freemarker.core._CoreAPI; import freemarker.template.*; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.entity.drivers.EntityDriver; @@ -866,6 +868,15 @@ public class TemplateProcessor { return Reflections.invokeMethodFromArgs(expr, evalMethod.get(), MutableList.of(env), true); } catch (Exception e) { + Exceptions.propagateIfFatal(e); + TemplateException te = Exceptions.getFirstThrowableOfType(e, TemplateException.class); + if (te!=null) { + try { + return new ForgivingFreemarkerTemplateExceptionHandler(errorMode).handleSingleVariableExpressionTemplate(te, templateContents); + } catch (TemplateException ex) { + throw Exceptions.propagate(ex); + } + } throw Exceptions.propagate(e); } }); @@ -926,23 +937,43 @@ public class TemplateProcessor { } public void handleTemplateException(TemplateException te, Environment env, Writer out) throws TemplateException { if (errorMode==null || errorMode==InterpolationErrorMode.FAIL) throw te; - if (errorMode==InterpolationErrorMode.BLANK) return; if (errorMode==InterpolationErrorMode.IGNORE) { try { // below won't work for complex expressions but those are discouraged anyways - out.write("${" + te.getBlamedExpressionString() + "}"); + // it also doesn't work for nested maps where an early variable is unavailable, and those _are_ supported + // eg ${entity.config.exists_but_does_not_have_key.key} -> it returns ${entity.config.exists_but_does_not_have_key} + /// out.write("${" + te.getBlamedExpressionString() + "}"); + + // this could work, but gives a string result so we need to intercept the renderer (which is a static :( ) -- or parse it to find the element we want +// te.getFTLInstructionStack(); // this would work better, if we want to access private fields - //te.getFTLInstructionStack(); // TemplateElement els[] = env.instructionStack; // env.instructionStackSize - 1 + + // aha but this allows us access! + TemplateElement[] instructions = _CoreAPI.getInstructionStackSnapshot(env); + if (instructions.length>0) { + out.write(instructions[instructions.length-1].getCanonicalForm()); + return; + } + + // can't find the instructions, so throw + throw Exceptions.propagateAnnotated("Unable to retrieve instruction so cannot ignore error in processing it", te); + } catch (IOException e) { throw Exceptions.propagate(e); } - return; } } + + public TemplateModel handleSingleVariableExpressionTemplate(TemplateException te, String templateText) throws TemplateException { + if (errorMode == InterpolationErrorMode.BLANK) return new SimpleScalar(""); + if (errorMode == InterpolationErrorMode.IGNORE) return new SimpleScalar(templateText); + //otherwise -- if (errorMode==null || errorMode==InterpolationErrorMode.FAIL) + throw te; + } } }
