You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2024/02/02 14:48:05 UTC

(brooklyn-server) branch master updated (f2a35dc6ee -> deefd6cf7c)

This is an automated email from the ASF dual-hosted git repository.

heneveld pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git


    from f2a35dc6ee Change version to 1.2.0-SNAPSHOT
     new fc9009d072 log handled workflow errors and explicit fail steps as debug not warn
     new deefd6cf7c support boolean values and suppliers being set for dsl predicate / conditions

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../core/workflow/WorkflowErrorHandling.java       | 10 +++++-
 .../util/core/predicates/DslPredicates.java        | 10 ++++++
 .../workflow/WorkflowPersistReplayErrorsTest.java  | 42 +++++++++++++++-------
 .../util/core/predicates/DslPredicateTest.java     | 41 +++++++++++++++++++++
 4 files changed, 89 insertions(+), 14 deletions(-)


(brooklyn-server) 02/02: support boolean values and suppliers being set for dsl predicate / conditions

Posted by he...@apache.org.
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 deefd6cf7cc3ca0227eebe89c1b2aecbfdc74c7f
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Feb 2 14:47:42 2024 +0000

    support boolean values and suppliers being set for dsl predicate / conditions
---
 .../util/core/predicates/DslPredicates.java        | 10 ++++++
 .../util/core/predicates/DslPredicateTest.java     | 41 ++++++++++++++++++++++
 2 files changed, 51 insertions(+)

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 76efad2c9b..d185c7a3ff 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
@@ -93,6 +93,7 @@ public class DslPredicates {
 
         // could use json shorthand instead, but this is simpler
         TypeCoercions.registerAdapter(String.class, DslPredicate.class, DslPredicates::implicitlyEqualTo);
+        TypeCoercions.registerAdapter(Boolean.class, DslPredicate.class, DslPredicates::always);
 
 //        TypeCoercions.registerAdapter(DeferredSupplier.class, DslPredicate.class, DslPredicates::implicitlyEqualTo);
 //        TypeCoercions.registerAdapter(WorkflowExpressionResolution.WrappedUnresolvedExpression.class, DslPredicate.class, DslPredicates::implicitlyEqualTo);
@@ -503,6 +504,11 @@ public class DslPredicates {
                     return nestedPredicateCheck((DslPredicate) test, result);
                 }
 
+                if (test instanceof Boolean) {
+                    // if a boolean is supplied as an implicit, return it; it was probably a condition
+                    return (Boolean) test;
+                }
+
                 if ((!(test instanceof BrooklynObject) && value instanceof BrooklynObject) ||
                         (!(test instanceof Iterable) && value instanceof Iterable)) {
                     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.");
@@ -1057,6 +1063,10 @@ public class DslPredicates {
         return result;
     }
 
+    public static DslPredicate always(boolean x) {
+        return x ? alwaysTrue() : alwaysFalse();
+    }
+
     public static DslPredicate equalTo(Object x) {
         DslEntityPredicateDefault result = new DslEntityPredicateDefault();
         result.equals = WrappedValue.of(x);
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 016059ebbd..a043064db3 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
@@ -26,12 +26,15 @@ import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import org.apache.brooklyn.util.core.predicates.DslPredicates.DslPredicate;
+import org.apache.brooklyn.util.core.task.DeferredSupplier;
+import org.apache.brooklyn.util.javalang.AtomicReferences;
 import org.apache.brooklyn.util.time.Duration;
 import org.apache.brooklyn.util.time.Time;
 import org.testng.annotations.Test;
 
 import java.util.Arrays;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
@@ -609,4 +612,42 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
         Asserts.assertFailsWith(() -> { p2.test(MutableList.of("x", "zz")); }, e -> Asserts.expectedFailureContainsIgnoreCase(e, "assert", "value", "zz"));
     }
 
+    @Test
+    public void testCoercionAndSuppliers() {
+        DslPredicates.DslPredicate p;
+
+        p = TypeCoercions.coerce("yessir", DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test("yessir"));
+        Asserts.assertFalse(p.test("no sir"));
+
+        p = TypeCoercions.coerce( (DeferredSupplier) ()->"yessir", DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test("yessir"));
+        Asserts.assertFalse(p.test("no sir"));
+
+        AtomicReference fickle = new AtomicReference("yessir");
+        p = TypeCoercions.coerce( (DeferredSupplier) ()->fickle.get(), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test("yessir"));
+        Asserts.assertFalse(p.test("no sir"));
+        fickle.set("no sir");
+        Asserts.assertFalse(p.test("yessir"));
+        Asserts.assertTrue(p.test("no sir"));
+
+        // now boolean, especially atomic boolean
+        Asserts.assertTrue(TypeCoercions.coerce(true, DslPredicates.DslPredicate.class).test("anything"));
+        Asserts.assertFalse(TypeCoercions.coerce(false, DslPredicates.DslPredicate.class).test("anything"));
+
+        fickle.set(true);
+        Asserts.assertTrue(p.test(true));
+        Asserts.assertTrue(p.test("anything"));
+        Asserts.assertTrue(p.test(false));
+
+        fickle.set(false);
+        Asserts.assertFalse(p.test(true));
+        Asserts.assertFalse(p.test("anything"));
+        Asserts.assertFalse(p.test(false));
+
+        p = TypeCoercions.coerce( (DeferredSupplier) ()->false, DslPredicates.DslPredicate.class);
+        Asserts.assertFalse(p.test("anything"));
+    }
+
 }


(brooklyn-server) 01/02: log handled workflow errors and explicit fail steps as debug not warn

Posted by he...@apache.org.
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 fc9009d0724ff1ac25f504f6b33560bc6893a704
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri Feb 2 13:14:17 2024 +0000

    log handled workflow errors and explicit fail steps as debug not warn
---
 .../core/workflow/WorkflowErrorHandling.java       | 10 +++++-
 .../workflow/WorkflowPersistReplayErrorsTest.java  | 42 +++++++++++++++-------
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
index ea39a0d1ff..e93c30dd4d 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowErrorHandling.java
@@ -23,6 +23,7 @@ import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.Task;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
+import org.apache.brooklyn.core.workflow.steps.flow.FailWorkflowStep.WorkflowFailException;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.core.predicates.DslPredicates;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
@@ -242,6 +243,9 @@ public class WorkflowErrorHandling implements Callable<WorkflowErrorHandling.Wor
             } catch (Exception e2) {
                 if (Exceptions.getCausalChain(e2).stream().anyMatch(e3 -> e3== error)) {
                     // is, or wraps, original error, don't need to log
+                } else if (e2 instanceof WorkflowFailException) {
+                    log.debug("Workflow fail in step '"+stepTaskThrowingError.getDisplayName()+"'; new error -- "+Exceptions.collapseText(e2)+" -- replacing original error: "+Exceptions.collapseText(error));
+                    log.trace("Full trace of original error was: " + error, error);
                 } else {
                     logWarnOnExceptionOrDebugIfKnown(entity, e2, "Error in step '" + stepTaskThrowingError.getDisplayName() + "' error handler for -- " + Exceptions.collapseText(error) + " -- threw another error (rethrowing): " + Exceptions.collapseText(e2));
                     log.debug("Full trace of original error was: " + error, error);
@@ -257,9 +261,13 @@ public class WorkflowErrorHandling implements Callable<WorkflowErrorHandling.Wor
 
         // don't consider replaying automatically here; only done at workflow level
 
-        logWarnOnExceptionOrDebugIfKnown(entity, error,
+        // previously logged as warning, but author has done everything right so we shouldn't warn, just debug
+//        logWarnOnExceptionOrDebugIfKnown(entity, error,
+        log.debug(
                 currentStepInstance.getWorkflowExectionContext().getName() + ": " +
                 "Error in step '" + stepTaskThrowingError.getDisplayName() + "'; " + problemHere + "rethrowing: " + Exceptions.collapseText(error));
+        log.trace("Trace for error being rethrown", error);
+
         throw Exceptions.propagate(error);
     }
 
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
index 88962ea712..e0635e174a 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
@@ -18,6 +18,8 @@
  */
 package org.apache.brooklyn.core.workflow;
 
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.spi.ILoggingEvent;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.Iterables;
 import org.apache.brooklyn.api.entity.EntityLocal;
@@ -648,18 +650,32 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl
 
     @Test
     public void testSimpleErrorHandlerOnWorkflowFailing() throws IOException {
-        lastInvocation = runSteps(MutableList.of("invoke-effector does-not-exist"),
-                null,
-                ConfigBag.newInstance().configure(
-                        WorkflowEffector.ON_ERROR, MutableList.of("set-sensor had_error = yes", "fail rethrow message rethrown")) );
-        Asserts.assertFailsWith(() -> lastInvocation.getUnchecked(), e -> {
-            Asserts.expectedFailureContains(e, "rethrown");
-            Asserts.assertThat(Exceptions.getCausalChain(e), ee -> ee.stream().filter(e2 -> e2.toString().contains("does-not-exist")).findAny().isPresent());
-            return true;
-        });
-        Asserts.assertEquals(app.sensors().get(Sensors.newSensor(Object.class, "had_error")), "yes");
-        findSingleLastWorkflow();
-        Asserts.assertEquals(lastWorkflowContext.status, WorkflowExecutionContext.WorkflowStatus.ERROR);
+        try (ClassLogWatcher logWatcher = new ClassLogWatcher(getClass().getPackage().getName()) {
+            @Override
+            protected void append(ILoggingEvent iLoggingEvent) {
+                if (iLoggingEvent.getLevel().isGreaterOrEqual(Level.INFO)) super.append(iLoggingEvent);
+            }
+        }) {
+            lastInvocation = runSteps(MutableList.of("invoke-effector does-not-exist"),
+                    null,
+                    ConfigBag.newInstance().configure(
+                            WorkflowEffector.ON_ERROR, MutableList.of("set-sensor had_error = yes", "fail rethrow message rethrown")));
+            Asserts.assertFailsWith(() -> lastInvocation.getUnchecked(), e -> {
+                Asserts.expectedFailureContains(e, "rethrown");
+                Asserts.assertThat(Exceptions.getCausalChain(e), ee -> ee.stream().filter(e2 -> e2.toString().contains("does-not-exist")).findAny().isPresent());
+                return true;
+            });
+
+            List<String> msgs = logWatcher.getMessages();
+            if (!msgs.isEmpty()) {
+                log.info("Error handler output:\n" + msgs.stream().collect(Collectors.joining("\n")));
+                Asserts.fail("No info/warn log messages expected if workflow has error handler, even if it runs an explicit fail rethrow step");
+            }
+
+            Asserts.assertEquals(app.sensors().get(Sensors.newSensor(Object.class, "had_error")), "yes");
+            findSingleLastWorkflow();
+            Asserts.assertEquals(lastWorkflowContext.status, WorkflowExecutionContext.WorkflowStatus.ERROR);
+        }
     }
 
     @Test
@@ -705,7 +721,7 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl
     }
 
     @Test
-    public void testErrorHandlerRethrows() throws IOException {
+    public void testErrorHandlerRethrowsIfConditionNotMatched() throws IOException {
         lastInvocation = runSteps(MutableList.of(
                         MutableMap.of("step", "fail message expected exception",
                                 "output", "should have failed",