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 050f79317d3b2b243ffe54c5adf9e4878f435455 Author: Alex Heneveld <[email protected]> AuthorDate: Mon Jun 19 17:48:05 2023 +0100 let DslPredicate use wrapped values so we can inject supplied values into this workflow uses this so that conditions can be parsed once and processed subsequently. if too messy we might scratch that, always do late resolution. primarily an issue for foreach and custom where the condition needs to be evaluated after scratch variables are inserted. (that could be handled another way which might be simpler.) but the abilities introduced here, for conditions to be more dynamic are useful. however they currently store the context which is not good. --- .../brooklyn/CustomTypeConfigYamlRebindTest.java | 5 +- .../brooklyn/spi/dsl/DslPredicateYamlTest.java | 4 +- .../core/resolve/jackson/BeanWithTypeUtils.java | 24 +++- .../jackson/ObjectReferencingSerialization.java | 7 +- .../core/workflow/WorkflowExecutionContext.java | 58 ++++++---- .../workflow/WorkflowExpressionResolution.java | 125 ++++++++++++++++----- .../core/workflow/WorkflowStepDefinition.java | 2 +- .../WorkflowStepInstanceExecutionContext.java | 4 +- .../core/workflow/steps/CustomWorkflowStep.java | 4 +- .../steps/appmodel/SetSensorWorkflowStep.java | 3 +- .../workflow/steps/external/HttpWorkflowStep.java | 2 +- .../workflow/steps/external/SshWorkflowStep.java | 2 +- .../steps/variables/SetVariableWorkflowStep.java | 2 +- .../util/core/predicates/DslPredicates.java | 112 +++++++++++------- .../WorkflowNestedAndCustomExtensionTest.java | 16 +++ .../util/core/predicates/DslPredicateTest.java | 5 +- .../tasks/kubectl/ContainerWorkflowStep.java | 2 +- .../brooklyn/location/winrm/WinrmWorkflowStep.java | 2 +- .../brooklyn/util/javalang/coerce/TryCoercer.java | 3 +- 19 files changed, 270 insertions(+), 112 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlRebindTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlRebindTest.java index ea464d641e..433d80abc5 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlRebindTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlRebindTest.java @@ -142,7 +142,10 @@ public class CustomTypeConfigYamlRebindTest extends AbstractYamlRebindTest { () -> child.config().set((ConfigKey) EntityWithCustomTypeConfig.CUSTOM_TYPE_KEY.subKey("i2"), MutableMap.of("x", DslUtils.parseBrooklynDsl(mgmt(), "$brooklyn:attributeWhenReady(\"set-later\")"))), - e -> Asserts.expectedFailureContains(e, "Cannot deserialize value", "String", "from Object")); + e -> Asserts.expectedFailureContains(e, + //"Cannot deserialize value", "String", "from Object" <- deeply returns this + "Cannot coerce or set", "x=$brooklyn:attributeWhenReady(\"set-later\")", "customTypeKey.i2" + )); // but is allowed at map level (is additive) child.config().set((ConfigKey) EntityWithCustomTypeConfig.CUSTOM_TYPE_KEY, diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java index 0059b22005..f23709d80c 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java @@ -93,12 +93,12 @@ public class DslPredicateYamlTest extends AbstractYamlTest { // this is simpler and more efficient, although it might be surprising app.config().set(ConfigKeys.newStringConfigKey("expected"), "y"); Asserts.assertFalse( predicate.apply(app) ); - Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)predicate).equals, "x" ); + Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)predicate).equals.get(), "x" ); // per above, if we re-retrieve the predicate it should work fine predicate = app.config().get(TestEntity.CONF_PREDICATE); Asserts.assertTrue( predicate.apply(app) ); - Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)predicate).equals, "y" ); + Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)predicate).equals.get(), "y" ); } static class PredicateAndSpec { 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 ff5902eee1..a920995cac 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 @@ -169,11 +169,28 @@ public class BeanWithTypeUtils { try { stack.push(mapOrListToSerializeThenDeserialize); - return convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes); + // prefer this because (a) it's cheaper, and (b) it supports deferred values more nicely; + // ObjectReferencingSerialization.deserializeWrapper will do type coercion so very few things if any should need deep coercion now + T result = convertShallow(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes); + +// T result2 = null; +// try { +// result2 = convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes); +// } catch (Exception e2) { +// Exceptions.propagateIfFatal(e2); +// // otherwise ignore +// } +// if (!Objects.equals(result, result2)) { +// // legacy preferred convert deeply; in a few places this mattered. +// // need to investigate when/why +// return result2; +// } + + return result; } catch (Exception e) { try { - return convertShallow(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes); + return convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes); } catch (Exception e2) { throw Exceptions.propagate(Arrays.asList(e, e2)); } @@ -186,7 +203,8 @@ public class BeanWithTypeUtils { @Beta public static <T> T convertShallow(ManagementContext mgmt, Object mapOrListToSerializeThenDeserialize, TypeToken<T> type, boolean allowRegisteredTypes, BrooklynClassLoadingContext loader, boolean allowJavaTypes) throws JsonProcessingException { - // try with complex types are saved as objects rather than serialized, but won't work if special deserialization is wanted to apply to a map inside a complex type + // try with complex types are saved as objects rather than serialized; might not work if special deserialization is wanted to apply to a map inside a complex type, + // but type coercions might mean that it does actually work but doing a nested convert shallow ObjectMapper mapper = YAMLMapper.builder().build(); mapper = BeanWithTypeUtils.applyCommonMapperConfig(mapper, mgmt, allowRegisteredTypes, loader, allowJavaTypes); mapper = new ObjectReferencingSerialization().useAndApplytoMapper(mapper); diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/ObjectReferencingSerialization.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/ObjectReferencingSerialization.java index e703f3a868..9d3af1c773 100644 --- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/ObjectReferencingSerialization.java +++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/ObjectReferencingSerialization.java @@ -178,7 +178,12 @@ public class ObjectReferencingSerialization { // and if the expected type is overly strict, return the original object. // if the token buffer is used too much we might have lost the alias reference and still end up with a string, // but so long as this deserializer is preferred which it normally is, losing the alias reference is okay. - return ((Maybe)TypeCoercions.tryCoerce(result, TypeToken.of(expected))).or(result); + Maybe resultCoerced = ((Maybe) TypeCoercions.tryCoerce(result, TypeToken.of(expected))); + if (resultCoerced.isAbsent()) { + // not uncommon when we are trying to deserialize in a few different ways, or if we are using a string deserializer because the json input is a string +// if (LOG.isDebugEnabled()) LOG.debug("Reference to "+result+" when deserialization could not be coerced to expected type "+expected+"; proceeding but might cause errors"); + } + return resultCoerced.or(result); } else { LOG.debug("Odd - what looks like a reference was received but not found: "+v); } 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 015d9e8e10..33c7722a50 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 @@ -18,7 +18,10 @@ */ package org.apache.brooklyn.core.workflow; -import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; import com.fasterxml.jackson.databind.JavaType; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.type.TypeFactory; @@ -137,7 +140,7 @@ public class WorkflowExecutionContext { @JsonDeserialize(contentUsing = JsonPassThroughDeserializer.class) List<Object> stepsDefinition; - DslPredicates.DslPredicate condition; + Object condition; @JsonInclude(JsonInclude.Include.NON_EMPTY) Map<String,Object> input = MutableMap.of(); @@ -307,7 +310,7 @@ public class WorkflowExecutionContext { WorkflowStepResolution.resolveSubSteps(w.getManagementContext(), "error handling", WorkflowErrorHandling.wrappedInListIfNecessaryOrNullIfEmpty(w.onError)); // some fields need to be resolved at setting time, in the context of the workflow - w.setCondition(w.resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage.WORKFLOW_STARTING_POST_INPUT, paramsDefiningWorkflow.getStringKey(WorkflowCommonConfig.CONDITION.getName()), WorkflowCommonConfig.CONDITION.getTypeToken())); + w.setCondition(paramsDefiningWorkflow.getStringKey(WorkflowCommonConfig.CONDITION.getName())); // finished -- checkpoint noting this has been created but not yet started w.updateStatus(WorkflowStatus.STAGED); @@ -392,7 +395,7 @@ public class WorkflowExecutionContext { return stepsWithExplicitId; } - public void setCondition(DslPredicates.DslPredicate condition) { + public void setCondition(Object condition) { this.condition = condition; } @@ -442,23 +445,29 @@ public class WorkflowExecutionContext { return retryRecords; } - @JsonIgnore - public Object getConditionTarget() { - if (getWorkflowScratchVariables()!=null) { - Object v = getWorkflowScratchVariables().get("target"); - // should we also set the entity? otherwise it will take from the task. but that should only apply - // in a task where the context entity is set, so for now rely on that. - if (v!=null) return v; + public Maybe<Task<Object>> getTask(boolean checkCondition) { + if (checkCondition) return getTaskCheckingConditionWithTarget(getEntityOrAdjunctWhereRunning()); + else return getTaskSkippingCondition(); + } + DslPredicates.DslPredicate resolveCondition(Object condition) { + if (condition==null) return null; + // condition is resolved wrapped for two reasons: + // - target cannot be a fully resolved string unless it is something like 'children', and that constant should be + // different to a var ${x} (even if x evaluates to children) + // - some tests allow things to throw errors and check for error, so e.g. an expression that doesn't resolve isn't necessarily a problem + return resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, condition, TypeToken.of(DslPredicates.DslPredicate.class), + WorkflowExpressionResolution.WrappingMode.WRAPPED_RESULT_DEFER_THROWING_ERROR_BUT_NO_RETRY); + } + public Maybe<Task<Object>> getTaskCheckingConditionWithTarget(Object conditionTarget) { + DslPredicates.DslPredicate conditionResolved = resolveCondition(condition); + if (conditionResolved != null) { + if (!conditionResolved.apply(conditionTarget)) + return Maybe.absent(new IllegalStateException("This workflow cannot be run at present: condition not satisfied")); } - return getEntityOrAdjunctWhereRunning(); + return getTaskSkippingCondition(); } - @JsonIgnore - public Maybe<Task<Object>> getTask(boolean checkCondition) { - if (checkCondition && condition!=null) { - if (!condition.apply(getConditionTarget())) return Maybe.absent(new IllegalStateException("This workflow cannot be run at present: condition not satisfied")); - } - + public Maybe<Task<Object>> getTaskSkippingCondition() { if (task==null) { if (taskId!=null) { task = (Task<Object>) getManagementContext().getExecutionManager().getTask(taskId); @@ -672,22 +681,23 @@ public class WorkflowExecutionContext { /** resolution of ${interpolation} and $brooklyn:dsl and deferred suppliers, followed by type coercion. * if the type is a string, null is not permitted, otherwise it is. */ public <T> T resolve(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) { - return new WorkflowExpressionResolution(this, stage, false, false).resolveWithTemplates(expression, type); + return new WorkflowExpressionResolution(this, stage, false, WorkflowExpressionResolution.WrappingMode.NONE).resolveWithTemplates(expression, type); } public <T> T resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) { - return new WorkflowExpressionResolution(this, stage, false, false).resolveCoercingOnly(expression, type); + return new WorkflowExpressionResolution(this, stage, false, WorkflowExpressionResolution.WrappingMode.NONE).resolveCoercingOnly(expression, type); } - /** as {@link #resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, TypeToken)}, but returning DSL/supplier for values (so the indication of their dynamic nature is preserved, even if the value returned by it is resolved; + /** as {@link #resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, TypeToken)}, + * but returning DSL/supplier for values (so the indication of their dynamic nature is preserved, even if the value returned by it is resolved; * this is needed e.g. for conditions which treat dynamic expressions differently to explicit values) */ - public <T> T resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) { - return new WorkflowExpressionResolution(this, stage, false, true).resolveWithTemplates(expression, type); + public <T> T resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type, WorkflowExpressionResolution.WrappingMode wrappingMode) { + return new WorkflowExpressionResolution(this, stage, false, wrappingMode).resolveWithTemplates(expression, type); } /** as {@link #resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, TypeToken)}, but waiting on any expressions which aren't ready */ public <T> T resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) { - return new WorkflowExpressionResolution(this, stage, true, false).resolveWithTemplates(expression, type); + return new WorkflowExpressionResolution(this, stage, true, WorkflowExpressionResolution.WrappingMode.NONE).resolveWithTemplates(expression, type); } /** resolution of ${interpolation} and $brooklyn:dsl and deferred suppliers, followed by type coercion */ diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java index b3e6c0589f..5c207c8b6c 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java @@ -18,6 +18,7 @@ */ package org.apache.brooklyn.core.workflow; +import com.google.common.annotations.Beta; import com.google.common.reflect.TypeToken; import freemarker.template.TemplateHashModel; import freemarker.template.TemplateModel; @@ -70,18 +71,55 @@ public class WorkflowExpressionResolution { private static final Logger log = LoggerFactory.getLogger(WorkflowExpressionResolution.class); private final WorkflowExecutionContext context; private final boolean allowWaiting; - private final boolean useWrappedValue; private final WorkflowExpressionStage stage; private final TemplateProcessor.InterpolationErrorMode errorMode; + private final WrappingMode wrappingMode; + + public static class WrappingMode { + public final boolean wrapResolvedStrings; + public final boolean deferThrowingError; + public final boolean deferAndRetryErroneousExpressions; + public final boolean deferBrooklynDsl; + public final boolean deferInterpolation; + + protected WrappingMode(boolean wrapResolvedStrings, boolean deferThrowingError, boolean deferAndRetryErroneousExpressions, boolean deferBrooklynDsl, boolean deferInterpolation) { + this.wrapResolvedStrings = wrapResolvedStrings; + this.deferThrowingError = deferThrowingError; + this.deferAndRetryErroneousExpressions = deferAndRetryErroneousExpressions; + this.deferBrooklynDsl = deferBrooklynDsl; + this.deferInterpolation = deferInterpolation; + } + + /** do not re-evaluate anything, but if there is an error don't throw it until accessed; useful for conditions that should be evaluated immediately */ + public final static WrappingMode WRAPPED_RESULT_DEFER_THROWING_ERROR_BUT_NO_RETRY = new WrappingMode(true, true, false, false, false); + + /** no wrapping; everything evaluated immediately, errors thrown immediately */ + public final static WrappingMode NONE = new WrappingMode(false, false, false, false, false); + + /** this was the old default when wrapping was requested, but was an odd one - wraps error throwing and DSL resolution but not interpolation */ + @Deprecated @Beta // might re-introduce but for now needs to cache workflow context so discouraged + final static WrappingMode OLD_DEFAULT_DEFER_THROWING_ERROR_AND_DSL = new WrappingMode(true, true, false, true, false); + /** allow subsequent re-evaluation for things that are not recognized, but evaluate everything else now; cf InterpolationErrorMode.IGNORE */ + @Deprecated @Beta // might re-introduce but for now needs to cache workflow context so discouraged + public final static WrappingMode DEFER_RETRY_ON_ERROR_ONLY = new WrappingMode(false, false, true, false, false); + /** defer the evaluation of all vars (but evaluate now so if string is static it can be returned as a static) */ + @Deprecated @Beta // might re-introduce but for now needs to cache workflow context so discouraged + public final static WrappingMode ALL_NON_STATIC = new WrappingMode(true /* no effect here */, true /* no effect here */, true, true, true); + + public WrappingMode wrappingModeWhenResolving() { + // this works for our current use cases, which is conditions; other uses might want it not to throw something deferred however + return WRAPPED_RESULT_DEFER_THROWING_ERROR_BUT_NO_RETRY; + } + } - public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, boolean wrapExpressionValues) { + public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, WrappingMode wrapExpressionValues) { this(context, stage, allowWaiting, wrapExpressionValues, TemplateProcessor.InterpolationErrorMode.FAIL); } - public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, boolean wrapExpressionValues, TemplateProcessor.InterpolationErrorMode errorMode) { + public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, WrappingMode wrapExpressionValues, TemplateProcessor.InterpolationErrorMode errorMode) { this.context = context; this.stage = stage; this.allowWaiting = allowWaiting; - this.useWrappedValue = wrapExpressionValues; + this.wrappingMode = wrapExpressionValues == null ? WrappingMode.NONE : wrapExpressionValues; this.errorMode = errorMode; } @@ -120,7 +158,7 @@ public class WorkflowExpressionResolution { return ifNoMatches(); } - Object candidate; + Object candidate = null; if (stage.after(WorkflowExpressionStage.STEP_PRE_INPUT)) { //somevar -> workflow.current_step.output.somevar @@ -134,7 +172,9 @@ public class WorkflowExpressionResolution { //somevar -> workflow.current_step.input.somevar try { - candidate = currentStep.getInput(key, Object.class); + if (currentStep!=null) { + candidate = currentStep.getInput(key, Object.class); + } } catch (Throwable t) { Exceptions.propagateIfFatal(t); if (stage==WorkflowExpressionStage.STEP_INPUT && WorkflowVariableResolutionStackEntry.isStackForSettingVariable(RESOLVE_STACK.getAll(true), key) && Exceptions.getFirstThrowableOfType(t, WorkflowVariableRecursiveReference.class)!=null) { @@ -458,11 +498,11 @@ public class WorkflowExpressionResolution { public static class AllowBrooklynDslMode { public static AllowBrooklynDslMode ALL = new AllowBrooklynDslMode(true, null); - static { ALL.next = () -> ALL; } + static { ALL.next = Maybe.of(ALL); } public static AllowBrooklynDslMode NONE = new AllowBrooklynDslMode(false, null); - static { NONE.next = () -> NONE; } - public static AllowBrooklynDslMode CHILDREN_BUT_NOT_HERE = new AllowBrooklynDslMode(false, ()->ALL); - //public static AllowBrooklynDslMode HERE_BUT_NOT_CHILDREN = new AllowBrooklynDslMode(true, ()->NONE); + static { NONE.next = Maybe.of(NONE); } + public static AllowBrooklynDslMode CHILDREN_BUT_NOT_HERE = new AllowBrooklynDslMode(false, Maybe.of(ALL)); + //public static AllowBrooklynDslMode HERE_BUT_NOT_CHILDREN = new AllowBrooklynDslMode(true, Maybe.of(NONE)); private Supplier<AllowBrooklynDslMode> next; private boolean allowedHere; @@ -530,22 +570,15 @@ public class WorkflowExpressionResolution { public Object processTemplateExpressionString(String expression, AllowBrooklynDslMode allowBrooklynDsl) { if (expression==null) return null; if (expression.startsWith("$brooklyn:") && allowBrooklynDsl.isAllowedHere()) { - + if (wrappingMode.deferBrooklynDsl) { + return WrappedUnresolvedExpression.ofExpression(expression, this, allowBrooklynDsl); + } Object expressionTemplateResolved = processTemplateExpressionString(expression, AllowBrooklynDslMode.NONE); + // resolve interpolation before brooklyn DSL, so brooklyn DSL can be passed interpolated vars like workflow scratch; + // this means $brooklyn bits that return interpolated strings do not have their interpolation evaluated, which is probably sensible; + // and $brooklyn cannot be used inside an interpolated string, which is okay. Object expressionTemplateAndDslResolved = resolveDsl(expressionTemplateResolved); return expressionTemplateAndDslResolved; - - // previous to 2023-03-30, instead of above, we resolved DSL first. this meant DSL expressions that contained workflow expressions were allowed, - // which might be useful but probably shouldn't be supported; and furthermore you couldn't pass workflow vars to DSL expressions which should be supported. -// if (!Objects.equals(e2, expression)) { -// if (e2 instanceof String) { -// // proceed to below -// expression = (String) e2; -// } else { -// return processTemplateExpression(e2); -// } -// } - } TemplateHashModel model = new WorkflowFreemarkerModel(); @@ -556,10 +589,13 @@ public class WorkflowExpressionResolution { result = TemplateProcessor.processTemplateContents("workflow", expression, model, true, false, errorMode); } catch (Exception e) { Exception e2 = e; + if (wrappingMode.deferAndRetryErroneousExpressions) { + return WrappedUnresolvedExpression.ofExpression(expression, this, allowBrooklynDsl); + } if (!allowWaiting && Exceptions.isCausedByInterruptInAnyThread(e)) { e2 = new IllegalArgumentException("Expression value '"+expression+"' unavailable and not permitted to wait: "+ Exceptions.collapseText(e), e); } - if (useWrappedValue) { + if (wrappingMode.deferThrowingError) { // in wrapped value mode, errors don't throw until accessed, and when used in conditions they can be tested as absent return WrappedResolvedExpression.ofError(expression, new ResolutionFailureTreatedAsAbsent.ResolutionFailureTreatedAsAbsentDefaultException(e2)); } else { @@ -570,12 +606,19 @@ public class WorkflowExpressionResolution { } if (!expression.equals(result)) { - if (useWrappedValue) { + // not a static string + if (wrappingMode.deferInterpolation) { + return WrappedUnresolvedExpression.ofExpression(expression, this, allowBrooklynDsl); + } + if (wrappingMode.deferBrooklynDsl) { + return new WrappedResolvedExpression<Object>(expression, result); + } + // we try, but don't guarantee, that DSL expressions aren't re-resolved, ie $brooklyn:literal("$brooklyn:literal(\"x\")") won't return x; + // this block will return a supplier + result = processDslComponents(result); + + if (wrappingMode.wrapResolvedStrings) { return new WrappedResolvedExpression<Object>(expression, result); - } else { - // we try, but don't guarantee, that DSL expressions aren't re-resolved, ie $brooklyn:literal("$brooklyn:literal(\"x\")") won't return x; - // this block will - result = processDslComponents(result); } } @@ -636,6 +679,7 @@ public class WorkflowExpressionResolution { result.error = error; return result; } + @Override public T get() { if (error!=null) { @@ -651,4 +695,27 @@ public class WorkflowExpressionResolution { } } + public static class WrappedUnresolvedExpression implements DeferredSupplier<Object> { + + @Deprecated @Beta // might re-introduce but for now needs to cache workflow context -- via resolver -- so discouraged + public static WrappedUnresolvedExpression ofExpression(String expression, WorkflowExpressionResolution resolver, AllowBrooklynDslMode dslMode) { + return new WrappedUnresolvedExpression(expression, resolver, dslMode); + } + protected WrappedUnresolvedExpression(String expression, WorkflowExpressionResolution resolver, AllowBrooklynDslMode dslMode) { + this.expression = expression; + this.resolver = resolver; + this.dslMode = dslMode; + } + + String expression; + WorkflowExpressionResolution resolver; + AllowBrooklynDslMode dslMode; + + public Object get() { + WorkflowExpressionResolution resolverNow = new WorkflowExpressionResolution(resolver.context, resolver.stage, resolver.allowWaiting, + resolver.wrappingMode.wrappingModeWhenResolving(), resolver.errorMode); + return resolverNow.processTemplateExpression(expression, dslMode); + } + } + } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java index e7ac95b261..f6a2cdc54a 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepDefinition.java @@ -96,7 +96,7 @@ public abstract class WorkflowStepDefinition { @JsonIgnore public DslPredicates.DslPredicate getConditionResolved(WorkflowStepInstanceExecutionContext context) { try { - return context.resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, getConditionRaw(), TypeToken.of(DslPredicates.DslPredicate.class)); + return context.context.resolveCondition(getConditionRaw()); } catch (Exception e) { throw Exceptions.propagateAnnotated("Unresolveable condition (" + getConditionRaw() + ")", e); } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java index 2e421cddcf..ee2c1379d7 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepInstanceExecutionContext.java @@ -212,8 +212,8 @@ public class WorkflowStepInstanceExecutionContext { public <T> T resolve(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) { return context.resolve(stage, expression, type); } - public <T> T resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) { - return context.resolveWrapped(stage, expression, type); + public <T> T resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type, WorkflowExpressionResolution.WrappingMode wrappingMode) { + return context.resolveWrapped(stage, expression, type, wrappingMode); } public <T> T resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) { return context.resolveWaiting(stage, expression, type); diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java index 1a4d5a9200..0fdd8ab981 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java @@ -246,7 +246,9 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl AtomicInteger index = new AtomicInteger(0); ((Iterable<?>) targetR).forEach(t -> { WorkflowExecutionContext nw = newWorkflow(context, t, wasList ? index.getAndIncrement() : null); - Maybe<Task<Object>> mt = nw.getTask(true); + // workflow expressions are accessible in the condition, and the condition was resolveWrapped so will be looked up in the context of the invocation; + // thus ${target} can be used anywhere in it and should work + Maybe<Task<Object>> mt = nw.getTaskCheckingConditionWithTarget(t); String targetS = wasList || t !=null ? " for target '"+t+"'" : ""; if (mt.isAbsent()) { diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetSensorWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetSensorWorkflowStep.java index 93af80a119..72d7a11ebb 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetSensorWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetSensorWorkflowStep.java @@ -25,6 +25,7 @@ import org.apache.brooklyn.api.sensor.AttributeSensor; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.entity.AbstractEntity; +import org.apache.brooklyn.core.resolve.jackson.WrappedValue; import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution; import org.apache.brooklyn.core.workflow.WorkflowStepDefinition; @@ -132,7 +133,7 @@ public class SetSensorWorkflowStep extends WorkflowStepDefinition { if (old == null && !((AbstractEntity.BasicSensorSupport) entity.sensors()).contains(sensorBase.getName())) { DslPredicates.DslEntityPredicateDefault requireTweaked = new DslPredicates.DslEntityPredicateDefault(); requireTweaked.sensor = sensorNameFull; - requireTweaked.check = require; + requireTweaked.check = WrappedValue.of(require); if (!requireTweaked.apply(entity)) { throw new SensorRequirementFailedAbsent("Sensor " + sensorNameFull + " unset or unavailable when there is a non-absent requirement"); } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/HttpWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/HttpWorkflowStep.java index 1c93c96d5c..b262f1efb2 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/HttpWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/HttpWorkflowStep.java @@ -177,7 +177,7 @@ public class HttpWorkflowStep extends WorkflowStepDefinition { protected void checkExitCode(Integer code, Predicate<Integer> exitcode) { if (exitcode==null) return; if (exitcode instanceof DslPredicates.DslPredicateBase) { - Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEquals; + Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEqualsUnwrapped(); if (implicit!=null) { if ("any".equalsIgnoreCase(""+implicit)) { // if any is supplied as the implicit value, we accept; e.g. user says "exit_code: any" diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/SshWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/SshWorkflowStep.java index cb62a4ec53..32743191a1 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/SshWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/SshWorkflowStep.java @@ -119,7 +119,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition { } if (exitcode instanceof DslPredicates.DslPredicateBase) { - Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEquals; + Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEqualsUnwrapped(); if (implicit!=null) { if ("any".equalsIgnoreCase(""+implicit)) { // if any is supplied as the implicit value, we accept; e.g. user says "exit_code: any" 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 a9b112a520..2cb1bfd108 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 @@ -216,7 +216,7 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition { } <T> T resolveSubPart(Object v, TypeToken<T> type) { - return new WorkflowExpressionResolution(context.getWorkflowExectionContext(), WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, false, false, errorMode) + return new WorkflowExpressionResolution(context.getWorkflowExectionContext(), WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, false, WorkflowExpressionResolution.WrappingMode.NONE, errorMode) .resolveWithTemplates(v, type); } diff --git a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java index 8ed0c3533f..f3d6673095 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/predicates/DslPredicates.java @@ -42,6 +42,7 @@ import org.apache.brooklyn.core.mgmt.BrooklynTaskTags; import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils; import org.apache.brooklyn.core.resolve.jackson.BrooklynJacksonSerializationUtils; import org.apache.brooklyn.core.resolve.jackson.JsonSymbolDependentDeserializer; +import org.apache.brooklyn.core.resolve.jackson.WrappedValue; import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.util.JavaGroovyEquivalents; import org.apache.brooklyn.util.collections.MutableList; @@ -59,6 +60,7 @@ import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.guava.SerializablePredicate; import org.apache.brooklyn.util.javalang.Boxing; import org.apache.brooklyn.util.javalang.Reflections; +import org.apache.brooklyn.util.javalang.coerce.TryCoercer; import org.apache.brooklyn.util.text.NaturalOrderComparator; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.text.WildcardGlobs; @@ -87,12 +89,21 @@ public class DslPredicates { TypeCoercions.registerAdapter(java.util.function.Predicate.class, DslEntityPredicate.class, DslEntityPredicateAdapter::new); TypeCoercions.registerAdapter(java.util.function.Predicate.class, DslPredicate.class, DslPredicateAdapter::new); - // subsumed in above -// TypeCoercions.registerAdapter(com.google.common.base.Predicate.class, DslEntityPredicate.class, DslEntityPredicateAdapter::new); -// TypeCoercions.registerAdapter(com.google.common.base.Predicate.class, DslPredicate.class, DslPredicateAdapter::new); - // TODO could use json shorthand instead? + // could use json shorthand instead, but this is simpler TypeCoercions.registerAdapter(String.class, DslPredicate.class, DslPredicates::implicitlyEqualTo); + +// TypeCoercions.registerAdapter(DeferredSupplier.class, DslPredicate.class, DslPredicates::implicitlyEqualTo); +// TypeCoercions.registerAdapter(WorkflowExpressionResolution.WrappedUnresolvedExpression.class, DslPredicate.class, DslPredicates::implicitlyEqualTo); + // not sure why above don't work, but below does + TypeCoercions.registerAdapter("60-expression-to-predicate", new TryCoercer() { + @Override + public <T> Maybe<T> tryCoerce(Object input, TypeToken<T> type) { + if (!(input instanceof DeferredSupplier)) return null; + if (!DslPredicate.class.isAssignableFrom(type.getRawType())) return null; + return (Maybe) Maybe.of(type.getRawType().cast(implicitlyEqualTo(input))); + } + }); } static { init(); @@ -114,6 +125,20 @@ public class DslPredicates { /** always returns false */ NEVER, } + static <T> T unwrapped(WrappedValue<T> t) { + return WrappedValue.get(t); + } + + static Object unwrappedObject(Object t) { + if (t instanceof WrappedValue) return ((WrappedValue)t).get(); + return t; + } + + static Object undeferred(Object t) { + if (t instanceof DeferredSupplier) return ((DeferredSupplier)t).get(); + return t; + } + public static final boolean coercedEqual(Object a, Object b) { if (a==null || b==null) return a==null && b==null; @@ -123,7 +148,7 @@ public class DslPredicates { if (a.equals(b) || b.equals(a)) return true; if (a instanceof DeferredSupplier || b instanceof DeferredSupplier) - return coercedEqual(a instanceof DeferredSupplier ? ((DeferredSupplier)a).get() : a, b instanceof DeferredSupplier ? ((DeferredSupplier)b).get() : b); + return coercedEqual(undeferred(a), undeferred(b)); // if classes are equal or one is a subclass of the other, and the above check was false, that is decisive if (a.getClass().isAssignableFrom(b.getClass())) return false; @@ -190,7 +215,7 @@ public class DslPredicates { } if (a instanceof DeferredSupplier || b instanceof DeferredSupplier) - return coercedCompare(a instanceof DeferredSupplier ? ((DeferredSupplier)a).get() : a, b instanceof DeferredSupplier ? ((DeferredSupplier)b).get() : b); + return coercedCompare(undeferred(a), undeferred(b)); // if classes are equal or one is a subclass of the other, and the above check was false, that is decisive if (a.getClass().isAssignableFrom(b.getClass()) && b instanceof Comparable) return -((Comparable) b).compareTo(a); @@ -243,16 +268,16 @@ public class DslPredicates { @JsonInclude(JsonInclude.Include.NON_NULL) public static class DslPredicateBase<T> { - public Object implicitEquals; - public Object equals; - public String regex; - public String glob; + public WrappedValue<Object> implicitEquals; + public WrappedValue<Object> equals; + public WrappedValue<String> regex; + public WrappedValue<String> glob; /** nested check */ - public DslPredicate check; - public DslPredicate not; - public List<DslPredicate> any; - public List<DslPredicate> all; + public WrappedValue<DslPredicate> check; + public WrappedValue<DslPredicate> not; + public List<WrappedValue<DslPredicate>> any; + public List<WrappedValue<DslPredicate>> all; @JsonProperty("assert") public DslPredicate assertCondition; @@ -280,19 +305,19 @@ public class DslPredicates { int checksApplicable = 0; int checksPassed = 0; - public <T> void checkTest(T test, java.util.function.Predicate<T> predicateForTest) { - if (test!=null) { + public <T> void checkTest(T testFieldValue, java.util.function.Predicate<T> predicateForTest) { + if (testFieldValue!=null) { checksDefined++; checksApplicable++; - if (predicateForTest.test(test)) checksPassed++; + if (predicateForTest.test(testFieldValue)) checksPassed++; } } - public <T> void check(T test, Maybe<Object> value, java.util.function.BiPredicate<T,Object> check) { + public <T> void check(T testFieldValue, Maybe<Object> value, java.util.function.BiPredicate<T,Object> check) { if (value.isPresent()) { - checkTest(test, t -> check.test(t, value.get())); + checkTest(testFieldValue, t -> check.test(t, value.get())); } else { - if (test!=null) { + if (testFieldValue!=null) { checksDefined++; } } @@ -314,6 +339,8 @@ public class DslPredicates { } } + public Object implicitEqualsUnwrapped() { return unwrapped(implicitEquals); } + public boolean apply(T input) { Maybe<Object> result = resolveTargetAgainstInput(input); if (result.isPresent() && result.get() instanceof RetargettedPredicateEvaluation) { @@ -463,16 +490,24 @@ public class DslPredicates { public void applyToResolved(Maybe<Object> result, CheckCounts checker) { if (assertCondition!=null) failOnAssertCondition(result, checker); - checker.check(implicitEquals, result, (test, value) -> { + checker.check(implicitEquals, result, (implicitTestSpec, value) -> { + + // if a condition somehow gets put into the implicit equals, e.g. via an expression returning an expression, then recognize it as a condition + Object test = unwrapped(implicitTestSpec); + if (test instanceof DslPredicate) { + return nestedPredicateCheck((DslPredicate) test, result); + } + if ((!(test instanceof BrooklynObject) && value instanceof BrooklynObject) || (!(test instanceof Iterable) && value instanceof Iterable)) { - throw new IllegalStateException("Implicit string used for equality check comparing "+test+" with "+value+", which is probably not what was meant. Use explicit 'equals: ...' syntax for this case."); + throw new IllegalStateException("Implicit value used for equality check comparing "+test+" with "+value+", which is probably not what was meant. Use explicit 'equals: ...' syntax for this case."); } - return DslPredicates.coercedEqual(test, value); + + return DslPredicates.coercedEqual(implicitTestSpec, value); }); checker.check(equals, result, DslPredicates::coercedEqual); - checker.check(regex, result, (test, value) -> asStringTestOrFalse(value, v -> Pattern.compile(test, Pattern.DOTALL).matcher(v).matches())); - checker.check(glob, result, (test, value) -> asStringTestOrFalse(value, v -> WildcardGlobs.isGlobMatched(test, v))); + checker.check(regex, result, (test, value) -> asStringTestOrFalse(value, v -> Pattern.compile(unwrapped(test), Pattern.DOTALL).matcher(v).matches())); + checker.check(glob, result, (test, value) -> asStringTestOrFalse(value, v -> WildcardGlobs.isGlobMatched(unwrapped(test), v))); checker.check(inRange, result, (test,value) -> // current Range only supports Integer, but this code will support any @@ -503,10 +538,10 @@ public class DslPredicates { return nestedPredicateCheck(test, Maybe.of(computedSize)); }); - checker.checkTest(not, test -> !nestedPredicateCheck(test, result)); - checker.checkTest(check, test -> nestedPredicateCheck(test, result)); - checker.checkTest(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(p, result))); - checker.checkTest(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(p, result))); + checker.checkTest(not, test -> !nestedPredicateCheck(unwrapped(test), result)); + checker.checkTest(check, test -> nestedPredicateCheck(unwrapped(test), result)); + checker.checkTest(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(unwrapped(p), result))); + checker.checkTest(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(unwrapped(p), result))); checker.check(javaInstanceOf, result, this::checkJavaInstanceOf); } @@ -517,7 +552,7 @@ public class DslPredicates { boolean assertionPassed; if (assertCondition instanceof DslPredicateBase) { - Object implicitWhen = ((DslPredicateBase) assertCondition).implicitEquals; + Object implicitWhen = ((DslPredicateBase) assertCondition).implicitEqualsUnwrapped(); if (implicitWhen!=null) { // can assume no other checks, if one is implicit CheckCounts checker = new CheckCounts(); @@ -579,7 +614,7 @@ public class DslPredicates { // first check if implicitly equal to a registered type if (javaInstanceOf instanceof DslPredicateBase) { - Object implicitRegisteredType = ((DslPredicateBase) javaInstanceOf).implicitEquals; + Object implicitRegisteredType = ((DslPredicateBase) javaInstanceOf).implicitEqualsUnwrapped(); if (implicitRegisteredType instanceof String) { Entity ent = null; if (value instanceof Entity) ent = (Entity)value; @@ -673,16 +708,17 @@ public class DslPredicates { Entity.class, EntityAdjuncts.getEntity(bo, true).orNull())); } + /** default implementation */ @Beta public static class DslPredicateDefault<T2> extends DslPredicateBase<T2> implements DslPredicate<T2>, Cloneable { public DslPredicateDefault() {} // allow a string or int or other common types to be an implicit equality target - public DslPredicateDefault(String implicitEquals) { this.implicitEquals = implicitEquals; } - public DslPredicateDefault(Integer implicitEquals) { this.implicitEquals = implicitEquals; } - public DslPredicateDefault(Double implicitEquals) { this.implicitEquals = implicitEquals; } - public DslPredicateDefault(Long implicitEquals) { this.implicitEquals = implicitEquals; } - public DslPredicateDefault(Number implicitEquals) { this.implicitEquals = implicitEquals; } // note: Number is not matched by jackson bean constructor + public DslPredicateDefault(String implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); } + public DslPredicateDefault(Integer implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); } + public DslPredicateDefault(Double implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); } + public DslPredicateDefault(Long implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); } + public DslPredicateDefault(Number implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); } // note: Number is not matched by jackson bean constructor // not used by code, but allows clients to store other information public Object metadata; @@ -1018,13 +1054,13 @@ public class DslPredicates { public static DslPredicate equalTo(Object x) { DslEntityPredicateDefault result = new DslEntityPredicateDefault(); - result.equals = x; + result.equals = WrappedValue.of(x); return result; } public static DslPredicate implicitlyEqualTo(Object x) { DslEntityPredicateDefault result = new DslEntityPredicateDefault(); - result.implicitEquals = x; + result.implicitEquals = WrappedValue.of(x); return result; } diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowNestedAndCustomExtensionTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowNestedAndCustomExtensionTest.java index 517195896d..9df6e26cc7 100644 --- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowNestedAndCustomExtensionTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowNestedAndCustomExtensionTest.java @@ -901,4 +901,20 @@ public class WorkflowNestedAndCustomExtensionTest extends RebindTestFixture<Test "steps", MutableList.of("return element-${key}-${value}")))); Asserts.assertEquals(output, MutableList.of("element-K1-V1", "element-K2-V2")); } + + @Test + public void testForeachCondition() throws Exception { + Object output = invokeWorkflowStepsWithLogging(MutableList.of( + "let list L = [ a, b, c ]", + MutableMap.of("step", "foreach item in ${L}", + "steps", MutableList.of("return ${item}"), + "condition", MutableMap.of("any", MutableList.of( + "a", + MutableMap.of("target", MutableList.of("x", "c"), + "has-element", + "${item}" +// MutableMap.of("equals", "${item}") + )))))); + Asserts.assertEquals(output, MutableList.of("a", "c")); + } } diff --git a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java index fefe1e28fa..08a3c77d86 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/predicates/DslPredicateTest.java @@ -18,6 +18,7 @@ */ package org.apache.brooklyn.util.core.predicates; +import org.apache.brooklyn.core.resolve.jackson.WrappedValue; import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.collections.MutableList; @@ -533,7 +534,7 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport { "tag", "locationTagValueMatched"), DslPredicates.DslPredicate.class); Asserts.assertInstanceOf(p, DslPredicates.DslPredicateDefault.class); Asserts.assertInstanceOf( ((DslPredicates.DslPredicateDefault)p).tag, DslPredicates.DslPredicateDefault.class); - Asserts.assertEquals( ((DslPredicates.DslPredicateDefault) ((DslPredicates.DslPredicateDefault)p).tag).implicitEquals, "locationTagValueMatched"); + Asserts.assertEquals( ((DslPredicates.DslPredicateDefault) ((DslPredicates.DslPredicateDefault)p).tag).implicitEqualsUnwrapped(), "locationTagValueMatched"); } @Test @@ -542,7 +543,7 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport { DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of("target", "location", "equals", kvMap, "config", "x"), DslPredicates.DslPredicate.class); Asserts.assertInstanceOf(p, DslPredicates.DslPredicateDefault.class); - Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)p).equals, kvMap); + Asserts.assertEquals( WrappedValue.get( ((DslPredicates.DslPredicateDefault)p).equals ), kvMap); Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)p).config, "x"); } diff --git a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java index 9c4ad3d39e..367e3575da 100644 --- a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java +++ b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java @@ -108,7 +108,7 @@ public class ContainerWorkflowStep extends WorkflowStepDefinition { protected void checkExitCode(ContainerTaskResult ptw, DslPredicates.DslPredicate<Integer> exitcode) { if (exitcode==null) return; if (exitcode instanceof DslPredicates.DslPredicateBase) { - Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEquals; + Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEqualsUnwrapped(); if (implicit!=null) { if ("any".equalsIgnoreCase(""+implicit)) { // if any is supplied as the implicit value, we accept; e.g. user says "exit-code: any" diff --git a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java index ba75c6aed5..087d710fcb 100644 --- a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java +++ b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java @@ -106,7 +106,7 @@ public class WinrmWorkflowStep extends WorkflowStepDefinition { protected void checkExitCode(ProcessTaskWrapper<?> ptw, DslPredicates.DslPredicate<Integer> exitcode) { if (exitcode==null) return; if (exitcode instanceof DslPredicates.DslPredicateBase) { - Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEquals; + Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEqualsUnwrapped(); if (implicit!=null) { if ("any".equalsIgnoreCase(""+implicit)) { // if any is supplied as the implicit value, we accept; e.g. user says "exit-code: any" diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TryCoercer.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TryCoercer.java index f410fbc92d..27243a40d6 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TryCoercer.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TryCoercer.java @@ -18,10 +18,9 @@ */ package org.apache.brooklyn.util.javalang.coerce; -import org.apache.brooklyn.util.guava.Maybe; - import com.google.common.annotations.Beta; import com.google.common.reflect.TypeToken; +import org.apache.brooklyn.util.guava.Maybe; /** * A coercer that can be registered, which will try to coerce the given input to the given type.
