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 0931246804a2c7747cb80278d0d19337c1a7a826 Author: Alex Heneveld <a...@cloudsoft.io> AuthorDate: Thu Feb 22 17:24:44 2024 +0000 add if step, and improve deep workflow partial resolution for conditions --- .../brooklyn/camp/brooklyn/WorkflowYamlTest.java | 2 +- .../core/workflow/WorkflowExecutionContext.java | 13 +- .../workflow/WorkflowExpressionResolution.java | 137 ++++++++++++--------- .../core/workflow/WorkflowStepDefinition.java | 9 +- .../WorkflowStepInstanceExecutionContext.java | 2 + .../core/workflow/WorkflowStepResolution.java | 15 +-- .../core/workflow/steps/flow/IfWorkflowStep.java | 101 +++++++++++++++ .../workflow/steps/flow/ReturnWorkflowStep.java | 5 +- .../core/workflow/steps/flow/SubWorkflowStep.java | 3 +- .../brooklyn/core/workflow/WorkflowBasicTest.java | 2 + ...> WorkflowSubIfAndCustomExtensionEdgeTest.java} | 101 +++++++++++---- .../core/workflow/WorkflowTransformTest.java | 24 ++-- .../brooklyn/util/core/ResourceUtilsTest.java | 2 +- karaf/init/src/main/resources/catalog.bom | 5 + .../brooklyn/launcher/BrooklynWebServerTest.java | 4 +- parent/pom.xml | 2 +- 16 files changed, 310 insertions(+), 117 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 7779fc6d63..b7ba499072 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 @@ -689,7 +689,7 @@ public class WorkflowYamlTest extends AbstractYamlTest { @Test public void testConditionBadSerialization() throws Exception { Asserts.assertFailsWith(() -> doTestCondition("- regex: .*oh no.*"), - e -> Asserts.expectedFailureContainsIgnoreCase(e, "unresolveable", "regex")); + Asserts.expectedFailureContainsIgnoreCase("unresolvable", "regex")); } @Test public void testBadExpressionAllowedInCondition() throws Exception { 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 72b5477b04..a26febbd3e 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 @@ -709,7 +709,7 @@ public class WorkflowExecutionContext { } /** 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; + * but returning DSL/supplier for dynamic 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, WorkflowExpressionResolution.WrappingMode wrappingMode) { return new WorkflowExpressionResolution(this, stage, false, wrappingMode).resolveWithTemplates(expression, type); @@ -1597,8 +1597,19 @@ public class WorkflowExecutionContext { log.debug(prefix + "no further steps: Workflow completed"); } } else if (specialNext instanceof String) { + boolean isInLocalSubworkflow = getParent()!=null && getParent().currentStepInstance!=null && Boolean.TRUE.equals(getParent().currentStepInstance.isLocalSubworkflow); + if (isInLocalSubworkflow && Boolean.TRUE.equals(currentStepInstance.nextIsReturn)) { + // parent of local subworkflow should return also + getParent().currentStepInstance.next = specialNext; + } + String explicitNext = (String)specialNext; Maybe<Pair<Integer, Boolean>> nextResolved = getIndexOfStepId(explicitNext); + if (nextResolved.isAbsent() && isInLocalSubworkflow) { + // if in subworkflow, you can goto something in the outer workflow, if not found here + getParent().currentStepInstance.next = specialNext; + nextResolved = getIndexOfStepId(STEP_TARGET_NAME_FOR_END); + } if (nextResolved.isAbsent()) { log.warn(prefix + (inferredNext ? "inferred" : "explicit") + " next step '"+explicitNext+"' not found (failing)"); // throw 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 ab4836fc56..c8e32abad5 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,9 +18,20 @@ */ package org.apache.brooklyn.core.workflow; +import java.time.Instant; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.function.BiFunction; +import java.util.function.Supplier; +import java.util.stream.Collectors; + import com.google.common.annotations.Beta; import com.google.common.reflect.TypeToken; -import freemarker.core.InvalidReferenceException; import freemarker.template.TemplateHashModel; import freemarker.template.TemplateModel; import freemarker.template.TemplateModelException; @@ -49,12 +60,6 @@ import org.apache.commons.lang3.tuple.Pair; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.time.Instant; -import java.util.*; -import java.util.function.BiFunction; -import java.util.function.Supplier; -import java.util.stream.Collectors; - public class WorkflowExpressionResolution { public static ConfigKey<BiFunction<String,WorkflowExpressionResolution,Object>> WORKFLOW_CUSTOM_INTERPOLATION_FUNCTION = ConfigKeys.newConfigKey(new TypeToken<BiFunction<String,WorkflowExpressionResolution,Object>>() {}, "workflow.custom_interpolation_function"); @@ -82,14 +87,14 @@ public class WorkflowExpressionResolution { private final WrappingMode wrappingMode; public static class WrappingMode { - public final boolean wrapResolvedStrings; + public final boolean wrapResolvedValues; 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; + protected WrappingMode(boolean wrapResolvedValues, boolean deferThrowingError, boolean deferAndRetryErroneousExpressions, boolean deferBrooklynDsl, boolean deferInterpolation) { + this.wrapResolvedValues = wrapResolvedValues; this.deferThrowingError = deferThrowingError; this.deferAndRetryErroneousExpressions = deferAndRetryErroneousExpressions; this.deferBrooklynDsl = deferBrooklynDsl; @@ -375,14 +380,17 @@ public class WorkflowExpressionResolution { } } - AllowBrooklynDslMode defaultAllowBrooklynDsl = AllowBrooklynDslMode.ALL; + AllowBrooklynDslMode defaultAllowBrooklynDsl = null; + //AllowBrooklynDslMode.ALL; public void setDefaultAllowBrooklynDsl(AllowBrooklynDslMode defaultAllowBrooklynDsl) { this.defaultAllowBrooklynDsl = defaultAllowBrooklynDsl; } public AllowBrooklynDslMode getDefaultAllowBrooklynDsl() { - return defaultAllowBrooklynDsl; + if (defaultAllowBrooklynDsl!=null) return defaultAllowBrooklynDsl; + if (wrappingMode.deferBrooklynDsl) return AllowBrooklynDslMode.NONE; + return AllowBrooklynDslMode.ALL; } public <T> T resolveWithTemplates(Object expression, TypeToken<T> type) { @@ -550,14 +558,36 @@ public class WorkflowExpressionResolution { throw new WorkflowVariableRecursiveReference("Reference exceeded max depth 100: " + RESOLVE_STACK.getAll(false).stream().map(p -> "" + p.object).collect(Collectors.joining("->"))); } - if (expression instanceof String) return processTemplateExpressionString((String) expression, allowBrooklynDsl); - if (expression instanceof Map) return processTemplateExpressionMap((Map) expression, allowBrooklynDsl); - if (expression instanceof Collection) - return processTemplateExpressionCollection((Collection) expression, allowBrooklynDsl); - if (expression == null || Boxing.isPrimitiveOrBoxedObject(expression)) return expression; - // otherwise resolve DSL - return allowBrooklynDsl.isAllowedHere() ? resolveDsl(expression) : expression; + Object result; + if (expression instanceof String) result = processTemplateExpressionString((String) expression, allowBrooklynDsl); + else if (expression instanceof Map) result = processTemplateExpressionMap((Map) expression, allowBrooklynDsl); + else if (expression instanceof Collection) + result = processTemplateExpressionCollection((Collection) expression, allowBrooklynDsl); + else if (expression == null || Boxing.isPrimitiveOrBoxedObject(expression)) result = expression; + else { + // otherwise resolve DSL + result = allowBrooklynDsl.isAllowedHere() ? resolveDsl(expression) : expression; + if (wrappingMode.wrapResolvedValues && !Objects.equals(result, expression) && !(result instanceof DeferredSupplier)) { + result = WrappedResolvedExpression.ifNonDeferred(expression, result); + } + } + + return result; + } 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 (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 { + throw Exceptions.propagate(e2); + } } finally { if (entry != null) RESOLVE_STACK.pop(entry); } @@ -602,41 +632,28 @@ 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; - } - Object result; - - boolean ourWait = interruptSetIfNeededToPreventWaiting(); + boolean ourWait = false; try { + 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; + } + + ourWait = interruptSetIfNeededToPreventWaiting(); BiFunction<String, WorkflowExpressionResolution, Object> fn = context.getManagementContext().getScratchpad().get(WORKFLOW_CUSTOM_INTERPOLATION_FUNCTION); if (fn!=null) result = fn.apply(expression, this); else result = TemplateProcessor.processTemplateContentsForWorkflow("workflow", expression, newWorkflowFreemarkerModel(), 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 (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 { - throw Exceptions.propagate(e2); - } + } finally { if (ourWait) interruptClear(); } @@ -647,14 +664,14 @@ public class WorkflowExpressionResolution { return WrappedUnresolvedExpression.ofExpression(expression, this, allowBrooklynDsl); } if (wrappingMode.deferBrooklynDsl) { - return new WrappedResolvedExpression<Object>(expression, result); + return new WrappedResolvedExpression<>(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); + if (wrappingMode.wrapResolvedValues) { + return new WrappedResolvedExpression<>(expression, result); } } @@ -702,19 +719,23 @@ public class WorkflowExpressionResolution { } public static class WrappedResolvedExpression<T> implements DeferredSupplier<T> { - String expression; + Object expression; T value; Throwable error; public WrappedResolvedExpression() {} - public WrappedResolvedExpression(String expression, T value) { + public WrappedResolvedExpression(Object expression, T value) { this.expression = expression; this.value = value; } - public static WrappedResolvedExpression ofError(String expression, Throwable error) { + public static WrappedResolvedExpression ofError(Object expression, Throwable error) { WrappedResolvedExpression result = new WrappedResolvedExpression(expression, null); result.error = error; return result; } + public static <T> DeferredSupplier<T> ifNonDeferred(Object expression, T value) { + if (value instanceof DeferredSupplier) return (DeferredSupplier<T>) value; + return new WrappedResolvedExpression(expression, value); + } @Override public T get() { @@ -723,7 +744,7 @@ public class WorkflowExpressionResolution { } return value; } - public String getExpression() { + public Object getExpression() { return expression; } public Throwable getError() { @@ -734,16 +755,16 @@ 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) { + public static WrappedUnresolvedExpression ofExpression(Object expression, WorkflowExpressionResolution resolver, AllowBrooklynDslMode dslMode) { return new WrappedUnresolvedExpression(expression, resolver, dslMode); } - protected WrappedUnresolvedExpression(String expression, WorkflowExpressionResolution resolver, AllowBrooklynDslMode dslMode) { + protected WrappedUnresolvedExpression(Object expression, WorkflowExpressionResolution resolver, AllowBrooklynDslMode dslMode) { this.expression = expression; this.resolver = resolver; this.dslMode = dslMode; } - String expression; + Object expression; WorkflowExpressionResolution resolver; AllowBrooklynDslMode 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 25ca27e38c..3a5ab6b530 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,10 +96,15 @@ public abstract class WorkflowStepDefinition { @JsonIgnore public DslPredicates.DslPredicate getConditionResolved(WorkflowStepInstanceExecutionContext context) { + return getConditionResolved(context, getConditionRaw()); + } + + @JsonIgnore + public DslPredicates.DslPredicate getConditionResolved(WorkflowStepInstanceExecutionContext context, Object conditionRaw) { try { - return context.context.resolveCondition(getConditionRaw()); + return context.context.resolveCondition(conditionRaw); } catch (Exception e) { - throw Exceptions.propagateAnnotated("Unresolveable condition (" + getConditionRaw() + ")", e); + throw Exceptions.propagateAnnotated("Unresolvable 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 db8543f361..e1c6df8316 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 @@ -81,6 +81,8 @@ public class WorkflowStepInstanceExecutionContext { transient WorkflowExecutionContext context; // replay instructions or a string explicit next step identifier public Object next; + public Boolean nextIsReturn; + public Boolean isLocalSubworkflow; /** Return any error we are handling, if the step is in an error handler, * or an unhandled error if the step is not in an error handler, 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 c08b1d2c36..358687a048 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 @@ -34,6 +34,7 @@ import org.apache.brooklyn.core.mgmt.internal.AbstractManagementContext; import org.apache.brooklyn.core.objs.BrooklynObjectInternal; import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils; import org.apache.brooklyn.core.typereg.RegisteredTypes; +import org.apache.brooklyn.core.workflow.steps.CustomWorkflowStep; import org.apache.brooklyn.core.workflow.steps.flow.SubWorkflowStep; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; @@ -113,20 +114,14 @@ public class WorkflowStepResolution { public List<WorkflowStepDefinition> resolveSubSteps(String scope, List<Object> subSteps) { List<WorkflowStepDefinition> result = MutableList.of(); if (subSteps!=null) { - // it's useful to allow subworkflows - // XXX also useful to compress if a long-winded syntax was used -// if (subSteps.size()==1) { -// WorkflowStepDefinition subStepResolved = resolveStep(mgmt, Iterables.getOnlyElement(subSteps)); -// if (subStepResolved instanceof SubWorkflowStep && ((SubWorkflowStep)subStepResolved).isSimpleListOfStepsOnly()) { -// return resolveSubSteps(mgmt, scope, ((SubWorkflowStep)subStepResolved).peekSteps()); -// } -// } subSteps.forEach(subStep -> { WorkflowStepDefinition subStepResolved = resolveStep(subStep); if (subStepResolved.getId() != null) throw new IllegalArgumentException("Sub steps for "+scope+" are not permitted to have IDs: " + subStep); -// if (subStepResolved instanceof CustomWorkflowStep && ((CustomWorkflowStep)subStepResolved).peekSteps()!=null) -// throw new IllegalArgumentException("Sub steps for "+scope+" are not permitted to run sub-workflows: " + subStep); + // don't allow foreach and workflow with target, but do allow subworkflow and if + if (subStepResolved instanceof CustomWorkflowStep && !(subStepResolved instanceof SubWorkflowStep) && + ((CustomWorkflowStep)subStepResolved).peekSteps()!=null) + throw new IllegalArgumentException("Sub steps for "+scope+" are not permitted to run custom or foreach sub-workflows: " + subStep); result.add(subStepResolved); }); } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/IfWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/IfWorkflowStep.java new file mode 100644 index 0000000000..0fcdfdda8c --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/IfWorkflowStep.java @@ -0,0 +1,101 @@ +/* + * 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.flow; + +import java.util.Map; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.google.common.reflect.TypeToken; +import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution.WorkflowExpressionStage; +import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution.WrappedResolvedExpression; +import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution.WrappingMode; +import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext; +import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.collections.MutableMap; +import org.apache.brooklyn.util.core.predicates.DslPredicates.DslPredicate; +import org.apache.brooklyn.util.core.predicates.DslPredicates.WhenPresencePredicate; + +public class IfWorkflowStep extends SubWorkflowStep { + + // more conditions could be done in future, or (probably better) we bake it in to the interpolation langauge + public static final String SHORTHAND = "${condition_target} [ \"==\" ${condition_equals} ] [ \" then \" ${step...} ]"; + + public static final String SHORTHAND_TYPE_NAME_DEFAULT = "if"; + + protected boolean isInternalClassNotExtendedAndUserAllowedToSetMostThings(String typeBestGuess) { + return !isRegisteredTypeExtensionToClass(IfWorkflowStep.class, SHORTHAND_TYPE_NAME_DEFAULT, typeBestGuess); + } + + Object condition_target; + Object condition_equals; + + @Override + public void populateFromShorthand(String value) { + if (input==null) input = MutableMap.of(); + populateFromShorthandTemplate(SHORTHAND, value, true, true); + + if (input.containsKey("step")) { + Object step = input.remove("step"); + if (this.steps!=null) throw new IllegalArgumentException("Cannot set step in shorthand and steps in body"); + this.steps = MutableList.of(step); + } else if (steps==null) throw new IllegalArgumentException("'if' step requires a step or steps"); + + if (input.containsKey("condition_target")) { + if (this.condition!=null) throw new IllegalArgumentException("Cannot set condition in shorthand and in body"); + this.condition_target = input.remove("condition_target"); + if (input.containsKey("condition_equals")) { + this.condition_equals = input.remove("condition_equals"); + } + } else if (condition == null) throw new IllegalArgumentException("'if' step requires a condition"); + } + + @Override + public Object getConditionRaw() { + // not accurate, but not used + return super.getConditionRaw(); + } + + @JsonIgnore + @Override + public DslPredicate getConditionResolved(WorkflowStepInstanceExecutionContext context) { + Map<String, Object> raw = MutableMap.of("target", getConditionRaw()); + Object conditionConstructed = this.condition; + if (conditionConstructed==null) { + Object k = context.getWorkflowExectionContext().resolveWrapped( + WorkflowExpressionStage.STEP_RUNNING, condition_target, TypeToken.of(Object.class), + WrappingMode.WRAPPED_RESULT_DEFER_THROWING_ERROR_BUT_NO_RETRY); + Map c = MutableMap.of("target", WrappedResolvedExpression.ifNonDeferred(condition_target, k)); + + if (condition_equals !=null) { + Object v = context.getWorkflowExectionContext().resolveWrapped( + WorkflowExpressionStage.STEP_RUNNING, condition_equals, TypeToken.of(Object.class), + WrappingMode.WRAPPED_RESULT_DEFER_THROWING_ERROR_BUT_NO_RETRY); + c.put("check", WrappedResolvedExpression.ifNonDeferred(condition_equals, v)); + c.put("assert", MutableMap.of("when", WhenPresencePredicate.PRESENT)); // when doing equals we need LHS and RHS to be present + } else { + c.put("when", WhenPresencePredicate.TRUTHY); + } + conditionConstructed = c; + } + + DslPredicate result = getConditionResolved(context, conditionConstructed); + + return result; + } +} diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/ReturnWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/ReturnWorkflowStep.java index ed7eb0cc66..6316d4521b 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/ReturnWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/ReturnWorkflowStep.java @@ -39,7 +39,10 @@ public class ReturnWorkflowStep extends WorkflowStepDefinition { @Override protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) { - if (next==null) context.next = STEP_TARGET_NAME_FOR_END; + if (next==null) { + context.next = STEP_TARGET_NAME_FOR_END; + context.nextIsReturn = true; + } return context.getInput(VALUE); } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/SubWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/SubWorkflowStep.java index 416853aa23..3ce6c3336b 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/SubWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/SubWorkflowStep.java @@ -56,7 +56,7 @@ public class SubWorkflowStep extends CustomWorkflowStep { protected void checkCallerSuppliedDefinition(String typeBestGuess, Map m) { if (!isInternalClassNotExtendedAndUserAllowedToSetMostThings(typeBestGuess)) { - throw new IllegalArgumentException("Not permitted to define a custom subworkflow step"); + throw new IllegalArgumentException("Not permitted to define a custom subworkflow step with this supertype"); } // these can't be set by user or registered type for subworkflow FORBIDDEN_IN_SUBWORKFLOW_STEP_ALWAYS.stream().filter(m::containsKey).forEach(forbiddenKey -> { @@ -75,6 +75,7 @@ public class SubWorkflowStep extends CustomWorkflowStep { @Override protected Map initializeReducingVariables(WorkflowStepInstanceExecutionContext context, Map<String, Object> reducing) { + context.isLocalSubworkflow = true; return super.initializeReducingVariables(context, context.getWorkflowExectionContext().getWorkflowScratchVariables()); } 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 10124ada75..f89f79b206 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 @@ -72,6 +72,7 @@ import org.apache.brooklyn.core.workflow.steps.external.SshWorkflowStep; import org.apache.brooklyn.core.workflow.steps.flow.FailWorkflowStep; import org.apache.brooklyn.core.workflow.steps.flow.ForeachWorkflowStep; import org.apache.brooklyn.core.workflow.steps.flow.GotoWorkflowStep; +import org.apache.brooklyn.core.workflow.steps.flow.IfWorkflowStep; import org.apache.brooklyn.core.workflow.steps.flow.LogWorkflowStep; import org.apache.brooklyn.core.workflow.steps.flow.NoOpWorkflowStep; import org.apache.brooklyn.core.workflow.steps.flow.RetryWorkflowStep; @@ -141,6 +142,7 @@ public class WorkflowBasicTest extends BrooklynMgmtUnitTestSupport { addRegisteredTypeBean(mgmt, "clear-workflow-variable", ClearVariableWorkflowStep.class); addRegisteredTypeBean(mgmt, "wait", WaitWorkflowStep.class); addRegisteredTypeBean(mgmt, "return", ReturnWorkflowStep.class); + addRegisteredTypeBean(mgmt, "if", IfWorkflowStep.class); addRegisteredTypeBean(mgmt, "goto", GotoWorkflowStep.class); addRegisteredTypeBean(mgmt, "switch", SwitchWorkflowStep.class); addRegisteredTypeBean(mgmt, "fail", FailWorkflowStep.class); diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubAndCustomExtensionEdgeTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubIfAndCustomExtensionEdgeTest.java similarity index 62% rename from core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubAndCustomExtensionEdgeTest.java rename to core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubIfAndCustomExtensionEdgeTest.java index 6431170142..4505bd8f15 100644 --- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubAndCustomExtensionEdgeTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubIfAndCustomExtensionEdgeTest.java @@ -20,8 +20,10 @@ package org.apache.brooklyn.core.workflow; import java.io.Serializable; import java.util.List; +import java.util.Map; +import java.util.function.BiFunction; +import java.util.function.Function; -import org.apache.brooklyn.api.entity.EntityLocal; import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.mgmt.Task; import org.apache.brooklyn.api.typereg.RegisteredType; @@ -45,9 +47,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.annotations.Test; -public class WorkflowSubAndCustomExtensionEdgeTest extends RebindTestFixture<TestApplication> { +public class WorkflowSubIfAndCustomExtensionEdgeTest extends RebindTestFixture<TestApplication> { - private static final Logger log = LoggerFactory.getLogger(WorkflowSubAndCustomExtensionEdgeTest.class); + private static final Logger log = LoggerFactory.getLogger(WorkflowSubIfAndCustomExtensionEdgeTest.class); @Override protected LocalManagementContext decorateOrigOrNewManagementContext(LocalManagementContext mgmt) { @@ -74,16 +76,16 @@ public class WorkflowSubAndCustomExtensionEdgeTest extends RebindTestFixture<Tes TestApplication app; Task<?> lastInvocation; - Object runWorkflow(List<Object> steps) throws Exception { + Object runWorkflow(List<Object> steps) { return runWorkflow(steps, null); } - Object runWorkflow(List<Object> steps, ConfigBag extraEffectorConfig) throws Exception { + Object runWorkflow(List<Object> steps, ConfigBag extraEffectorConfig) { if (app==null) app = mgmt().getEntityManager().createEntity(EntitySpec.create(TestApplication.class)); WorkflowEffector eff = new WorkflowEffector(ConfigBag.newInstance() .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow") .configure(WorkflowEffector.STEPS, steps) .putAll(extraEffectorConfig)); - eff.apply((EntityLocal)app); + eff.apply(app); lastInvocation = app.invoke(app.getEntityType().getEffectorByName("myWorkflow").get(), null); return lastInvocation.getUnchecked(); @@ -143,31 +145,76 @@ public class WorkflowSubAndCustomExtensionEdgeTest extends RebindTestFixture<Tes @Test public void testSubWorkflowStep() throws Exception { - MutableList<String> steps = MutableList.of( - "let v0 = ${v0}B", - "let v1 = ${v1}B", - "let v3 = V3B", - "return done"); - runWorkflow(MutableList.of( - "let v1 = V1", - "let v2 = V2", - MutableMap.of( - "step", "subworkflow", - "steps", steps), - "return ${v0}-${v1}-${v2}-${v3}-${output}"), - ConfigBag.newInstance().configure(WorkflowCommonConfig.INPUT, MutableMap.of("v0", "V0")) ); - Asserts.assertEquals(lastInvocation.getUnchecked(), "V0B-V1B-V2-V3B-done"); + Function<Boolean,Object> test = (explicitSubworkflow) -> + runWorkflow(MutableList.of( + "let v1 = V1", + "let v2 = V2", + MutableMap.<String,Object>of("steps", MutableList.of( + "let v0 = ${v0}B", + "let v1 = ${v1}B", + "let v3 = V3B")) + .add(explicitSubworkflow ? MutableMap.of("step", "subworkflow") : null), + "return ${v0}-${v1}-${v2}-${v3}"), + ConfigBag.newInstance().configure(WorkflowCommonConfig.INPUT, MutableMap.of("v0", "V0")) ); + Asserts.assertEquals(test.apply(true), "V0B-V1B-V2-V3B"); // subworkflow is chosen implicitly if step is omitted + Asserts.assertEquals(test.apply(false), "V0B-V1B-V2-V3B"); + runWorkflow(MutableList.of( + MutableMap.of("steps", MutableList.of( "let v1 = V1", - "let v2 = V2", - MutableMap.of( - //"step", "subworkflow", - "steps", steps), - "return ${v0}-${v1}-${v2}-${v3}-${output}"), - ConfigBag.newInstance().configure(WorkflowCommonConfig.INPUT, MutableMap.of("v0", "V0")) ); - Asserts.assertEquals(lastInvocation.getUnchecked(), "V0B-V1B-V2-V3B-done"); + "goto marker", // prefers inner id 'marker' + "let v1 = NOT_V1_1", + MutableMap.of("id", "marker", "step", "goto end"), // goes to end of this subworkflow + "let v1 = NOT_V1_2")), + "let v2 = V2", + MutableMap.of("steps", MutableList.of( + "let v3 = V3", + "goto marker", // goes to outer id 'marker' because nothing local matching + "let v3 = NOT_V3")), + MutableMap.of("id", "marker", "step", "let v4 = V4"), + MutableMap.of("steps", MutableList.of( + "return ${v1}-${v2}-${v3}-${v4}")), // returns from outer workflow + "let v4 = NOT_V4")); + Asserts.assertEquals(lastInvocation.getUnchecked(), "V1-V2-V3-V4"); + } + + @Test + public void testIfWorkflowStep() throws Exception { + BiFunction<String,String,Object> run = (preface,cond) -> + runWorkflow(MutableList.<Object>of(preface==null ? "let x = hi" : preface, + "let y = no", + "if "+(cond==null ? "${x}" : cond)+" then let y = yes", + "return ${y}")); + + Asserts.assertEquals(run.apply(null, null), "yes"); + + Asserts.assertEquals(run.apply("let boolean x = true", null), "yes"); + Asserts.assertEquals(run.apply("let boolean x = false", null), "no"); + + Asserts.assertEquals(run.apply(null, "${x} == hi"), "yes"); + Asserts.assertEquals(run.apply(null, "${x} == ho"), "no"); + Asserts.assertEquals(run.apply(null, "hi == ${x}"), "yes"); + Asserts.assertEquals(run.apply(null, "${x} == ${x}"), "yes"); + Asserts.assertEquals(run.apply("let boolean x = true", "${x} == true"), "yes"); + + // unresolvable things -- allowed iff no == block + Asserts.assertEquals(run.apply(null, "${unresolvable_without_equals}"), "no"); + Asserts.assertFailsWith(() -> run.apply(null, "${x} == ${unresolvable_on_rhs}"), + Asserts.expectedFailureContainsIgnoreCase("unresolvable_on_rhs", "missing")); + Asserts.assertFailsWith(() -> run.apply(null, "${unresolvable_on_lhs} == ${x}"), + Asserts.expectedFailureContainsIgnoreCase("unresolvable_on_lhs", "missing")); + + // flow - returns directly + Asserts.assertEquals(runWorkflow(MutableList.of("let boolean x = true", + "if ${x} then return yes", + "return no")), "yes"); + + Asserts.assertEquals(runWorkflow(MutableList.of("let integer x = 1", + MutableMap.of("id", "loop", "step", "let x = ${x} + 1"), + "if ${x} == 2 then goto loop", + "return ${x}")), 3); } } diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java index 4a29156f4a..c6ab621164 100644 --- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java @@ -135,33 +135,33 @@ public class WorkflowTransformTest extends BrooklynMgmtUnitTestSupport { Asserts.assertEquals(transform("value 'silly world' | replace regex l. k"), "siky world"); Asserts.assertEquals(transform("value 'silly world' | replace all regex l. k"), "siky work"); // with slash - Asserts.assertEquals(transform("value 'abc/def/ghi' | replace regex 'c/d' XXX"), "abXXXef/ghi"); + Asserts.assertEquals(transform("value 'abc/def/ghi' | replace regex 'c/d' 000"), "ab000ef/ghi"); // with space - Asserts.assertEquals(transform("value 'abc def ghi' | replace regex 'c d' XXX"), "abXXXef ghi"); + Asserts.assertEquals(transform("value 'abc def ghi' | replace regex 'c d' 000"), "ab000ef ghi"); // greedy - Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace regex 'c.*d' XXX"), "abXXX"); - Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace all regex 'c.*d' XXX"), "abXXX"); + Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace regex 'c.*d' 000"), "ab000"); + Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace all regex 'c.*d' 000"), "ab000"); // non-greedy qualifier - Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace regex 'c.*?d' XXX"), "abXXXef ghi c2d"); - Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace all regex 'c.*?d' XXX"), "abXXXef ghi XXX"); + Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace regex 'c.*?d' 000"), "ab000ef ghi c2d"); + Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace all regex 'c.*?d' 000"), "ab000ef ghi 000"); Asserts.assertEquals(transform("value 'abc def ghi' | replace regex 'c d' ''"), "abef ghi"); } @Test public void testTransformLiteral() { - Asserts.assertEquals(transform("value 'abc def ghi' | replace literal c.*d XXX"), "abc def ghi"); - Asserts.assertEquals(transform("value 'abc.*def ghi c.*d' | replace literal c.*d XXX"), "abXXXef ghi c.*d"); - Asserts.assertEquals(transform("value 'abc.*def ghi c.*d' | replace all literal c.*d XXX"), "abXXXef ghi XXX"); + Asserts.assertEquals(transform("value 'abc def ghi' | replace literal c.*d 000"), "abc def ghi"); + Asserts.assertEquals(transform("value 'abc.*def ghi c.*d' | replace literal c.*d 000"), "ab000ef ghi c.*d"); + Asserts.assertEquals(transform("value 'abc.*def ghi c.*d' | replace all literal c.*d 000"), "ab000ef ghi 000"); } @Test public void testTransformGlob() { - Asserts.assertEquals(transform("value 'abc def ghi' | replace glob c*e XXX"), "abXXXf ghi"); + Asserts.assertEquals(transform("value 'abc def ghi' | replace glob c*e 000"), "ab000f ghi"); // glob is greedy, unless all is specified where it is not - Asserts.assertEquals(transform("value 'abc def ghi c2e' | replace glob c*e XXX"), "abXXX"); - Asserts.assertEquals(transform("value 'abc def ghi c2e' | replace all glob c*e XXX"), "abXXXf ghi XXX"); + Asserts.assertEquals(transform("value 'abc def ghi c2e' | replace glob c*e 000"), "ab000"); + Asserts.assertEquals(transform("value 'abc def ghi c2e' | replace all glob c*e 000"), "ab000f ghi 000"); } @Test diff --git a/core/src/test/java/org/apache/brooklyn/util/core/ResourceUtilsTest.java b/core/src/test/java/org/apache/brooklyn/util/core/ResourceUtilsTest.java index 26195ffcf3..8a9063fe6d 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/ResourceUtilsTest.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/ResourceUtilsTest.java @@ -149,7 +149,7 @@ public class ResourceUtilsTest { @Test(expectedExceptions={NoSuchElementException.class}) public void testClassLoaderDirNotFound() throws Exception { - String d = utils.getClassLoaderDir("/somewhere/not/found/XXX.xxx"); + String d = utils.getClassLoaderDir("/somewhere/not/found/ignored.x"); // above should fail log.warn("Uh oh found imaginary resource in: "+d); } diff --git a/karaf/init/src/main/resources/catalog.bom b/karaf/init/src/main/resources/catalog.bom index c606ff1914..30d0836297 100644 --- a/karaf/init/src/main/resources/catalog.bom +++ b/karaf/init/src/main/resources/catalog.bom @@ -158,6 +158,11 @@ brooklyn.catalog: itemType: bean item: type: org.apache.brooklyn.core.workflow.steps.flow.ReturnWorkflowStep + - id: if + format: java-type-name + itemType: bean + item: + type: org.apache.brooklyn.core.workflow.steps.flow.IfWorkflowStep - id: goto format: java-type-name itemType: bean diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynWebServerTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynWebServerTest.java index b1190db633..647a54d8f9 100644 --- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynWebServerTest.java +++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynWebServerTest.java @@ -182,8 +182,8 @@ public class BrooklynWebServerTest { @Test public void verifyHttpsCiphers() throws Exception { brooklynProperties.put(BrooklynWebConfig.HTTPS_REQUIRED, true); - brooklynProperties.put(BrooklynWebConfig.TRANSPORT_PROTOCOLS, "XXX"); - brooklynProperties.put(BrooklynWebConfig.TRANSPORT_CIPHERS, "XXX"); + brooklynProperties.put(BrooklynWebConfig.TRANSPORT_PROTOCOLS, "IGNORED"); + brooklynProperties.put(BrooklynWebConfig.TRANSPORT_CIPHERS, "IGNORED"); try { verifyHttpsFromConfig(brooklynProperties); fail("Expected to fail due to unsupported ciphers during connection negotiation"); diff --git a/parent/pom.xml b/parent/pom.xml index 919f640bff..c7f53fcec5 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -116,7 +116,7 @@ <artifactId>maven-jar-plugin</artifactId> <!-- version 2.4+ seems to break eclipse integration as per https://github.com/tesla/m2eclipse-extras/issues/10 but cannot find issue on GitHub any more - 404 error --> - <!-- XXX if this version is changed, update the MavenArtifactTest --> + <!-- if this version is changed, update the MavenArtifactTest --> <version>2.6</version> </plugin> <plugin>