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 c1fb900595a81b0ee9c0179b49db601305418809 Author: Alex Heneveld <[email protected]> AuthorDate: Sun Oct 23 20:38:14 2022 +0100 unresolveable expressions in conditions are treated as absent not errors --- .../brooklyn/camp/brooklyn/WorkflowYamlTest.java | 32 ++++++++++------- .../core/workflow/WorkflowExecutionContext.java | 2 +- .../workflow/WorkflowExpressionResolution.java | 22 ++++++++++-- .../util/core/predicates/DslPredicates.java | 10 +++++- .../ResolutionFailureTreatedAsAbsent.java | 41 ++++++++++++++++++++++ 5 files changed, 90 insertions(+), 17 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 a77910a9f3..756f6d3efd 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 @@ -68,6 +68,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.function.Predicate; @@ -228,14 +229,15 @@ public class WorkflowYamlTest extends AbstractYamlTest { Duration d3 = Duration.of(sw).subtract(d2); // the next iteration should obey the time constraint specified above if (!timeCheckOrNullIfShouldFail.test(d3)) Asserts.fail("Timing error, took " + d3); + + WorkflowExecutionContext lastWorkflowContext = new WorkflowStatePersistenceViaSensors(mgmt()).getWorkflows(entity).values().iterator().next(); + List<Object> defs = lastWorkflowContext.getStepsDefinition(); + // step definitions should not be resolved by jackson + defs.forEach(def -> Asserts.assertThat(def, d -> !(d instanceof WorkflowStepDefinition))); } else { EntityAsserts.assertAttributeEqualsContinually(entity, s, null); + Asserts.assertThat(new WorkflowStatePersistenceViaSensors(mgmt()).getWorkflows(entity).values(), Collection::isEmpty); } - - WorkflowExecutionContext lastWorkflowContext = new WorkflowStatePersistenceViaSensors(mgmt()).getWorkflows(entity).values().iterator().next(); - List<Object> defs = lastWorkflowContext.getStepsDefinition(); - // step definitions should not be resolved by jackson - defs.forEach(def -> Asserts.assertThat(def, d -> !(d instanceof WorkflowStepDefinition))); } public void doTestWorkflowPolicy(String triggers, Predicate<Duration> timeCheckOrNullIfShouldFail) throws Exception { @@ -551,15 +553,19 @@ public class WorkflowYamlTest extends AbstractYamlTest { e -> Asserts.expectedFailureContainsIgnoreCase(e, "unresolveable", "regex")); } @Test - public void testConditionBadExpression() throws Exception { - // TODO would be nice if it could silently ignore this condition - // TODO also handle multi-line errors (eg from freemarker) - Asserts.assertFailsWith(() -> doTestCondition(Strings.lines( + public void testBadExpressionAllowedInCondition() throws Exception { + Asserts.assertEquals(doTestCondition(Strings.lines( + "any:", + "- target: ${bad_var}", + " when: absent" + )), "expected failure"); + } + @Test + public void testMultilineErrorMessageRegexHandling() throws Exception { + Asserts.assertEquals(doTestCondition(Strings.lines( "any:", - "- regex: .*oh no.*", - "- target: ${bad_var}", - " when: absent")), - e -> Asserts.expectedFailureContainsIgnoreCase(e, "unresolveable", "bad_var")); + "- regex: .*oh no.*" + )), "expected failure"); } Object doTestCondition(String lines) 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 849b980a90..3a4ee58864 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 @@ -223,7 +223,7 @@ public class WorkflowExecutionContext { WorkflowStepResolution.resolveOnErrorSteps(w.getManagementContext(), w.onError); // some fields need to be resolved at setting time, in the context of the workflow - w.setCondition(w.resolveConfig(paramsDefiningWorkflow, WorkflowCommonConfig.CONDITION)); + w.setCondition(w.resolveWrapped(paramsDefiningWorkflow.getStringKey(WorkflowCommonConfig.CONDITION.getName()), WorkflowCommonConfig.CONDITION.getTypeToken())); // finished -- checkpoint noting this has been created but not yet started w.status = WorkflowStatus.STAGED; 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 4f417eb744..a336f437ed 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 @@ -27,6 +27,7 @@ import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils; import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.flags.TypeCoercions; +import org.apache.brooklyn.util.core.predicates.ResolutionFailureTreatedAsAbsent; import org.apache.brooklyn.util.core.task.DeferredSupplier; import org.apache.brooklyn.util.core.text.TemplateProcessor; import org.apache.brooklyn.util.exceptions.Exceptions; @@ -232,10 +233,15 @@ public class WorkflowExpressionResolution { try { result = TemplateProcessor.processTemplateContents("workflow", expression, model, true, false); } catch (Exception e) { + Exception e2 = e; if (!allowWaiting && Exceptions.isCausedByInterruptInAnyThread(e)) { - throw new IllegalArgumentException("Expression value '"+expression+"' unavailable and not permitted to wait: "+ Exceptions.collapseText(e), e); + e2 = new IllegalArgumentException("Expression value '"+expression+"' unavailable and not permitted to wait: "+ Exceptions.collapseText(e), e); + } + if (useWrappedValue) { + // 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(e); + throw Exceptions.propagate(e2); } } finally { if (!allowWaiting) { @@ -265,18 +271,30 @@ public class WorkflowExpressionResolution { public static class WrappedResolvedExpression<T> implements DeferredSupplier<T> { String expression; T value; + Throwable error; public WrappedResolvedExpression() {} public WrappedResolvedExpression(String expression, T value) { this.expression = expression; this.value = value; } + public static WrappedResolvedExpression ofError(String expression, Throwable error) { + WrappedResolvedExpression result = new WrappedResolvedExpression(expression, null); + result.error = error; + return result; + } @Override public T get() { + if (error!=null) { + throw Exceptions.propagate(error); + } return value; } public String getExpression() { return expression; } + public Throwable getError() { + return error; + } } } 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 f8558254ca..0ff98ce03d 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 @@ -706,7 +706,15 @@ public class DslPredicates { ValueResolver<Object> resolver = Tasks.resolving(target).as(Object.class).allowDeepResolution(true).immediately(true); Entity entity = getTypeFromValueOrContext(Entity.class, input); if (entity!=null) resolver = resolver.context(entity); - result = resolver.getMaybe(); + try { + result = resolver.getMaybe(); + } catch (Throwable t) { + if (Exceptions.getCausalChain(t).stream().anyMatch(ti -> ti instanceof ResolutionFailureTreatedAsAbsent)) { + result = Maybe.absent(t); + } else { + throw Exceptions.propagate(t); + } + } } result = result.isPresent() ? super.resolveTargetAgainstInput(result.get()) : result; diff --git a/core/src/main/java/org/apache/brooklyn/util/core/predicates/ResolutionFailureTreatedAsAbsent.java b/core/src/main/java/org/apache/brooklyn/util/core/predicates/ResolutionFailureTreatedAsAbsent.java new file mode 100644 index 0000000000..a3089b297b --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/util/core/predicates/ResolutionFailureTreatedAsAbsent.java @@ -0,0 +1,41 @@ +/* + * 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.util.core.predicates; + +/** marker interface to indicate that a resolution failure should be treated as absent rather than failing immediately */ +public interface ResolutionFailureTreatedAsAbsent { + + class ResolutionFailureTreatedAsAbsentDefaultException extends RuntimeException implements ResolutionFailureTreatedAsAbsent { + public ResolutionFailureTreatedAsAbsentDefaultException() { + } + + public ResolutionFailureTreatedAsAbsentDefaultException(String message) { + super(message); + } + + public ResolutionFailureTreatedAsAbsentDefaultException(String message, Throwable cause) { + super(message, cause); + } + + public ResolutionFailureTreatedAsAbsentDefaultException(Throwable cause) { + super(cause); + } + } + +}
