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 0e63887532fc149e3317caf49c27f297f20abf79
Author: Alex Heneveld <[email protected]>
AuthorDate: Wed May 17 10:48:44 2023 +0100

    complete sum for list with coercion
    
    simplifying code in places.
    also convert to integer/long where appropriate.
---
 .../brooklyn/camp/brooklyn/WorkflowYamlTest.java   |   4 +-
 .../steps/variables/SetVariableWorkflowStep.java   |   2 +-
 .../workflow/steps/variables/TransformSum.java     | 111 ---------------------
 .../variables/TransformVariableWorkflowStep.java   |  57 +++++------
 4 files changed, 28 insertions(+), 146 deletions(-)

diff --git 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
index 23ca417594..7cfbc8329e 100644
--- 
a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
+++ 
b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WorkflowYamlTest.java
@@ -1026,7 +1026,7 @@ public class WorkflowYamlTest extends AbstractYamlTest {
 
         entity.sensors().set(Sensors.newStringSensor("good_interpolation"), 
"run");
         EntityAsserts.assertAttributeEventually(entity, 
Sensors.newSensor(Object.class, "mySum"), v -> v!=null);
-        EntityAsserts.assertAttributeEquals(entity, 
Sensors.newSensor(Object.class, "mySum"), 7D);
+        EntityAsserts.assertAttributeEquals(entity, 
Sensors.newSensor(Object.class, "mySum"), 7);
     }
 
     @Test
@@ -1050,7 +1050,7 @@ public class WorkflowYamlTest extends AbstractYamlTest {
 
         entity.sensors().set(Sensors.newStringSensor("good_interpolation"), 
"run");
         EntityAsserts.assertAttributeEventually(entity, 
Sensors.newSensor(Object.class, "mySum"), v -> v!=null);
-        EntityAsserts.assertAttributeEquals(entity, 
Sensors.newSensor(Object.class, "mySum"), 7D);
+        EntityAsserts.assertAttributeEquals(entity, 
Sensors.newSensor(Object.class, "mySum"), 7);
     }
 
     @Test
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 feaccced85..9e1c202521 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
@@ -363,7 +363,7 @@ public class SetVariableWorkflowStep extends 
WorkflowStepDefinition {
 
         Maybe<Double> asDouble(Object x) {
             Maybe<Double> v = TypeCoercions.tryCoerce(x, Double.class);
-            if (v.isPresent() && !Double.isFinite(v.get())) return 
Maybe.absent("Value is undefined");
+            if (v.isPresent() && !Double.isFinite(v.get())) return 
Maybe.absent(() -> new IllegalArgumentException("Value cannot be coerced to 
double: "+v));
             return v;
         }
 
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformSum.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformSum.java
deleted file mode 100644
index 6819a0e492..0000000000
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformSum.java
+++ /dev/null
@@ -1,111 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.brooklyn.core.workflow.steps.variables;
-import static 
org.apache.brooklyn.core.workflow.steps.variables.SetVariableWorkflowStep.ConfigurableInterpolationEvaluation;
-
-import com.fasterxml.jackson.annotation.JsonInclude;
-import com.google.common.collect.Iterables;
-import com.google.common.reflect.TypeToken;
-import org.apache.brooklyn.config.ConfigKey;
-import org.apache.brooklyn.core.config.ConfigKeys;
-import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
-import org.apache.brooklyn.util.core.text.TemplateProcessor;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.util.*;
-
-public class TransformSum extends WorkflowTransformDefault {
-
-    private static final Logger log = 
LoggerFactory.getLogger(TransformSum.class);
-
-    protected static final ConfigKey<TypedValueToSet> VARIABLE = 
ConfigKeys.newConfigKey(TypedValueToSet.class, "variable");
-
-//    @JsonInclude(JsonInclude.Include.NON_EMPTY)
-//    protected final Map<String,Object> input;
-    public static final ConfigKey<SetVariableWorkflowStep.InterpolationMode> 
INTERPOLATION_MODE = 
ConfigKeys.newConfigKey(SetVariableWorkflowStep.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 Object apply(Object v) {
-        log.info("apply called with object: {" + v.getClass().getSimpleName() 
+ "} "  + v);
-        log.info("apply has context: " + this.context);
-
-        if (v==null) return null;
-        checkIsIterable(v);
-        double result = 0;
-        for (Object vi: (Iterable)v) {
-            if (!(vi instanceof Number)) throw new 
IllegalArgumentException("Argument is not a number; cannot compute sum");
-            result += ((Number)vi).doubleValue();
-        }
-
-//        final ConfigurableInterpolationEvaluation evaluation = 
this.getEvaluation();
-//        final Iterable<List<String>> list = (Iterable)v;
-//        final Iterator<List<String>> it = list.iterator();
-//        final int numberOfSums = Iterables.size(list) - 1;
-//
-//        if (numberOfSums < 1) { // add the one item to the sum, if any
-//            for (Object vi: list) {
-//                checkIsNumberOrString(vi);
-//                result += ((Number)vi).doubleValue();
-//            }
-//        } else { // process items via handleAdd() to coerce numbers from 
Strings
-//            Object left = it.next();
-//            Object right;
-//            for (int count=1; count<= numberOfSums; count++) {
-//                right = it.next();
-//                checkIsNumberOrString(left);
-//                checkIsNumberOrString(right);
-//
-//                result += ((Number)evaluation.handleAdd(Arrays.asList(left), 
Arrays.asList(right))).doubleValue();
-//
-//                left = right;
-//            }
-//        }
-        return result;
-    }
-
-//    protected ConfigurableInterpolationEvaluation getEvaluation() {
-//        WorkflowStepInstanceExecutionContext instance = 
this.context.getCurrentStepInstance();
-//        TypeToken<?> type = context.lookupType(this.VARIABLE.getName(), () 
-> null);
-          // need a way to get input/unresolvedValue in order to form the 
ConfigurableInterpolationEvaluation instance
-          // as it is the only way to access the `handleAdd` method.
-//        Object unresolvedValue = input.get(VALUE.getName());
-//
-//        return new ConfigurableInterpolationEvaluation(instance, type, 
unresolvedValue,
-//                instance.getInputOrDefault(INTERPOLATION_MODE),
-//                instance.getInputOrDefault(INTERPOLATION_ERRORS)
-//        );
-//    }
-
-    private static void checkIsIterable(Object o) {
-        if (!(o instanceof Iterable)) throw new 
IllegalArgumentException("Value is not an iterable; cannot take sum");
-    }
-    private static void checkIsNumberOrString(Object o) {
-        if (!(o instanceof Number) && !(o instanceof String)) throw new 
IllegalArgumentException("Argument is not a number; cannot compute sum");
-    }
-}
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformVariableWorkflowStep.java
 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformVariableWorkflowStep.java
index 4383415b2f..5b795c3535 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformVariableWorkflowStep.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformVariableWorkflowStep.java
@@ -32,6 +32,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.collections.*;
+import org.apache.brooklyn.util.core.flags.TypeCoercions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.text.Strings;
 import org.slf4j.Logger;
@@ -168,8 +169,7 @@ public class TransformVariableWorkflowStep extends 
WorkflowStepDefinition {
         });
         TRANSFORMATIONS.put("max", () -> v -> minmax(v, "max", i -> i>0));
         TRANSFORMATIONS.put("min", () -> v -> minmax(v, "min", i -> i<0));
-//        TRANSFORMATIONS.put("sum", () -> v -> sum(v, "sum"));
-        TRANSFORMATIONS.put("sum", () -> new TransformSum());
+        TRANSFORMATIONS.put("sum", () -> v -> sum(v, "sum"));
         TRANSFORMATIONS.put("average", () -> v -> average(v, "average"));
         TRANSFORMATIONS.put("size", () -> v -> size(v, "size"));
         TRANSFORMATIONS.put("get", () -> v -> {
@@ -190,43 +190,36 @@ public class TransformVariableWorkflowStep extends 
WorkflowStepDefinition {
         return result;
     }
 
-    static final Double sum(Object v, String word) {
+    static final Number sum(Object v, String word) {
         if (v==null) return null;
         if (!(v instanceof Iterable)) throw new 
IllegalArgumentException("Value is not an iterable; cannot take "+word);
         double result = 0;
         for (Object vi: (Iterable)v) {
-            if (!(vi instanceof Number)) throw new 
IllegalArgumentException("Argument is not a number; cannot compute "+word);
-            result += ((Number)vi).doubleValue();
+            result += asDouble(vi).get();
         }
-
-        // PROBLEM: context is supplied to getTransform dynamically, but sum() 
is static...
-//        final ConfigurableInterpolationEvaluation evaluation = new 
ConfigurableInterpolationEvaluation(/* ... */);
-//        final Iterable<List<String>> list = (Iterable)v;
-//        final Iterator<List<String>> it = list.iterator();
-//        final int numberOfSums = Iterables.size(list) - 1;
-//
-//        if (numberOfSums < 1) { // add the one item to the sum, if any
-//            for (Object vi: list) {
-//                checkIsSummable(vi);
-//                result += ((Number)vi).doubleValue();
-//            }
-//        } else { // process items via handleAdd() to coerce numbers from 
Strings
-//            Object left = it.next();
-//            Object right;
-//            for (int count=1; count<= numberOfSums; count++) {
-//                right = it.next();
-//                checkIsSummable(left);
-//                checkIsSummable(right);
-//
-//                result += ((Number)evaluation.handleAdd(Arrays.asList(left), 
Arrays.asList(right))).doubleValue();
-//
-//                left = right;
-//            }
-//        }
-        return result;
+        return simplifiedToIntOrLongIfPossible(result);
     }
 
+    static Maybe<Double> asDouble(Object x) {
+        Maybe<Double> v = TypeCoercions.tryCoerce(x, Double.class);
+        if (v.isPresent() && !Double.isFinite(v.get())) return Maybe.absent(() 
-> new IllegalArgumentException("Value cannot be coerced to double: "+v));
+        return v;
+    }
 
+    static Number simplifiedToIntOrLongIfPossible(Number x) {
+        if (x instanceof Integer) return x;
+        if (x instanceof Long) {
+            if (x.longValue() == x.intValue()) return x.intValue();
+            return x;
+        }
+        if (x instanceof Double || x instanceof Float) {
+            double xd = x.doubleValue();
+            if (Math.abs(xd - Math.round(xd)) < 0.000000001) {
+                return simplifiedToIntOrLongIfPossible(Math.round(xd));
+            }
+        }
+        return x;
+    }
 
     static final Integer size(Object v, String word) {
         if (v==null) return null;
@@ -239,7 +232,7 @@ public class TransformVariableWorkflowStep extends 
WorkflowStepDefinition {
         if (v==null) return null;
         Integer count = size(v, "average");
         if (count==null || count==0) throw new IllegalArgumentException("Value 
is empty; cannot take "+word);
-        return sum(v, "average") / count;
+        return simplifiedToIntOrLongIfPossible(asDouble(sum(v, 
"average")).get() / count);
     }
 
     @Override protected Boolean isDefaultIdempotent() { return true; }

Reply via email to