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/03/21 13:48:53 UTC

(brooklyn-server) branch master updated (2bca22a8fa -> b47cbd8107)

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 2bca22a8fa make more context for workflow step resolution
     new 8975991af7 allow if workflow step to take condition objects
     new 1a2c5510b6 make better subworkflow name and store on tag for easier navigation
     new fa635a16d5 better naming for switch step sub-tasks
     new b47cbd8107 better checking for string/primitive, and better error for workflow input

The 4 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:
 .../org/apache/brooklyn/core/config/Sanitizer.java |  2 +-
 .../brooklyn/core/mgmt/BrooklynTaskTags.java       |  2 +
 .../mgmt/internal/AbstractManagementContext.java   |  2 +-
 .../core/resolve/jackson/BeanWithTypeUtils.java    | 10 ++-
 .../WorkflowStepInstanceExecutionContext.java      | 11 +++-
 .../core/workflow/WorkflowStepResolution.java      |  4 +-
 .../core/workflow/steps/CustomWorkflowStep.java    | 75 ++++++++++++++--------
 .../workflow/steps/external/HttpWorkflowStep.java  |  5 +-
 .../core/workflow/steps/flow/IfWorkflowStep.java   | 58 +++++++++++++----
 .../workflow/steps/flow/SwitchWorkflowStep.java    | 27 +++++---
 .../workflow/steps/variables/TransformJoin.java    |  4 +-
 .../steps/variables/TransformPrependAppend.java    |  4 +-
 .../brooklyn/util/core/flags/TypeCoercions.java    |  2 +-
 .../workflow/WorkflowPersistReplayErrorsTest.java  |  2 +-
 .../WorkflowSubIfAndCustomExtensionEdgeTest.java   | 18 +++++-
 .../tasks/kubectl/ContainerWorkflowStep.java       |  3 +-
 .../apache/brooklyn/util/collections/Jsonya.java   |  5 +-
 .../org/apache/brooklyn/util/javalang/Boxing.java  |  3 +
 18 files changed, 169 insertions(+), 68 deletions(-)


(brooklyn-server) 03/04: better naming for switch step sub-tasks

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 fa635a16d57595b8f4b74b8952d4365ef72b98c5
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Wed Mar 20 16:22:23 2024 +0000

    better naming for switch step sub-tasks
---
 .../core/workflow/WorkflowStepResolution.java      |  4 +++-
 .../workflow/steps/flow/SwitchWorkflowStep.java    | 27 ++++++++++++++--------
 2 files changed, 21 insertions(+), 10 deletions(-)

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 cbcff72f17..e356ee1a56 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
@@ -30,6 +30,7 @@ import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext;
 import org.apache.brooklyn.api.objs.BrooklynObject;
 import org.apache.brooklyn.core.entity.Entities;
 import org.apache.brooklyn.core.entity.EntityAdjuncts.EntityAdjunctProxyable;
+import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.mgmt.internal.AbstractManagementContext;
 import org.apache.brooklyn.core.objs.BrooklynObjectInternal;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
@@ -228,7 +229,8 @@ public class WorkflowStepResolution {
                 def = converter.call();
             } else {
                 // run in a task context if we can, to facilitate conversion and type lookup
-                def = Entities.submit(entity, Tasks.create("convert steps", converter)).getUnchecked();
+                def = Entities.submit(entity,
+                        Tasks.builder().displayName("convert steps").body(converter).tag(BrooklynTaskTags.TRANSIENT_TASK_TAG).build()).getUnchecked();
             }
 
             if (def instanceof WorkflowStepDefinition.WorkflowStepDefinitionWithSpecialDeserialization) {
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/SwitchWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/SwitchWorkflowStep.java
index 42598bf8f0..a1e7831329 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/SwitchWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/SwitchWorkflowStep.java
@@ -100,35 +100,44 @@ public class SwitchWorkflowStep extends WorkflowStepDefinition implements Workfl
         for (int i = 0; i<stepsResolved.size(); i++) {
             // go through steps, find first that matches
 
+            String potentialTaskName = Tasks.current().getDisplayName()+"-"+(i+1);
+
             WorkflowStepDefinition subStep = stepsResolved.get(i);
+            String name = subStep.getName();
+            if (Strings.isBlank(name)) {
+                subStep.setName(potentialTaskName);
+                name = subStep.getName();
+                potentialTaskName = null;
+            }
 
             WorkflowStepInstanceExecutionContext subStepContext = new WorkflowStepInstanceExecutionContext(
                 /** use same step index */ context.getStepIndex(), subStep, context.getWorkflowExectionContext());
 
-            // might want to record sub-step context somewhere; but for now we don't
-
-            String potentialTaskName = Tasks.current().getDisplayName()+"-"+(i+1);
+            // sub-step context gets recorded in selectedStepContext,
+            // and promoted and used by the UI
 
             DslPredicates.DslPredicate condition = subStep.getConditionResolved(subStepContext);
 
             if (condition!=null) {
-                if (log.isTraceEnabled()) log.trace("Considering condition " + condition + " for " + potentialTaskName);
+                if (log.isTraceEnabled()) log.trace("Considering condition " + condition + " for " + name);
                 boolean conditionMet = DslPredicates.evaluateDslPredicateWithBrooklynObjectContext(condition, valueResolved, subStepContext.getEntity());
-                if (log.isTraceEnabled()) log.trace("Considered condition " + condition + " for " + potentialTaskName + ": " + conditionMet);
+                if (log.isTraceEnabled()) log.trace("Considered condition " + condition + " for " + name + ": " + conditionMet);
                 if (!conditionMet) continue;
             }
 
             setStepState(context, true, subStep, subStepContext);  // persist this, so when we resume we can pick up the same one
+            // provide some details of other step
+            name = subStepContext.getName();
+            context.noteOtherMetadata("Switch match", "Case "+(i+1)+(Strings.isNonBlank(name) && Strings.isNonBlank(potentialTaskName) ? ": "+name : ""));
+
             Task<?> handlerI = subStep.newTaskAsSubTask(subStepContext,
-                    potentialTaskName, BrooklynTaskTags.tagForWorkflowSubStep(context, i));
+                    null, BrooklynTaskTags.tagForWorkflowSubStep(context, i));
 
-            log.debug("Switch matched at substep "+i+", running " + potentialTaskName + " '" + subStep.computeName(subStepContext, false)+"' in task "+handlerI.getId());
+            log.debug("Switch matched at substep "+i+", running " + (potentialTaskName!=null ? potentialTaskName : "case") + " '" + subStep.computeName(subStepContext, false)+"' in task "+handlerI.getId());
 
             Object result = DynamicTasks.queue(handlerI).getUnchecked();
             context.next = WorkflowReplayUtils.getNext(subStepContext, subStep, context, this);
 
-            // provide some details of other step
-            context.noteOtherMetadata("Switch match", "Case "+(i+1)+": "+Strings.firstNonBlank(subStepContext.getName(), subStepContext.getWorkflowStepReference()));
             context.otherMetadata.putAll(subStepContext.otherMetadata);
 
             return result;


(brooklyn-server) 02/04: make better subworkflow name and store on tag for easier navigation

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 1a2c5510b6f7253169405cb5e87ee0fee68a41aa
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Mon Mar 18 08:49:54 2024 +0000

    make better subworkflow name and store on tag for easier navigation
---
 .../brooklyn/core/mgmt/BrooklynTaskTags.java       |  2 +
 .../core/workflow/steps/CustomWorkflowStep.java    | 75 ++++++++++++++--------
 2 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
index 950713a6d1..52086f44c3 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
@@ -479,6 +479,7 @@ public class BrooklynTaskTags extends TaskTags {
         protected String applicationId;
         protected String entityId;
         protected String workflowId;
+        protected String workflowName;
 
         protected Integer stepIndex;
         // TODO handle these in the UI:
@@ -536,6 +537,7 @@ public class BrooklynTaskTags extends TaskTags {
         t.applicationId = workflow.getEntity().getApplicationId();
         t.entityId = workflow.getEntity().getId();
         t.workflowId = workflow.getWorkflowId();
+        t.workflowName = workflow.getName();
         return t;
     }
     public static WorkflowTaskTag tagForWorkflow(WorkflowStepInstanceExecutionContext workflowStep) {
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 ddfc6d9f17..d61cf7bd8a 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
@@ -18,6 +18,15 @@
  */
 package org.apache.brooklyn.core.workflow.steps;
 
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Collectors;
+import javax.annotation.Nonnull;
+
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.core.JsonProcessingException;
@@ -38,7 +47,13 @@ import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.resolve.jackson.JsonPassThroughDeserializer;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
-import org.apache.brooklyn.core.workflow.*;
+import org.apache.brooklyn.core.workflow.WorkflowCommonConfig;
+import org.apache.brooklyn.core.workflow.WorkflowExecutionContext;
+import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
+import org.apache.brooklyn.core.workflow.WorkflowReplayUtils;
+import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
+import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
+import org.apache.brooklyn.core.workflow.WorkflowStepResolution;
 import org.apache.brooklyn.core.workflow.utils.WorkflowConcurrencyParser;
 import org.apache.brooklyn.core.workflow.utils.WorkflowRetentionParser;
 import org.apache.brooklyn.util.collections.MutableList;
@@ -56,16 +71,6 @@ import org.apache.commons.lang3.tuple.Pair;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.annotation.Nonnull;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
-import java.util.concurrent.atomic.AtomicInteger;
-import java.util.function.Supplier;
-import java.util.stream.Collectors;
-
 public class CustomWorkflowStep extends WorkflowStepDefinition implements WorkflowStepDefinition.WorkflowStepDefinitionWithSpecialDeserialization, WorkflowStepDefinition.WorkflowStepDefinitionWithSubWorkflow {
 
     private static final Logger LOG = LoggerFactory.getLogger(CustomWorkflowStep.class);
@@ -136,7 +141,7 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl
             if (SHORTHAND_TYPE_NAME_DEFAULT.equals(shorthandTypeName)) {
                 shorthand = WORKFLOW_SETTING_SHORTHAND;
             } else {
-                throw new IllegalStateException("Shorthand not supported for " + getNameOrDefault());
+                throw new IllegalStateException("Shorthand not supported for " + getStepTypeName());
             }
         }
         if (input==null) input = MutableMap.of();
@@ -258,7 +263,6 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl
         boolean wasList = targetR instanceof Iterable;
 
         targetR = checkTarget(targetR);
-
         Map reducingV = initializeReducingVariables(context, reducing);
 
         AtomicInteger index = new AtomicInteger(0);
@@ -504,11 +508,36 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl
         throw new IllegalArgumentException("Invalid target '"+target+"'; if a string, it must match a known keyword ('children' or 'members') or pattern (a range, '1..10')");
     }
 
-    public String getNameOrDefault() {
-        return getNameOrDefault(() -> Strings.isNonBlank(shorthandTypeName) ? shorthandTypeName : "custom step");
-    }
-    public String getNameOrDefault(Supplier<String> defaultSupplier) {
-        return Strings.isNonBlank(getName()) ? getName() : defaultSupplier==null ? null : defaultSupplier.get();
+    private static final String FALLBACK_STEP_TYPE_NAME = "custom step";
+    @JsonIgnore
+    protected String getStepTypeName() {
+        return Strings.isNonBlank(shorthandTypeName) ? shorthandTypeName : FALLBACK_STEP_TYPE_NAME;
+    }
+    public String getSubworkflowName(Object target, Integer targetIndexOrNullIfNotList) {
+        // old subworkflow name
+//        String indexName = targetIndexOrNull==null ? "" : " "+(targetIndexOrNull+1);
+//        String name = getName();
+//        name = (name == null ? "Sub-workflow" + indexName : "Sub-workflow"+indexName+" for " + name);
+//        String targetString = target==null ? null : target.toString();
+//        if (targetString!=null && targetString.length()<60 && !Strings.isMultiLine(targetString)) name += " ("+targetString+")";
+//        return name;
+
+        // now:
+        String name = getName();
+        if (Strings.isBlank(name) || name.length()>30) name = getStepTypeName();
+        String targetString = target==null ? null :
+                (target instanceof BrooklynObject) ? ((BrooklynObject)target).getDisplayName()
+                : target.toString();
+        if (targetIndexOrNullIfNotList!=null) {
+            name = name + " ("+(targetIndexOrNullIfNotList+1)+")";
+        }
+        if (targetString!=null && targetString.length()<30) {
+            name = name + ": "+targetString;
+        } else if (targetIndexOrNullIfNotList==null) {
+            // need to say something to distinguish from parent
+            name = name + " (sub-workflow)";
+        }
+        return name;
     }
 
     @Override
@@ -572,14 +601,10 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl
         return typeBestGuess!=null && !shorthandDefault.equals(typeBestGuess) && !clazz.getName().equals(typeBestGuess);
     }
 
-    protected WorkflowExecutionContext newWorkflow(WorkflowStepInstanceExecutionContext context, Object target, Integer targetIndexOrNull) {
+    protected WorkflowExecutionContext newWorkflow(WorkflowStepInstanceExecutionContext context, Object target, Integer targetIndexOrNullIfNotList) {
         if (steps==null) throw new IllegalArgumentException("Cannot make new workflow with no steps");
 
-        String indexName = targetIndexOrNull==null ? "" : " "+(targetIndexOrNull+1);
-        String name = getNameOrDefault(null);
-        name = (name == null ? "Sub-workflow" + indexName : "Sub-workflow"+indexName+" for " + name);
-        String targetString = target==null ? null : target.toString();
-        if (targetString!=null && targetString.length()<60 && !Strings.isMultiLine(targetString)) name += " ("+targetString+")";
+        String name = getSubworkflowName(target, targetIndexOrNullIfNotList);
 
         WorkflowExecutionContext nestedWorkflowContext = WorkflowExecutionContext.newInstanceUnpersistedWithParent(
                 target instanceof BrooklynObject ? (BrooklynObject) target : context.getEntity(), context.getWorkflowExectionContext(),
@@ -590,7 +615,7 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl
 
 
         String tivn = context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, target_index_var_name, String.class);
-        if (targetIndexOrNull!=null) nestedWorkflowContext.updateWorkflowScratchVariable(tivn == null ? TARGET_INDEX_VAR_NAME_DEFAULT : tivn, targetIndexOrNull);
+        if (targetIndexOrNullIfNotList!=null) nestedWorkflowContext.updateWorkflowScratchVariable(tivn == null ? TARGET_INDEX_VAR_NAME_DEFAULT : tivn, targetIndexOrNullIfNotList);
         initializeSubWorkflowForTarget(context, target, nestedWorkflowContext);
 
         return nestedWorkflowContext;


(brooklyn-server) 04/04: better checking for string/primitive, and better error for workflow input

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 b47cbd8107454373ca18002f1697b05a03fdeb52
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Mar 21 13:48:25 2024 +0000

    better checking for string/primitive, and better error for workflow input
---
 .../main/java/org/apache/brooklyn/core/config/Sanitizer.java  |  2 +-
 .../core/mgmt/internal/AbstractManagementContext.java         |  2 +-
 .../brooklyn/core/resolve/jackson/BeanWithTypeUtils.java      | 10 ++++------
 .../core/workflow/WorkflowStepInstanceExecutionContext.java   | 11 +++++++++--
 .../core/workflow/steps/external/HttpWorkflowStep.java        |  5 ++++-
 .../brooklyn/core/workflow/steps/variables/TransformJoin.java |  4 ++--
 .../core/workflow/steps/variables/TransformPrependAppend.java |  4 ++--
 .../org/apache/brooklyn/util/core/flags/TypeCoercions.java    |  2 +-
 .../core/workflow/WorkflowPersistReplayErrorsTest.java        |  2 +-
 .../apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java  |  3 +--
 .../java/org/apache/brooklyn/util/collections/Jsonya.java     |  5 ++++-
 .../main/java/org/apache/brooklyn/util/javalang/Boxing.java   |  3 +++
 12 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java b/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java
index 4e7fa7b614..e79c220c52 100644
--- a/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java
+++ b/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java
@@ -176,7 +176,7 @@ public final class Sanitizer {
     }
 
     public static String suppressJson(Object value, boolean excludeBrooklynDslExpressions) {
-        if (value==null || Boxing.isPrimitiveOrBoxedObject(value) || value instanceof CharSequence) {
+        if (value==null || Boxing.isPrimitiveOrStringOrBoxedObject(value) || value instanceof CharSequence) {
             if (excludeBrooklynDslExpressions && value instanceof String && ((String)value).startsWith("$brooklyn:")) return (String)value;
             return suppress(value);
         }
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
index 722f6040c5..7d569dc146 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java
@@ -566,7 +566,7 @@ public abstract class AbstractManagementContext implements ManagementContextInte
         String entityId;
         if (entityO instanceof Entity) {
             entity = (Entity) entityO;
-        } else if (entityO instanceof String || Boxing.isPrimitiveOrBoxedObject(entityO)) {
+        } else if (Boxing.isPrimitiveOrStringOrBoxedObject(entityO)) {
             entityId = entityO.toString();
 
             List<Entity> firstGroupOfMatches = AppGroupTraverser.findFirstGroupOfMatches(contextEntity, true,
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 9b2a6dde2b..2c85657c93 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
@@ -120,9 +120,7 @@ public class BeanWithTypeUtils {
         return isJsonAndOthers(o, oo -> false);
     }
     public static boolean isJsonAndOthers(Object o, Predicate<Object> acceptOthers) {
-        if (o instanceof String) return true;
-        if (o==null) return true;
-        if (Boxing.isPrimitiveOrBoxedObject(o)) return true;
+        if (o==null || Boxing.isPrimitiveOrStringOrBoxedObject(o)) return true;
         if (o instanceof Collection) {
             for (Object oo : (Collection<?>) o) {
                 if (!isJsonAndOthers(oo, acceptOthers)) return false;
@@ -222,7 +220,7 @@ public class BeanWithTypeUtils {
         boolean useLonghandObjectWriter = true;
 
         if (type.getRawType().equals(Object.class)) useLonghandObjectWriter = false;
-        else if (mapOrListToSerializeThenDeserialize==null || mapOrListToSerializeThenDeserialize instanceof String || Boxing.isPrimitiveOrBoxedObject(mapOrListToSerializeThenDeserialize)) useLonghandObjectWriter = false;
+        else if (mapOrListToSerializeThenDeserialize==null || Boxing.isPrimitiveOrStringOrBoxedObject(mapOrListToSerializeThenDeserialize)) useLonghandObjectWriter = false;
 
         String serialization = !useLonghandObjectWriter ? mapper.writeValueAsString(mapOrListToSerializeThenDeserialize) : mapper.writerFor(Object.class).writeValueAsString(mapOrListToSerializeThenDeserialize);
         return mapper.readValue(serialization, BrooklynJacksonType.asJavaType(mapper, type));
@@ -245,7 +243,7 @@ public class BeanWithTypeUtils {
         if (inputMap.isAbsent()) return (Maybe<T>)inputMap;
 
         Object o = inputMap.get();
-        if (!(o instanceof Map) && !(o instanceof List) && !Boxing.isPrimitiveOrBoxedObject(o) && !(o instanceof String)) {
+        if (!(o instanceof Map) && !(o instanceof List) && !Boxing.isPrimitiveOrStringOrBoxedObject(o)) {
             if (type.isSupertypeOf(o.getClass())) {
                 return (Maybe<T>)inputMap;
             }  else {
@@ -337,6 +335,6 @@ public class BeanWithTypeUtils {
         }
 
         // we want some special object. if we have a map or a string or possibly a primitive then conversion might sort us out.
-        return (t instanceof Map || t instanceof String || Boxing.isPrimitiveOrBoxedObject(t)) ? 1 : -1;
+        return (t instanceof Map || Boxing.isPrimitiveOrStringOrBoxedObject(t)) ? 1 : -1;
     }
 }
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 e1c6df8316..3ede6cc447 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
@@ -33,6 +33,7 @@ import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.exceptions.LossySerializingThrowable;
+import org.apache.brooklyn.util.javalang.Boxing;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -179,8 +180,14 @@ public class WorkflowStepInstanceExecutionContext {
         if (inputResolved.containsKey(key)) return (T)inputResolved.get(key);
 
         Object v = input.get(key);
-        T v2 = WorkflowExpressionResolution.allowingRecursionWhenSetting(context, WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, key,
-                () -> context.resolve(stage, v, type));
+        T v2;
+        try {
+            v2 = WorkflowExpressionResolution.allowingRecursionWhenSetting(context, WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, key,
+                    () -> context.resolve(stage, v, type));
+        } catch (Exception e) {
+            throw Exceptions.propagateAnnotated("Cannot resolve input "+
+                    (Boxing.isPrimitiveOrStringOrBoxedObject(v) ? "'"+v+"'" : v.getClass().getName() + " ("+v+")"), e);
+        }
         if (REMEMBER_RESOLVED_INPUT) {
             if (!Objects.equals(v, v2)) {
                 inputResolved.put(key, v2);
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 b262f1efb2..fa47370772 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
@@ -46,6 +46,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.charset.Charset;
 import java.util.Map;
@@ -103,7 +104,9 @@ public class HttpWorkflowStep extends WorkflowStepDefinition {
                 new ShellEnvironmentSerializer(context.getManagementContext()).serialize(params)
                     .forEach((k, v) -> urib.addParameter(k, v));
             }
-            httpb.uri(urib.build());
+            URI uri = urib.build();
+            httpb.uri(uri);
+            context.noteOtherMetadata("URL", uri.toString());
         } catch (URISyntaxException e) {
             throw Exceptions.propagateAnnotated("Invalid URI: "+endpoint, e);
         }
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformJoin.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformJoin.java
index b14e8b77d9..885e45185b 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformJoin.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformJoin.java
@@ -40,14 +40,14 @@ public class TransformJoin extends WorkflowTransformDefault {
     @Override
     public Object apply(Object v) {
         Object separatorResolvedO = separator==null ? "" : context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, separator, Object.class);
-        if (!(separatorResolvedO instanceof String || Boxing.isPrimitiveOrBoxedObject(separatorResolvedO))) {
+        if (!(Boxing.isPrimitiveOrStringOrBoxedObject(separatorResolvedO))) {
             throw new IllegalStateException("Argument must be a string or primitive to use as the separator");
         }
         String separatorResolved = ""+separatorResolvedO;
         if (v instanceof Iterable) {
             List list = MutableList.copyOf((Iterable)v);
             return list.stream().map(x -> {
-                if (!(x instanceof String || Boxing.isPrimitiveOrBoxedObject(x))) {
+                if (!(Boxing.isPrimitiveOrStringOrBoxedObject(x))) {
                     throw new IllegalStateException("Elements in the list to join must be a strings or primitives");
                 }
                 return ""+x;
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformPrependAppend.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformPrependAppend.java
index 5c06ee9e58..522c750f58 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformPrependAppend.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformPrependAppend.java
@@ -44,8 +44,8 @@ public class TransformPrependAppend extends WorkflowTransformDefault {
     @Override
     public Object apply(Object v) {
         Object expressionResolved = context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, expression, Object.class);
-        if (v instanceof String || Boxing.isPrimitiveOrBoxedObject(v)) {
-            if (!(expressionResolved instanceof String || Boxing.isPrimitiveOrBoxedObject(expressionResolved))) {
+        if (Boxing.isPrimitiveOrStringOrBoxedObject(v)) {
+            if (!(Boxing.isPrimitiveOrStringOrBoxedObject(expressionResolved))) {
                 throw new IllegalStateException("Argument must be a string or primitive to prepend/append");
             }
             if (atEnd) return Strings.toString(v) + expressionResolved;
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
index b93ac1b986..3840c0b838 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
@@ -361,7 +361,7 @@ public class TypeCoercions {
 
                 @Override
                 public <T> Maybe<T> tryCoerce(Object input, TypeToken<T> type) {
-                    if (!(input instanceof Map || input instanceof Collection || Boxing.isPrimitiveOrBoxedObject(input) || input instanceof String)) {
+                    if (!(input instanceof Map || input instanceof Collection || Boxing.isPrimitiveOrStringOrBoxedObject(input))) {
                         return null;
                     }
                     if (BeanWithTypeUtils.isConversionRecommended(Maybe.of(input), type)) {
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 73e242de5f..32a0328970 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
@@ -742,7 +742,7 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl
             List<String> msgs = logWatcher.getMessages();
             log.info("Error handler output:\n"+msgs.stream().collect(Collectors.joining("\n")));
             Asserts.assertEquals(msgs.stream().filter(s -> s.contains("NOT")).findAny().orElse(null), null);
-            Asserts.assertEquals(msgs.stream().filter(s -> s.contains("created-but-not-logged") && !s.contains("Creating handler")).findAny().orElse(null), null);
+            Asserts.assertEquals(msgs.stream().filter(s -> s.contains("created-but-not-logged") && !s.contains("error-handler")).findAny().orElse(null), null);
             Asserts.assertNotNull(msgs.stream().filter(s -> s.contains("1-1")).findAny().orElse(null));
             Asserts.assertNotNull(msgs.stream().filter(s -> s.contains("1-2")).findAny().orElse(null));
             Asserts.assertNotNull(msgs.stream().filter(s -> s.contains("1-4")).findAny().orElse(null));
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 1d5938f4ab..56299751fe 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
@@ -103,8 +103,7 @@ public class ContainerWorkflowStep extends WorkflowStepDefinition {
         } else if (args instanceof Collection) {
             List<String> result = MutableList.of();
             ((Collection)args).forEach(x -> {
-                if (x instanceof String) result.add((String)x);
-                else if (Boxing.isPrimitiveOrBoxedObject(x)) result.add(x.toString());
+                if (Boxing.isPrimitiveOrStringOrBoxedObject(x)) result.add(x.toString());
                 else throw new IllegalArgumentException("Argument '"+x+"' not supported; should be a string");
             });
             tf.arguments(result);
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/collections/Jsonya.java b/utils/common/src/main/java/org/apache/brooklyn/util/collections/Jsonya.java
index b79d5ca0ae..e3a84f033d 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/collections/Jsonya.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/collections/Jsonya.java
@@ -114,9 +114,12 @@ public class Jsonya {
         if (x==null) return true;
         return x instanceof Map || x instanceof Collection || x instanceof String || Boxing.isPrimitiveOrBoxedObject(x);
     }
+    public static boolean isJsonPrimitive(Object x) {
+        return Boxing.isPrimitiveOrStringOrBoxedObject(x);
+    }
     public static boolean isJsonPrimitiveDeep(Object x) {
         if (x==null) return true;
-        if (x instanceof String || Boxing.isPrimitiveOrBoxedObject(x)) return true;
+        if (isJsonPrimitive(x)) return true;
         if (x instanceof Map) return !((Map<?,?>)x).entrySet().stream().anyMatch(ent -> !isJsonPrimitiveDeep(ent.getKey()) || !isJsonPrimitiveDeep(ent.getValue()));
         if (x instanceof Collection) return !((Collection)x).stream().anyMatch(ent -> !isJsonPrimitiveDeep(ent));
         return false;
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/Boxing.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/Boxing.java
index 7079fc2167..eda50850c0 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/Boxing.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/Boxing.java
@@ -93,6 +93,9 @@ public class Boxing {
         if (o==null) return false;
         return isPrimitiveOrBoxedClass(o.getClass());
     }
+    public static boolean isPrimitiveOrStringOrBoxedObject(Object o) {
+        return o instanceof String || isPrimitiveOrBoxedObject(o);
+    }
     public static boolean isPrimitiveOrBoxedClass(Class<?> t) {
         // TODO maybe better to switch to using Primitives, eg:
         // return Primitives.allPrimitiveTypes().contains(type) || Primitives.allWrapperTypes().contains(type);


(brooklyn-server) 01/04: allow if workflow step to take condition objects

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 8975991af7bb57331e9894c23de186e58a64e2ea
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Mon Feb 26 14:04:32 2024 +0000

    allow if workflow step to take condition objects
---
 .../core/workflow/steps/flow/IfWorkflowStep.java   | 58 +++++++++++++++++-----
 .../WorkflowSubIfAndCustomExtensionEdgeTest.java   | 18 ++++++-
 2 files changed, 63 insertions(+), 13 deletions(-)

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
index 0fcdfdda8c..c269616080 100644
--- 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
@@ -22,14 +22,18 @@ import java.util.Map;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.google.common.reflect.TypeToken;
+import org.apache.brooklyn.core.resolve.jackson.WrappedValue;
 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.DslEntityPredicateDefault;
 import org.apache.brooklyn.util.core.predicates.DslPredicates.DslPredicate;
+import org.apache.brooklyn.util.core.predicates.DslPredicates.DslPredicateDefault;
 import org.apache.brooklyn.util.core.predicates.DslPredicates.WhenPresencePredicate;
+import org.apache.brooklyn.util.core.task.DeferredSupplier;
 
 public class IfWorkflowStep extends SubWorkflowStep {
 
@@ -77,21 +81,51 @@ public class IfWorkflowStep extends SubWorkflowStep {
         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),
+            boolean isCondition = false;
+            boolean seemsStatic = false;
+            Object k = condition_target;
+            if (condition_equals==null) {
+                if (condition_target instanceof Map) isCondition = true;
+                else if (condition_target instanceof String) {
+                    if (((String) condition_target).trim().startsWith("$")) isCondition = false;
+                    if (((String) condition_target).trim().startsWith("{")) {
+                        isCondition = true;
+                        k = context.getWorkflowExectionContext().resolveWrapped(
+                                WorkflowExpressionStage.STEP_RUNNING, condition_target, TypeToken.of(Map.class),
+                                WrappingMode.WRAPPED_RESULT_DEFER_THROWING_ERROR_BUT_NO_RETRY);
+                    } else seemsStatic = true;
+                } else seemsStatic = true;
+            }
+            k = context.getWorkflowExectionContext().resolveWrapped(
+                    WorkflowExpressionStage.STEP_RUNNING, k, TypeToken.of(isCondition ? DslEntityPredicateDefault.class : 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
+
+            if (seemsStatic && !(k instanceof DslPredicate || k instanceof Boolean || k instanceof DeferredSupplier)) {
+                throw new IllegalArgumentException("Argument supplied as condition target will always evaluate as true.");
+            }
+
+            if (isCondition) {
+                if (WrappedValue.getMaybe( ((DslPredicateDefault<?>)k).implicitEquals ).map(
+                        impEq -> condition_target.equals(impEq)).or(false)) {
+                    // will probably have thrown error when coercing from map, so probably not needed, but for good measure
+                    throw new IllegalArgumentException("Argument supplied as condition target could not be evaluated as condition.");
+                }
+                conditionConstructed = k;
+
             } else {
-                c.put("when", WhenPresencePredicate.TRUTHY);
+                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;
             }
-            conditionConstructed = c;
         }
 
         DslPredicate result = getConditionResolved(context, conditionConstructed);
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubIfAndCustomExtensionEdgeTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubIfAndCustomExtensionEdgeTest.java
index 4505bd8f15..e411031ac0 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubIfAndCustomExtensionEdgeTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSubIfAndCustomExtensionEdgeTest.java
@@ -199,6 +199,11 @@ public class WorkflowSubIfAndCustomExtensionEdgeTest extends RebindTestFixture<T
         Asserts.assertEquals(run.apply(null, "${x} == ${x}"), "yes");
         Asserts.assertEquals(run.apply("let boolean x = true", "${x} == true"), "yes");
 
+        Asserts.assertEquals(run.apply("set-sensor xy = yes", "{ sensor: xy, equals: yes }"), "yes");
+        Asserts.assertEquals(run.apply(null, "{ sensor: xn, equals: yes }"), "no");
+        Asserts.assertFailsWith(() -> run.apply(null, "{ unknown_field: xn }"),
+                Asserts.expectedFailureContainsIgnoreCase("unknown_field", "predicate"));
+
         // unresolvable things -- allowed iff no == block
         Asserts.assertEquals(run.apply(null, "${unresolvable_without_equals}"), "no");
         Asserts.assertFailsWith(() -> run.apply(null, "${x} == ${unresolvable_on_rhs}"),
@@ -210,11 +215,22 @@ public class WorkflowSubIfAndCustomExtensionEdgeTest extends RebindTestFixture<T
         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);
     }
 
+    @Test
+    public void testIfWorkflowWithSteps() throws Exception {
+        BiFunction<String, String, Object> run = (preface, cond) ->
+                runWorkflow(MutableList.<Object>of(preface==null ? "let x = hi" : preface,
+                        MutableMap.of("step", "if "+(cond==null ? "${x}" : cond),
+                            "steps", MutableList.of("let y = yes", "return ${y}")),
+                        "return no"));
+
+        Asserts.assertEquals(run.apply(null, null), "yes");
+        Asserts.assertEquals(run.apply("let boolean x = false", null), "no");
+    }
+
 }