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 2023/03/22 16:23:52 UTC

[brooklyn-server] branch master updated (f19b623bbb -> 8b7b77994b)

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 f19b623bbb Merge branch 'allow-interpolated-expressions-in-nested-workflow-inputs'
     new 459a675191 include content_json as output from http
     new 50c54a39e8 fix workflow comparison bug where bigdecimal was used and not compared properly
     new 3be0143191 fix bug where backoff max wasn't checked for retries
     new 4daf9406a7 add `interpolation_mode` and `interpolation_errors` to `let` and `load`
     new 8b7b77994b Merge branch 'misc-workflow-fixes'

The 5 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/pom.xml                                       |   1 +
 .../workflow/WorkflowExpressionResolution.java     |  29 +++--
 .../core/workflow/WorkflowStepDefinition.java      |   5 +-
 .../WorkflowStepInstanceExecutionContext.java      |   5 +
 .../workflow/steps/external/HttpWorkflowStep.java  |  15 ++-
 .../workflow/steps/flow/RetryWorkflowStep.java     |   5 +
 .../workflow/steps/variables/LoadWorkflowStep.java |   5 +-
 .../steps/variables/SetVariableWorkflowStep.java   |  54 ++++++--
 .../util/core/predicates/DslPredicates.java        |  51 +++++++-
 .../brooklyn/util/core/text/TemplateProcessor.java |  42 ++++++
 .../workflow/WorkflowInputOutputExtensionTest.java | 141 ++++++++++++++++++++-
 .../brooklyn/core/workflow/WorkflowRetryTest.java  |  21 +++
 .../util/core/predicates/DslPredicateTest.java     |  78 ++++++++++++
 .../document-with-interpolated-expression.txt      |   1 +
 .../brooklyn/util/text/NaturalOrderComparator.java |  19 ++-
 15 files changed, 439 insertions(+), 33 deletions(-)
 create mode 100644 core/src/test/resources/document-with-interpolated-expression.txt


[brooklyn-server] 04/05: add `interpolation_mode` and `interpolation_errors` to `let` and `load`

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 4daf9406a7332aa4ff0760b7fa6d5d2624200a75
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Tue Mar 21 15:56:18 2023 +0000

    add `interpolation_mode` and `interpolation_errors` to `let` and `load`
---
 core/pom.xml                                       |   1 +
 .../workflow/WorkflowExpressionResolution.java     |  29 +++--
 .../core/workflow/WorkflowStepDefinition.java      |   5 +-
 .../WorkflowStepInstanceExecutionContext.java      |   5 +
 .../workflow/steps/variables/LoadWorkflowStep.java |   5 +-
 .../steps/variables/SetVariableWorkflowStep.java   |  54 ++++++--
 .../brooklyn/util/core/text/TemplateProcessor.java |  42 ++++++
 .../workflow/WorkflowInputOutputExtensionTest.java | 141 ++++++++++++++++++++-
 .../document-with-interpolated-expression.txt      |   1 +
 9 files changed, 257 insertions(+), 26 deletions(-)

diff --git a/core/pom.xml b/core/pom.xml
index 2eeef7af18..8cbfa5c021 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -325,6 +325,7 @@
                     <excludes combine.children="append">
                         <exclude>**/src/test/resources/org/apache/brooklyn/util/core/osgi/test/bundlemaker/nomanifest/**</exclude>
                         <exclude>**/src/test/resources/org/apache/brooklyn/util/core/osgi/test/bundlemaker/withmanifest/**</exclude>
+                        <exclude>**/src/test/resources/document-with-interpolated-expression.txt</exclude>
                     </excludes>
                 </configuration>
             </plugin>
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 eede7a77c6..64cbf7d2a2 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
@@ -28,7 +28,10 @@ import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
 import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
 import org.apache.brooklyn.core.resolve.jackson.BrooklynJacksonSerializationUtils;
 import org.apache.brooklyn.core.typereg.RegisteredTypes;
-import org.apache.brooklyn.util.collections.*;
+import org.apache.brooklyn.util.collections.Jsonya;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.ThreadLocalStack;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import org.apache.brooklyn.util.core.predicates.ResolutionFailureTreatedAsAbsent;
 import org.apache.brooklyn.util.core.task.DeferredSupplier;
@@ -43,7 +46,6 @@ import org.slf4j.LoggerFactory;
 
 import java.time.Instant;
 import java.util.*;
-import java.util.concurrent.Callable;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
@@ -69,17 +71,23 @@ public class WorkflowExpressionResolution {
     private final boolean allowWaiting;
     private final boolean useWrappedValue;
     private final WorkflowExpressionStage stage;
+    private final TemplateProcessor.InterpolationErrorMode errorMode;
 
     public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, boolean wrapExpressionValues) {
+        this(context, stage, allowWaiting, wrapExpressionValues, TemplateProcessor.InterpolationErrorMode.FAIL);
+    }
+    public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, boolean wrapExpressionValues, TemplateProcessor.InterpolationErrorMode errorMode) {
         this.context = context;
         this.stage = stage;
         this.allowWaiting = allowWaiting;
         this.useWrappedValue = wrapExpressionValues;
+        this.errorMode = errorMode;
     }
 
     TemplateModel ifNoMatches() {
-        // this causes the execution to fail. any other behaviour is hard with freemarker.
-        // recommendation is to use freemarker attempts/escapes to recover.
+        // fail here - any other behaviour is hard with freemarker (exceptions intercepted etc).
+        // error handling is done by 'process' method below, and by ?? notation handling in let,
+        // or if needed freemarker attempts/escapes to recover could be used (not currently used much)
         return null;
     }
 
@@ -439,21 +447,22 @@ public class WorkflowExpressionResolution {
             entry = WorkflowVariableResolutionStackEntry.of(context, stage, expression);
             if (!RESOLVE_STACK.push(entry)) {
                 entry = null;
-                throw new WorkflowVariableRecursiveReference("Recursive reference: "+RESOLVE_STACK.getAll(false).stream().map(p -> ""+p.object).collect(Collectors.joining("->")));
+                throw new WorkflowVariableRecursiveReference("Recursive reference: " + RESOLVE_STACK.getAll(false).stream().map(p -> "" + p.object).collect(Collectors.joining("->")));
             }
-            if (RESOLVE_STACK.size()>100) {
-                throw new WorkflowVariableRecursiveReference("Reference exceeded max depth 100: "+RESOLVE_STACK.getAll(false).stream().map(p -> ""+p.object).collect(Collectors.joining("->")));
+            if (RESOLVE_STACK.size() > 100) {
+                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);
             if (expression instanceof Map) return processTemplateExpressionMap((Map) expression);
-            if (expression instanceof Collection) return processTemplateExpressionCollection((Collection) expression);
+            if (expression instanceof Collection)
+                return processTemplateExpressionCollection((Collection) expression);
             if (expression == null || Boxing.isPrimitiveOrBoxedObject(expression)) return expression;
             // otherwise resolve DSL
             return resolveDsl(expression);
 
         } finally {
-            if (entry!=null) RESOLVE_STACK.pop(entry);
+            if (entry != null) RESOLVE_STACK.pop(entry);
         }
     }
 
@@ -502,7 +511,7 @@ public class WorkflowExpressionResolution {
 
         boolean ourWait = interruptSetIfNeededToPreventWaiting();
         try {
-            result = TemplateProcessor.processTemplateContents("workflow", expression, model, true, false);
+            result = TemplateProcessor.processTemplateContents("workflow", expression, model, true, false, errorMode);
         } catch (Exception e) {
             Exception e2 = e;
             if (!allowWaiting && Exceptions.isCausedByInterruptInAnyThread(e)) {
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 9c192497a4..c6ea01cabe 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
@@ -144,11 +144,12 @@ public abstract class WorkflowStepDefinition {
     protected void populateFromShorthandTemplate(String template, String value) {
         populateFromShorthandTemplate(template, value, false, true);
     }
-    protected void populateFromShorthandTemplate(String template, String value, boolean finalMatchRaw, boolean failOnMismatch) {
+    protected Maybe<Map<String, Object>> populateFromShorthandTemplate(String template, String value, boolean finalMatchRaw, boolean failOnMismatch) {
         Maybe<Map<String, Object>> result = new ShorthandProcessor(template).withFinalMatchRaw(finalMatchRaw).withFailOnMismatch(failOnMismatch).process(value);
         if (result.isAbsent()) throw new IllegalArgumentException("Invalid shorthand expression: '"+value+"'", Maybe.Absent.getException(result));
 
-        input.putAll((Map) CollectionMerger.builder().build().merge(input, result.get()));
+        input.putAll((Map<? extends String, ?>) CollectionMerger.builder().build().merge(input, result.get()));
+        return result;
     }
 
     final Task<?> newTask(WorkflowStepInstanceExecutionContext context) {
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 ed710f0e0b..8958904944 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
@@ -111,6 +111,11 @@ public class WorkflowStepInstanceExecutionContext {
     public <T> T getInput(ConfigKey<T> key) {
         return getInput(key.getName(), key.getTypeToken());
     }
+    public <T> T getInputOrDefault(ConfigKey<T> key) {
+        T result = getInput(key.getName(), key.getTypeToken());
+        if (result==null && !input.containsKey(key.getName())) result = key.getDefaultValue();
+        return result;
+    }
     /** Returns the resolved value of the given key, converting to the type of the key if the key is known */
     public Object getInput(String key) {
         ConfigKey<?> keyTyped = ConfigUtilsInternal.findConfigKeys(getClass(), null).get(key);
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/LoadWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/LoadWorkflowStep.java
index 3238045ac3..1a60f5a10c 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/LoadWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/LoadWorkflowStep.java
@@ -27,6 +27,7 @@ import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
 import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.core.ResourceUtils;
+import org.apache.brooklyn.util.core.text.TemplateProcessor;
 import org.apache.brooklyn.util.text.ByteSizeStrings;
 import org.apache.brooklyn.util.text.Strings;
 import org.slf4j.Logger;
@@ -45,6 +46,8 @@ public class LoadWorkflowStep extends WorkflowStepDefinition {
     public static final ConfigKey<TypedValueToSet> VARIABLE = ConfigKeys.newConfigKey(TypedValueToSet.class, "variable");
     public static final ConfigKey<Object> URL = ConfigKeys.newConfigKey(Object.class, "url");
     public static final ConfigKey<String> CHARSET = ConfigKeys.newStringConfigKey("charset");
+    public static final ConfigKey<SetVariableWorkflowStep.InterpolationMode> INTERPOLATION_MODE = ConfigKeys.newConfigKeyWithDefault(SetVariableWorkflowStep.INTERPOLATION_MODE, SetVariableWorkflowStep.InterpolationMode.DISABLED);
+    public static final ConfigKey<TemplateProcessor.InterpolationErrorMode> INTERPOLATION_ERRORS = ConfigKeys.newConfigKeyWithDefault(SetVariableWorkflowStep.INTERPOLATION_ERRORS, TemplateProcessor.InterpolationErrorMode.IGNORE);
 
     @Override
     public void populateFromShorthand(String expression) {
@@ -83,7 +86,7 @@ public class LoadWorkflowStep extends WorkflowStepDefinition {
             data = r.getResourceAsString("" + url);
         }
 
-        Object resolvedValue = context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_OUTPUT, data, type);
+        Object resolvedValue = new SetVariableWorkflowStep.ConfigurableInterpolationEvaluation(context, type, data, context.getInputOrDefault(INTERPOLATION_MODE), context.getInputOrDefault(INTERPOLATION_ERRORS)).evaluate();
 
         context.getWorkflowExectionContext().getWorkflowScratchVariables().put(name, resolvedValue);
 
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java
index 729a16e130..44e4969fbc 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/SetVariableWorkflowStep.java
@@ -28,6 +28,7 @@ import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
 import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
 import org.apache.brooklyn.util.collections.*;
 import org.apache.brooklyn.util.core.flags.TypeCoercions;
+import org.apache.brooklyn.util.core.text.TemplateProcessor;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.text.QuotedStringTokenizer;
@@ -46,26 +47,41 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
     private static final Logger log = LoggerFactory.getLogger(SetVariableWorkflowStep.class);
 
     public static final String SHORTHAND =
-
             "[ [ ${variable.type} ] ${variable.name} [ \"=\" ${value...} ] ]";
 
     public static final ConfigKey<TypedValueToSet> VARIABLE = ConfigKeys.newConfigKey(TypedValueToSet.class, "variable");
     public static final ConfigKey<Object> VALUE = ConfigKeys.newConfigKey(Object.class, "value");
 
+    public enum InterpolationMode {
+        WORDS,
+        DISABLED,
+        FULL,
+    }
+    public static final ConfigKey<InterpolationMode> INTERPOLATION_MODE = ConfigKeys.newConfigKey(InterpolationMode.class, "interpolation_mode",
+            "Whether interpolation runs on the full value (not touching quotes; the default in most places), " +
+                    "on words (if unquoted, unquoting others; the default for 'let var = value' shorthand), " +
+                    "or is disabled (not applied at all)");
+    public static final ConfigKey<TemplateProcessor.InterpolationErrorMode> INTERPOLATION_ERRORS = ConfigKeys.newConfigKey(TemplateProcessor.InterpolationErrorMode.class, "interpolation_errors",
+            "Whether unresolvable interpolated expressions fail and return an error (the default for 'let'), " +
+                    "ignore the expression leaving it in place (the default for 'load'), " +
+                    "or replace the expression with a blank string");
 
     @Override
     public void populateFromShorthand(String expression) {
-        populateFromShorthandTemplate(SHORTHAND, expression, true, true);
+        Maybe<Map<String, Object>> newInput = populateFromShorthandTemplate(SHORTHAND, expression, true, true);
+        if (newInput.isPresentAndNonNull() && newInput.get().get(VALUE.getName())!=null && input.get(INTERPOLATION_MODE.getName())==null) {
+            setInput(INTERPOLATION_MODE, InterpolationMode.WORDS);
+        }
     }
 
     @Override
     public void validateStep(@Nullable ManagementContext mgmt, @Nullable WorkflowExecutionContext workflow) {
         super.validateStep(mgmt, workflow);
 
-        if (!input.containsKey(VARIABLE.getName())) {
+        if (input.get(VARIABLE.getName())==null) {
             throw new IllegalArgumentException("Variable name is required");
         }
-        if (!input.containsKey(VALUE.getName())) {
+        if (input.get(VALUE.getName())==null) {
             throw new IllegalArgumentException("Value is required");
         }
     }
@@ -80,7 +96,7 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
 
         Object unresolvedValue = input.get(VALUE.getName());
 
-        Object resolvedValue = new SetVariableEvaluation(context, type, unresolvedValue).evaluate();
+        Object resolvedValue = new ConfigurableInterpolationEvaluation(context, type, unresolvedValue, context.getInputOrDefault(INTERPOLATION_MODE), context.getInputOrDefault(INTERPOLATION_ERRORS)).evaluate();
 
         Object oldValue = setWorkflowScratchVariableDotSeparated(context, name, resolvedValue);
         context.noteOtherMetadata("Value set", resolvedValue);
@@ -113,15 +129,22 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
 
     private enum LetMergeMode { NONE, SHALLOW, DEEP }
 
-    public static class SetVariableEvaluation<T> {
+    public static class ConfigurableInterpolationEvaluation<T> {
         protected final WorkflowStepInstanceExecutionContext context;
         protected final TypeToken<T> type;
         protected final Object unresolvedValue;
+        protected final InterpolationMode interpolationMode;
+        protected final TemplateProcessor.InterpolationErrorMode errorMode;
 
-        public SetVariableEvaluation(WorkflowStepInstanceExecutionContext context, TypeToken<T> type, Object unresolvedValue) {
+        public ConfigurableInterpolationEvaluation(WorkflowStepInstanceExecutionContext context, TypeToken<T> type, Object unresolvedValue) {
+            this(context, type, unresolvedValue, null, null);
+        }
+        public ConfigurableInterpolationEvaluation(WorkflowStepInstanceExecutionContext context, TypeToken<T> type, Object unresolvedValue, InterpolationMode interpolationMode, TemplateProcessor.InterpolationErrorMode errorMode) {
             this.context = context;
             this.unresolvedValue = unresolvedValue;
             this.type = type;
+            this.interpolationMode = interpolationMode;
+            this.errorMode = errorMode;
         }
 
         public boolean unquotedStartsWith(String s, char c) {
@@ -148,16 +171,27 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
 
             Object resultCoerced;
             TypeToken<? extends Object> typeIntermediate = type==null ? TypeToken.of(Object.class) : type;
-            if (result instanceof String) {
+
+            if (interpolationMode==InterpolationMode.DISABLED) {
+                resultCoerced = context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, result, typeIntermediate);
+
+            } else if (result instanceof String && interpolationMode==InterpolationMode.WORDS) {
                 result = process((String) result);
                 resultCoerced = context.getWorkflowExectionContext().resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, result, typeIntermediate);
+
             } else {
-                resultCoerced = context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, result, typeIntermediate);
+                // full, or null the default
+                resultCoerced = resolveSubPart(result, typeIntermediate);
             }
 
             return (T) resultCoerced;
         }
 
+        <T> T resolveSubPart(Object v, TypeToken<T> type) {
+            return new WorkflowExpressionResolution(context.getWorkflowExectionContext(), WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, false, false, errorMode)
+                    .resolveWithTemplates(v, type);
+        }
+
         Object process(String input) {
             if (Strings.isBlank(input)) return input;
 
@@ -201,7 +235,7 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
             List<Object> objs = w.stream().map(t -> {
                 if (qst.isQuoted(t)) return qst.unwrapIfQuoted(t);
                 TypeToken<?> target = resolveToString ? TypeToken.of(String.class) : TypeToken.of(Object.class);
-                return context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, t, target);
+                return resolveSubPart(t, target);
             }).collect(Collectors.toList());
             if (!resolveToString) return objs.get(0);
             return ((List<String>)(List)objs).stream().collect(Collectors.joining());
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java b/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java
index 3673db3390..511867b4f4 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/text/TemplateProcessor.java
@@ -840,6 +840,9 @@ public class TemplateProcessor {
     }
 
     public static Object processTemplateContents(String context, String templateContents, final TemplateHashModel substitutions, boolean allowSingleVariableObject, boolean logErrors) {
+        return processTemplateContents(context, templateContents, substitutions, allowSingleVariableObject, logErrors, InterpolationErrorMode.FAIL);
+    }
+    public static Object processTemplateContents(String context, String templateContents, final TemplateHashModel substitutions, boolean allowSingleVariableObject, boolean logErrors, InterpolationErrorMode errorMode) {
         try {
             Configuration cfg = new Configuration(Configuration.DEFAULT_INCOMPATIBLE_IMPROVEMENTS);
             cfg.setLogTemplateExceptions(logErrors);
@@ -885,6 +888,7 @@ public class TemplateProcessor {
             // TODO could expose CAMP '$brooklyn:' style dsl, based on template.createProcessingEnvironment
             ByteArrayOutputStream baos = new ByteArrayOutputStream();
             Writer out = new OutputStreamWriter(baos);
+            template.setTemplateExceptionHandler(new ForgivingFreemarkerTemplateExceptionHandler(errorMode));
             template.process(substitutions, out);
             out.flush();
 
@@ -903,4 +907,42 @@ public class TemplateProcessor {
         }
     }
 
+    public enum InterpolationErrorMode {
+        FAIL,
+        BLANK,
+        IGNORE,
+    }
+
+    InterpolationErrorMode interpolationErrorMode;
+
+    public void setInterpolationErrorMode(InterpolationErrorMode interpolationErrorMode) {
+        this.interpolationErrorMode = interpolationErrorMode;
+    }
+
+    public static class ForgivingFreemarkerTemplateExceptionHandler implements TemplateExceptionHandler {
+        private final InterpolationErrorMode errorMode;
+        public ForgivingFreemarkerTemplateExceptionHandler(InterpolationErrorMode errorMode) {
+            this.errorMode = errorMode;
+        }
+        public void handleTemplateException(TemplateException te, Environment env, Writer out) throws TemplateException {
+            if (errorMode==null || errorMode==InterpolationErrorMode.FAIL) throw te;
+
+            if (errorMode==InterpolationErrorMode.BLANK) return;
+            if (errorMode==InterpolationErrorMode.IGNORE) {
+                try {
+                    // below won't work for complex expressions but those are discouraged anyways
+                    out.write("${" + te.getBlamedExpressionString() + "}");
+
+                    // this would work better, if we want to access private fields
+                    //te.getFTLInstructionStack();
+//                TemplateElement els[] = env.instructionStack;
+//                env.instructionStackSize - 1
+                } catch (IOException e) {
+                    throw Exceptions.propagate(e);
+                }
+                return;
+            }
+        }
+    }
+
 }
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java
index ecd28d2f31..2a7b48bd3e 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowInputOutputExtensionTest.java
@@ -31,8 +31,8 @@ import org.apache.brooklyn.core.sensor.Sensors;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
 import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
-import org.apache.brooklyn.core.workflow.steps.flow.LogWorkflowStep;
 import org.apache.brooklyn.core.workflow.steps.appmodel.SetSensorWorkflowStep;
+import org.apache.brooklyn.core.workflow.steps.flow.LogWorkflowStep;
 import org.apache.brooklyn.entity.stock.BasicApplication;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.test.ClassLogWatcher;
@@ -48,6 +48,7 @@ import org.testng.annotations.Test;
 import java.time.Instant;
 import java.util.List;
 import java.util.Map;
+import java.util.function.BiConsumer;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -274,6 +275,138 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor
         Asserts.assertEquals(output, 0);
     }
 
+    @Test
+    public void testLetInterpolationMode() {
+        Consumer<Map<String,Object>> invoke = input -> { try {
+            invokeWorkflowStepsWithLogging(MutableList.of("let person = Anna",
+                    input,
+                    "log NOTE: ${x}"));
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        } };
+        BiFunction<Map<String,Object>,String,String> assertLetGives = (input,expected) -> {
+            invoke.accept(input);
+            String note = Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> s.startsWith("NOTE:")).findAny().get(), "NOTE: ");
+            Asserts.assertEquals(note, expected);
+            return note;
+        };
+
+        // disabled
+        assertLetGives.apply(MutableMap.of("step", "let x = \"${person}\"", "interpolation_mode", "disabled"), "\"${person}\"");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", "interpolation_mode", "disabled"), "${person}");
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", "\"${person}\"", "interpolation_mode", "disabled"), "\"${person}\"");
+
+        // words forced
+        assertLetGives.apply(MutableMap.of("step", "let x = \"${person}\"", "interpolation_mode", "words"), "${person}");
+        assertLetGives.apply(MutableMap.of("step", "let x = \"Anna\"", "interpolation_mode", "words"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", "interpolation_mode", "words"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", "\"${person}\"", "interpolation_mode", "words"), "${person}");
+
+        // full forced
+        assertLetGives.apply(MutableMap.of("step", "let x = \"${person}\"", "interpolation_mode", "full"), "\"Anna\"");
+        assertLetGives.apply(MutableMap.of("step", "let x = \"Anna\"", "interpolation_mode", "full"), "\"Anna\"");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", "interpolation_mode", "full"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", "\"${person}\"", "interpolation_mode", "full"), "\"Anna\"");
+
+        // defaults - words for shorthand
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = \"${person}\""), "${person}");
+        assertLetGives.apply(MutableMap.of("step", "let x = \"Anna\""), "Anna");
+
+        // defaults - full for separate value
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", "${person}"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x", "value", "\"${person}\""), "\"Anna\"");
+    }
+
+    @Test
+    public void testLetInterpolationErrorMode() {
+        Consumer<Map<String,Object>> invoke = input -> { try {
+            invokeWorkflowStepsWithLogging(MutableList.of("let person = Anna",
+                    input,
+                    "log NOTE: ${x}"));
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        } };
+        BiFunction<Map<String,Object>,String,String> assertLetGives = (input,expected) -> {
+            invoke.accept(input);
+            String note = Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> s.startsWith("NOTE:")).findAny().get(), "NOTE: ");
+            Asserts.assertEquals(note, expected);
+            return note;
+        };
+
+        // does nothing if value found
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", "interpolation_errors", "fail"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", "interpolation_errors", "blank"), "Anna");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${person}", "interpolation_errors", "ignore"), "Anna");
+
+        // but if value not found
+        Asserts.assertFailsWith(() -> assertLetGives.apply(MutableMap.of("step", "let x = ${unknown_person}"), "not-used"),
+                e -> Asserts.expectedFailureContains(e, "unknown_person"));
+        Asserts.assertFailsWith(() -> assertLetGives.apply(MutableMap.of("step", "let x = ${unknown_person}", "interpolation_errors", "fail"), "not-used"),
+            e -> Asserts.expectedFailureContains(e, "unknown_person"));
+        assertLetGives.apply(MutableMap.of("step", "let x = ${unknown_person}", "interpolation_errors", "blank"), "");
+        assertLetGives.apply(MutableMap.of("step", "let x = ${unknown_person}", "interpolation_errors", "ignore"), "${unknown_person}");
+    }
+
+    @Test
+    public void testLoadInterpolationMode() {
+        BiConsumer<String,Map<String,Object>> invoke = (otherVarName, input) -> { try {
+            invokeWorkflowStepsWithLogging(MutableList.of(
+                    "let person = Anna",
+                    "let "+otherVarName+" = Other",
+                    MutableMap.<String,Object>of("step", "load x = classpath://document-with-interpolated-expression.txt").add(input),
+                    "log NOTE: ${x}"));
+        } catch (Exception e) {
+            throw Exceptions.propagate(e);
+        } };
+        BiFunction<Map<String,Object>,String,String> assertLoadWithOtherVarGives = (input,expected) -> {
+            invoke.accept("other", input);
+            String note = Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> s.startsWith("NOTE:")).findAny().get(), "NOTE: ");
+            Asserts.assertEquals(note, expected);
+            return note;
+        };
+        BiFunction<Map<String,Object>,String,String> assertLoadWithoutOtherVarGives = (input,expected) -> {
+            invoke.accept("ignored", input);
+            String note = Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> s.startsWith("NOTE:")).findAny().get(), "NOTE: ");
+            Asserts.assertEquals(note, expected);
+            return note;
+        };
+
+        assertLoadWithOtherVarGives.apply(null, "This is a test. Person is ${person} and other is ${other}.");
+        assertLoadWithOtherVarGives.apply(MutableMap.of("interpolation_mode", "disabled"), "This is a test. Person is ${person} and other is ${other}.");
+        assertLoadWithOtherVarGives.apply(MutableMap.of("interpolation_mode", "full"), "This is a test. Person is Anna and other is Other.");
+
+        assertLoadWithoutOtherVarGives.apply(null, "This is a test. Person is ${person} and other is ${other}.");
+        assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", "disabled"), "This is a test. Person is ${person} and other is ${other}.");
+        assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", "full"), "This is a test. Person is Anna and other is ${other}.");
+
+
+        assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_error", "fail"), "This is a test. Person is ${person} and other is ${other}.");
+        assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", "disabled", "interpolation_errors", "fail"), "This is a test. Person is ${person} and other is ${other}.");
+        assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", "full"), "This is a test. Person is Anna and other is ${other}.");
+        assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", "full", "interpolation_errors", "ignore"), "This is a test. Person is Anna and other is ${other}.");
+        assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", "full", "interpolation_errors", "blank"), "This is a test. Person is Anna and other is .");
+        Asserts.assertFailsWith(() -> assertLoadWithoutOtherVarGives.apply(MutableMap.of("interpolation_mode", "full", "interpolation_error", "fail"), "not-used"),
+                e -> Asserts.expectedFailureContains(e, "other"));
+    }
+
+    @Test
+    public void testInterpolationNotRecursiveSoDisablingWorks() throws Exception {
+        // outwith inputs, interpolation is not recursive. for inputs, it is permitted to be.
+        Object result = invokeWorkflowStepsWithLogging(MutableList.of("let person1 = Anna",
+                MutableMap.of("step", "let person2 = ${person1}", "interpolation_mode", "disabled"),
+                MutableMap.of("step", "log P2: ${person2}"),
+                MutableMap.of("step", "let person3 = ${person2}"),
+                MutableMap.of("step", "log P3: ${person3}"),
+                "return ${person3}"));
+        Asserts.assertEquals(result, "${person1}");
+        String p2 = Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> s.startsWith("P2:")).findAny().get(), "P2: ");
+        Asserts.assertEquals(p2, "${person1}");
+        String p3 = Strings.removeFromStart(lastLogWatcher.getMessages().stream().filter(s -> s.startsWith("P3:")).findAny().get(), "P3: ");
+        Asserts.assertEquals(p3, "${person1}");
+    }
+
     @Test
     public void testLetQuoteVar() {
         Consumer<String> invoke = input -> { try {
@@ -362,8 +495,10 @@ public class WorkflowInputOutputExtensionTest extends BrooklynMgmtUnitTestSuppor
     }
 
     private Object transform(String filter, Object expression) throws Exception {
-        return invokeWorkflowStepsWithLogging(MutableList.of(
-                MutableMap.<String,Object>of("step", "let x1", "value", expression),
+        boolean inline = expression instanceof String && Strings.isNonEmpty((String)expression) && ((String)expression).trim().equals(expression);
+        Map<String,Object> letStep = MutableMap.<String,Object>of("step", "let x1" + (inline ? " = "+expression : ""));
+        if (!inline) letStep.put("value", expression);
+        return invokeWorkflowStepsWithLogging(MutableList.of(letStep,
                 "transform x2 = ${x1} | "+filter, "return ${x2}"));
     }
 
diff --git a/core/src/test/resources/document-with-interpolated-expression.txt b/core/src/test/resources/document-with-interpolated-expression.txt
new file mode 100644
index 0000000000..418013d90a
--- /dev/null
+++ b/core/src/test/resources/document-with-interpolated-expression.txt
@@ -0,0 +1 @@
+This is a test. Person is ${person} and other is ${other}.
\ No newline at end of file


[brooklyn-server] 01/05: include content_json as output from http

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 459a67519191d39a759667c9d98eacef3427f4df
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Tue Mar 21 09:23:24 2023 +0000

    include content_json as output from http
---
 .../core/workflow/steps/external/HttpWorkflowStep.java    | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

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 6f24f49af0..b3b10b9fda 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
@@ -30,7 +30,6 @@ import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.javalang.BrooklynHttpConfig;
 import org.apache.brooklyn.util.core.json.ShellEnvironmentSerializer;
 import org.apache.brooklyn.util.core.predicates.DslPredicates;
-import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.http.HttpTool;
 import org.apache.brooklyn.util.http.auth.UsernamePassword;
@@ -43,9 +42,10 @@ import org.apache.brooklyn.util.stream.Streams;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.apache.http.client.utils.URIBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.lang.reflect.Array;
 import java.net.URISyntaxException;
 import java.nio.charset.Charset;
 import java.util.Map;
@@ -53,6 +53,8 @@ import java.util.function.Predicate;
 
 public class HttpWorkflowStep extends WorkflowStepDefinition {
 
+    private static final Logger log = LoggerFactory.getLogger(HttpWorkflowStep.class);
+
     public static final String SHORTHAND = "${endpoint}";
 
     public static final ConfigKey<String> ENDPOINT = ConfigKeys.newStringConfigKey("endpoint");
@@ -160,7 +162,14 @@ public class HttpWorkflowStep extends WorkflowStepDefinition {
         Predicate<Integer> exitcode = context.getInput(STATUS_CODE);
         if (exitcode==null) exitcode = code -> HttpTool.isStatusCodeHealthy(code);
 
-        context.setOutput(MutableMap.of("status_code", response.code(), "headers", response.headers(), "content", contentString, "content_bytes", contentBytes, "duration", Duration.millis(endTime - startTime)));
+        Object content_json = null;
+        try {
+            content_json = BeanWithTypeUtils.newSimpleMapper().readValue(contentString, Object.class);
+        } catch (Exception e) {
+            Exceptions.propagateIfFatal(e);
+            log.debug("Content from web request is not json; not setting content_json: "+e);
+        }
+        context.setOutput(MutableMap.of("status_code", response.code(), "headers", response.headers(), "content", contentString, "content_bytes", contentBytes, "content_json", content_json, "duration", Duration.millis(endTime - startTime)));
         // make sure the output is set even if there is an error
         checkExitCode(response.code(), exitcode);
         return context.getOutput();


[brooklyn-server] 03/05: fix bug where backoff max wasn't checked for retries

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 3be0143191ea659cfad402565c13d2531b95e7de
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Tue Mar 21 10:57:00 2023 +0000

    fix bug where backoff max wasn't checked for retries
---
 .../core/workflow/steps/flow/RetryWorkflowStep.java |  5 +++++
 .../brooklyn/core/workflow/WorkflowRetryTest.java   | 21 +++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/RetryWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/RetryWorkflowStep.java
index 8770336e5f..e35574707f 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/RetryWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/flow/RetryWorkflowStep.java
@@ -284,6 +284,11 @@ public class RetryWorkflowStep extends WorkflowStepDefinition {
             if (backoff.increase !=null) delay = delay.add(backoff.increase.multiply(exponent));
 
             if (backoff.jitter!=null) delay = delay.multiply(1 + Math.random()*backoff.jitter);
+            if (backoff.max!=null && delay.isLongerThan(backoff.max)) {
+                delay = backoff.max;
+                // also apply a sigmoidal heuristic if jitter requested
+                if (backoff.jitter!=null) delay = delay.multiply(1 / (1+Math.random()*backoff.jitter));
+            }
 
             if (delay.isPositive()) {
                 Duration ddelay = delay;
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowRetryTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowRetryTest.java
index 6f538cf274..ef3b500f36 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowRetryTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowRetryTest.java
@@ -278,6 +278,27 @@ public class WorkflowRetryTest extends RebindTestFixture<BasicApplication> {
         Asserts.assertThat(Duration.of(sw), d -> d.isShorterThan(Duration.millis(EXPECTED_DELAY + GRACE)));
     }
 
+    @Test(groups="Integration")  // because slow
+    public void testRetryWithBackoffUpToAndLimit() {
+        Stopwatch sw = Stopwatch.createStarted();
+        try {
+            Task<?> lastInvocation = runSteps(MutableList.of(
+                "let integer x = ${entity.sensor.x} ?? 0",
+                "let x = ${x} + 1",
+                "set-sensor x = ${x}",
+                "retry from start limit 1s backoff 1ms increasing 2x up to 32ms"), null);
+            Asserts.shouldHaveFailedPreviously("Instead got "+lastInvocation.getUnchecked());
+        } catch (Exception e) {
+            Asserts.expectedFailure(e);
+            Asserts.assertNotNull(Exceptions.getFirstThrowableOfType(e, RetryWorkflowStep.RetriesExceeded.class), "Exception "+e);
+        }
+        long EXPECTED_DELAY = 999;
+        Asserts.assertThat(Duration.of(sw), d -> d.isLongerThan(Duration.millis(EXPECTED_DELAY)));
+        Integer x = app.sensors().get(Sensors.newIntegerSensor("x"));
+        Asserts.assertThat(x, xx -> xx > 12);   // shouldn't keep doubling beyond 32 ms
+        Asserts.assertThat(x, xx -> xx < 40);   // but should increase
+    }
+
     Task<?> lastInvocation;
 
     @Test


[brooklyn-server] 02/05: fix workflow comparison bug where bigdecimal was used and not compared properly

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 50c54a39e88235a3b155e2ffaf701fae332d17af
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Tue Mar 21 10:03:18 2023 +0000

    fix workflow comparison bug where bigdecimal was used and not compared properly
    
    also avoid any E notation for numbers
---
 .../util/core/predicates/DslPredicates.java        | 51 +++++++++++++-
 .../util/core/predicates/DslPredicateTest.java     | 78 ++++++++++++++++++++++
 .../brooklyn/util/text/NaturalOrderComparator.java | 19 +++++-
 3 files changed, 144 insertions(+), 4 deletions(-)

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 90622d1868..036a65b1aa 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
@@ -68,6 +68,8 @@ import org.slf4j.LoggerFactory;
 import javax.annotation.Nullable;
 import java.io.IOException;
 import java.lang.reflect.Method;
+import java.math.BigDecimal;
+import java.math.BigInteger;
 import java.util.*;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiFunction;
@@ -156,8 +158,35 @@ public class DslPredicates {
         // if either believes they're equal, believe it
         if (a.equals(b) || b.equals(a)) return 0;
 
-        if (isStringOrPrimitive(a) && isStringOrPrimitive(b)) {
-            return NaturalOrderComparator.INSTANCE.compare(a.toString(), b.toString());
+        if (a instanceof Number || b instanceof Number) {
+            try {
+                // if either number is a string try to treat both as string
+                if (a instanceof String) a = new BigDecimal((String) a);
+                if (b instanceof String) b = new BigDecimal((String) b);
+
+                if (a instanceof Number && b instanceof Number) {
+                    if (a.getClass().equals(b.getClass())) {
+                        if (a instanceof BigDecimal) return ((BigDecimal) a).compareTo((BigDecimal) b);
+                        if (a instanceof BigInteger) return ((BigInteger) a).compareTo((BigInteger) b);
+                        if (a instanceof Long) return ((Long) a).compareTo((Long) b);
+                        if (a instanceof Integer) return ((Integer) a).compareTo((Integer) b);
+                        return ((Double) (((Number) a).doubleValue())).compareTo(((Number) b).doubleValue());
+                    }
+                    if (a instanceof Integer || a instanceof Long || a instanceof Byte) a = BigDecimal.valueOf(((Number) a).longValue());
+                    if (b instanceof Integer || b instanceof Long || a instanceof Byte) b = BigDecimal.valueOf(((Number) b).longValue());
+                    if (a instanceof Double || a instanceof Float) a = BigDecimal.valueOf(((Number) a).doubleValue());
+                    if (b instanceof Double || b instanceof Float) b = BigDecimal.valueOf(((Number) b).doubleValue());
+                    // all now normally big decimal
+                    if (a instanceof BigDecimal && b instanceof BigDecimal) return ((BigDecimal) a).compareTo((BigDecimal) b);
+                    // some weird type; proceed to string stuff below
+                }
+            } catch (Exception e) {
+                // one number is not parseable as a string; proceed to natural order comparator
+            }
+        }
+
+        if (isStringOrPrimitiveOrNumber(a) && isStringOrPrimitiveOrNumber(b)) {
+            return NaturalOrderComparator.INSTANCE.compare(toStringForPrimitives(a), toStringForPrimitives(b));
         }
 
         if (a instanceof DeferredSupplier || b instanceof DeferredSupplier)
@@ -188,12 +217,28 @@ public class DslPredicates {
         return a!=null && (a instanceof String || Boxing.isPrimitiveOrBoxedClass(a.getClass()));
     }
 
+    private static boolean isStringOrPrimitiveOrNumber(Object a) {
+        return isStringOrPrimitive(a) || (a!=null && (a instanceof Number));
+    }
+
     private static boolean isJson(Object a) {
         return isStringOrPrimitive(a) || (a instanceof Map) || (a instanceof Collection);
     }
 
+    static String toStringForPrimitives(Object a) {
+        if (a==null) return null;
+        if (a instanceof Number) {
+            // print decimal numbers without E notation
+            if (a instanceof BigDecimal) return ((BigDecimal)a).toPlainString();
+            if (a instanceof Double) return toStringForPrimitives(new BigDecimal((Double)a));
+            if (a instanceof Float) return toStringForPrimitives(new BigDecimal((Float)a));
+            // fall through to below
+        }
+        return a.toString();
+    }
+
     static boolean asStringTestOrFalse(Object value, Predicate<String> test) {
-        return isStringOrPrimitive(value) || value instanceof Throwable ? test.test(value.toString()) : value instanceof Class ? test.test(((Class)value).getName()) : false;
+        return isStringOrPrimitiveOrNumber(value) || value instanceof Throwable ? test.test(toStringForPrimitives(value)) : value instanceof Class ? test.test(((Class)value).getName()) : false;
     }
 
     @JsonInclude(JsonInclude.Include.NON_NULL)
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 a971d8a79e..74bc686501 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
@@ -121,6 +121,84 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
         Asserts.assertFalse(p.test("5"));
     }
 
+    @Test
+    public void testGreaterThanOtherTypes() {
+        DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of(
+                "greater-than", "25.45"), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test(26.01d));
+        Asserts.assertTrue(p.test(101.01d));
+        Asserts.assertTrue(p.test(101));
+        Asserts.assertFalse(p.test(24.9d));
+        Asserts.assertFalse(p.test(3.3d));
+        Asserts.assertTrue(p.test(25.55d));
+        Asserts.assertFalse(p.test(25.35d));
+        Asserts.assertFalse(p.test(1d));
+        Asserts.assertFalse(p.test(1));
+        Asserts.assertFalse(p.test("1"));
+        Asserts.assertFalse(p.test(-1d));
+        Asserts.assertFalse(p.test(-1));
+        Asserts.assertFalse(p.test("-1"));
+        Asserts.assertFalse(p.test("."));
+        Asserts.assertTrue(p.test("A"));
+
+        p = TypeCoercions.coerce(MutableMap.of(
+                "greater-than", 25.45d), DslPredicates.DslPredicate.class);
+        Asserts.assertTrue(p.test(26.01d));
+        Asserts.assertTrue(p.test(101.01d));
+        Asserts.assertTrue(p.test(101));
+        Asserts.assertFalse(p.test(24.9d));
+        Asserts.assertFalse(p.test(3.3d));
+        Asserts.assertTrue(p.test(25.55d));
+        Asserts.assertFalse(p.test(25.35d));
+        Asserts.assertFalse(p.test(-25.55d));
+        Asserts.assertFalse(p.test(1d));
+        Asserts.assertFalse(p.test(1));
+        Asserts.assertFalse(p.test("1"));
+        Asserts.assertFalse(p.test(-1));
+        Asserts.assertFalse(p.test(-1d));
+        Asserts.assertFalse(p.test("-1"));
+        Asserts.assertFalse(p.test("."));
+        Asserts.assertTrue(p.test("A"));
+
+        p = TypeCoercions.coerce(MutableMap.of(
+                "greater-than", -25.45), DslPredicates.DslPredicate.class);
+        Asserts.assertFalse(p.test(-26.01d));
+        Asserts.assertFalse(p.test(-101.01d));
+        Asserts.assertFalse(p.test(-101));
+        Asserts.assertTrue(p.test(-24.9d));
+        Asserts.assertTrue(p.test(-3.3d));
+        Asserts.assertFalse(p.test(-25.55d));
+        Asserts.assertTrue(p.test(-25.35d));
+        Asserts.assertTrue(p.test(25.35d));
+        Asserts.assertTrue(p.test(1d));
+        Asserts.assertTrue(p.test(1));
+        Asserts.assertTrue(p.test("1"));
+        Asserts.assertTrue(p.test(-1d));
+        Asserts.assertTrue(p.test(-1));
+        Asserts.assertTrue(p.test("-1"));
+        Asserts.assertTrue(p.test("."));
+        Asserts.assertTrue(p.test("A"));
+
+        p = TypeCoercions.coerce(MutableMap.of(
+                "greater-than", "-25.45"), DslPredicates.DslPredicate.class);
+        Asserts.assertFalse(p.test(-26.01d));
+        Asserts.assertFalse(p.test(-101.01d));
+        Asserts.assertFalse(p.test(-101));
+        Asserts.assertTrue(p.test(-24.9d));
+        Asserts.assertTrue(p.test(-3.3d));
+        Asserts.assertFalse(p.test(-25.55d));
+        Asserts.assertTrue(p.test(-25.35d));
+        Asserts.assertTrue(p.test(25.35d));
+        Asserts.assertTrue(p.test(1d));
+        Asserts.assertTrue(p.test(1));
+        Asserts.assertTrue(p.test("1"));
+        Asserts.assertTrue(p.test(-1d));
+        Asserts.assertTrue(p.test(-1));
+        Asserts.assertTrue(p.test("-1"));
+        Asserts.assertTrue(p.test("."));  // . comes first alphabetically
+        Asserts.assertTrue(p.test("A"));
+    }
+
     @Test
     public void testLessThanOrEquals() {
         DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of(
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/text/NaturalOrderComparator.java b/utils/common/src/main/java/org/apache/brooklyn/util/text/NaturalOrderComparator.java
index bade244942..9c456577fc 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/text/NaturalOrderComparator.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/text/NaturalOrderComparator.java
@@ -48,6 +48,9 @@ misrepresented as being the original software.
  */
 package org.apache.brooklyn.util.text;
 
+import org.apache.brooklyn.util.exceptions.Exceptions;
+
+import java.math.BigDecimal;
 import java.util.Comparator;
 
 /** comparator which takes two strings and puts them in an order 
@@ -109,6 +112,20 @@ public class NaturalOrderComparator implements Comparator<String> {
 
     @Override
     public int compare(String a, String b) {
+        if (a.startsWith("-") || b.startsWith("-")) {
+            try {
+                BigDecimal ba = new BigDecimal(a);
+                BigDecimal bd = new BigDecimal(b);
+                return ba.compareTo(bd);
+            } catch (Exception e) {
+                Exceptions.propagateIfFatal(e);
+                // otherwise ignore; not decimal
+            }
+        }
+        return compareInternal(a, b);
+    }
+
+    public int compareInternal(String a, String b) {
 
         int ia = 0, ib = 0;
         int nza = 0, nzb = 0;
@@ -167,7 +184,7 @@ public class NaturalOrderComparator implements Comparator<String> {
                     if (ia >= a.length()) return -1;  // only b has more chars
                     if (ib >= b.length()) return 1;  // only a has more chars
                     // both have remaining chars; neither is numeric due to compareRight; recurse into remaining
-                    if ((result = compare(a.substring(ia), b.substring(ib))) != 0) {
+                    if ((result = compareInternal(a.substring(ia), b.substring(ib))) != 0) {
                         return result;
                     }
                 }


[brooklyn-server] 05/05: Merge branch 'misc-workflow-fixes'

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 8b7b77994b9e115cb91a954a21aae1f91c1ddb56
Merge: f19b623bbb 4daf9406a7
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Tue Mar 21 16:10:14 2023 +0000

    Merge branch 'misc-workflow-fixes'

 core/pom.xml                                       |   1 +
 .../workflow/WorkflowExpressionResolution.java     |  29 +++--
 .../core/workflow/WorkflowStepDefinition.java      |   5 +-
 .../WorkflowStepInstanceExecutionContext.java      |   5 +
 .../workflow/steps/external/HttpWorkflowStep.java  |  15 ++-
 .../workflow/steps/flow/RetryWorkflowStep.java     |   5 +
 .../workflow/steps/variables/LoadWorkflowStep.java |   5 +-
 .../steps/variables/SetVariableWorkflowStep.java   |  54 ++++++--
 .../util/core/predicates/DslPredicates.java        |  51 +++++++-
 .../brooklyn/util/core/text/TemplateProcessor.java |  42 ++++++
 .../workflow/WorkflowInputOutputExtensionTest.java | 141 ++++++++++++++++++++-
 .../brooklyn/core/workflow/WorkflowRetryTest.java  |  21 +++
 .../util/core/predicates/DslPredicateTest.java     |  78 ++++++++++++
 .../document-with-interpolated-expression.txt      |   1 +
 .../brooklyn/util/text/NaturalOrderComparator.java |  19 ++-
 15 files changed, 439 insertions(+), 33 deletions(-)