This is an automated email from the ASF dual-hosted git repository.
heneveld pushed a commit to branch expression-parsing
in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit 68597ff34b3a8e720a8eb78d1c94c1ece4ed615c
Author: Alex Heneveld <a...@cloudsoft.io>
AuthorDate: Wed Feb 7 17:06:43 2024 +0000
refactor how map sensors are updated, consistent between let and set-sensor
---
.../steps/appmodel/ClearSensorWorkflowStep.java | 36 ++-
.../steps/appmodel/SetConfigWorkflowStep.java | 22 +-
.../steps/appmodel/SetSensorWorkflowStep.java | 238 +++++-----------
.../steps/variables/SetVariableWorkflowStep.java | 57 +---
.../variables/TransformSetWorkflowVariable.java | 2 +-
.../variables/TransformVariableWorkflowStep.java | 27 +-
.../workflow/utils/WorkflowSettingItemsUtils.java | 312 +++++++++++++++++++++
.../core/workflow/WorkflowMapAndListTest.java | 243 +++++++++++++---
.../WorkflowNestedAndCustomExtensionTest.java | 3 +-
9 files changed, 651 insertions(+), 289 deletions(-)
diff --git
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/ClearSensorWorkflowStep.java
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/ClearSensorWorkflowStep.java
index 713735b1b8..e04b47b591 100644
---
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/ClearSensorWorkflowStep.java
+++
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/ClearSensorWorkflowStep.java
@@ -18,6 +18,10 @@
*/
package org.apache.brooklyn.core.workflow.steps.appmodel;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
import com.google.common.collect.Iterables;
import com.google.common.reflect.TypeToken;
import org.apache.brooklyn.api.entity.Entity;
@@ -25,19 +29,12 @@ import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
import org.apache.brooklyn.core.entity.EntityInternal;
import org.apache.brooklyn.core.sensor.Sensors;
-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.core.workflow.utils.WorkflowSettingItemsUtils;
import org.apache.brooklyn.util.collections.MutableList;
-import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.collections.MutableSet;
import org.apache.brooklyn.util.guava.Maybe;
-import org.apache.brooklyn.util.text.Strings;
-
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
+import org.apache.commons.lang3.tuple.Pair;
public class ClearSensorWorkflowStep extends WorkflowStepDefinition {
@@ -54,27 +51,26 @@ public class ClearSensorWorkflowStep extends
WorkflowStepDefinition {
protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
EntityValueToSet sensor = context.getInput(SENSOR);
if (sensor==null) throw new IllegalArgumentException("Sensor name is
required");
- String sensorNameFull =
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
sensor.name, String.class);
- if (Strings.isBlank(sensorNameFull)) throw new
IllegalArgumentException("Sensor name is required");
- List<Object> sensorNameIndexes = MutableList.of();
- String sensorNameBase =
SetSensorWorkflowStep.extractSensorNameBaseAndPopulateIndices(sensorNameFull,
sensorNameIndexes);
+ Pair<String, List<Object>> sensorNameAndIndices =
WorkflowSettingItemsUtils.resolveNameAndBracketedIndices(context, sensor.name,
false);
+ if (sensorNameAndIndices==null) throw new
IllegalArgumentException("Sensor name is required");
TypeToken<?> type = context.lookupType(sensor.type, () ->
TypeToken.of(Object.class));
Entity entity = sensor.entity;
if (entity==null) entity = context.getEntity();
- if (sensorNameIndexes.isEmpty()) {
- ((EntityInternal)
entity).sensors().remove(Sensors.newSensor(Object.class, sensorNameFull));
+ // TODO use WorkflowSettingItemsUtils
+ if (sensorNameAndIndices.getRight().isEmpty()) {
+ ((EntityInternal)
entity).sensors().remove(Sensors.newSensor(Object.class,
sensorNameAndIndices.getLeft()));
} else {
- ((EntityInternal)
entity).sensors().modify(Sensors.newSensor(Object.class, sensorNameBase), old
-> {
+ ((EntityInternal)
entity).sensors().modify(Sensors.newSensor(Object.class,
sensorNameAndIndices.getLeft()), old -> {
boolean setLast = false;
- Object newTarget = SetSensorWorkflowStep.makeMutable(old,
sensorNameIndexes);
+ Object newTarget =
WorkflowSettingItemsUtils.makeMutableOrUnchangedDefaultingToMap(old);
Object target = newTarget;
- MutableList<Object> indexes =
MutableList.copyOf(sensorNameIndexes);
+ MutableList<Object> indexes =
MutableList.copyOf(sensorNameAndIndices.getRight());
while (!indexes.isEmpty()) {
Object i = indexes.remove(0);
boolean isLast = indexes.isEmpty();
@@ -93,7 +89,7 @@ public class ClearSensorWorkflowStep extends
WorkflowStepDefinition {
} else {
nextTarget = ((Map) target).get(i);
if (nextTarget==null) break;
- ((Map) target).put(i,
SetSensorWorkflowStep.makeMutable(nextTarget, indexes));
+ ((Map) target).put(i,
WorkflowSettingItemsUtils.makeMutableOrUnchangedDefaultingToMap(nextTarget));
}
} else if (target instanceof Iterable && i instanceof
Integer) {
@@ -121,7 +117,7 @@ public class ClearSensorWorkflowStep extends
WorkflowStepDefinition {
break;
} else {
Object t0 = Iterables.get((Iterable) target, ii);
- nextTarget = SetSensorWorkflowStep.makeMutable(t0,
indexes);
+ nextTarget =
WorkflowSettingItemsUtils.makeMutableOrUnchangedDefaultingToMap(t0);
if (t0!=nextTarget) {
if (!(target instanceof List)) throw new
IllegalStateException("Cannot set numerical position index in a non-list
collection (and was not otherwise known as mutable; e.g. use MutableSet):
"+target);
((List) target).set(ii, nextTarget);
diff --git
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetConfigWorkflowStep.java
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetConfigWorkflowStep.java
index 958f94adb4..73b1e9fd4c 100644
---
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetConfigWorkflowStep.java
+++
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetConfigWorkflowStep.java
@@ -18,14 +18,16 @@
*/
package org.apache.brooklyn.core.workflow.steps.appmodel;
+import java.util.List;
+
import com.google.common.reflect.TypeToken;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
-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.text.Strings;
+import org.apache.brooklyn.core.workflow.utils.WorkflowSettingItemsUtils;
+import org.apache.commons.lang3.tuple.Pair;
public class SetConfigWorkflowStep extends WorkflowStepDefinition {
@@ -43,17 +45,19 @@ public class SetConfigWorkflowStep extends
WorkflowStepDefinition {
protected Object doTaskBody(WorkflowStepInstanceExecutionContext context) {
EntityValueToSet config = context.getInput(CONFIG);
if (config ==null) throw new IllegalArgumentException("Config key name
is required");
- String configName =
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
config.name, String.class);
- if (Strings.isBlank(configName)) throw new
IllegalArgumentException("Config key name is required");
+
+ Pair<String, List<Object>> nameAndIndices =
WorkflowSettingItemsUtils.resolveNameAndBracketedIndices(context, config.name,
false);
+ if (nameAndIndices==null) throw new IllegalArgumentException("Config
key name is required");
+
// see note on type in SetSensorWorkflowStep
TypeToken<?> type = context.lookupType(config.type, () ->
TypeToken.of(Object.class));
Object resolvedValue = context.getInput(VALUE.getName(), type);
- Entity entity = config.entity;
- if (entity==null) entity = context.getEntity();
- Object oldValue = entity.config().set((ConfigKey<Object>)
ConfigKeys.newConfigKey(type, configName), resolvedValue);
+ Entity entity = config.entity!=null ? config.entity :
context.getEntity();
- context.noteOtherMetadata("Value set", resolvedValue);
- if (oldValue!=null) context.noteOtherMetadata("Previous value",
oldValue);
+ Pair<Object, Object> oldValues =
WorkflowSettingItemsUtils.setAtIndex(nameAndIndices, true, (_oldValue) ->
resolvedValue,
+ name -> entity.config().get((ConfigKey<Object>)
ConfigKeys.newConfigKey(type, name)),
+ (name, value) -> entity.config().set((ConfigKey<Object>)
ConfigKeys.newConfigKey(type, name), value));
+ WorkflowSettingItemsUtils.noteValueSetNestedMetadata(context,
nameAndIndices, resolvedValue, oldValues);
return context.getPreviousStepOutput();
}
diff --git
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetSensorWorkflowStep.java
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetSensorWorkflowStep.java
index 72d7a11ebb..3b0d512b52 100644
---
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetSensorWorkflowStep.java
+++
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/appmodel/SetSensorWorkflowStep.java
@@ -18,43 +18,41 @@
*/
package org.apache.brooklyn.core.workflow.steps.appmodel;
-import com.google.common.collect.Iterables;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
+
+import com.google.common.base.Suppliers;
import com.google.common.reflect.TypeToken;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.sensor.AttributeSensor;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
-import org.apache.brooklyn.core.entity.AbstractEntity;
+import org.apache.brooklyn.core.entity.AbstractEntity.BasicSensorSupport;
import org.apache.brooklyn.core.resolve.jackson.WrappedValue;
import org.apache.brooklyn.core.sensor.Sensors;
-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.MutableList;
-import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.core.workflow.utils.WorkflowSettingItemsUtils;
import org.apache.brooklyn.util.core.predicates.DslPredicates;
+import
org.apache.brooklyn.util.core.predicates.DslPredicates.DslEntityPredicateDefault;
import org.apache.brooklyn.util.guava.Maybe;
-import org.apache.brooklyn.util.text.StringEscapes;
-import org.apache.brooklyn.util.text.Strings;
+import org.apache.commons.lang3.tuple.Pair;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
-
public class SetSensorWorkflowStep extends WorkflowStepDefinition {
private static final Logger LOG =
LoggerFactory.getLogger(SetSensorWorkflowStep.class);
+ static final boolean ALWAYS_USE_SENSOR_MODIFY = true;
+
public static final String SHORTHAND = "[ ${sensor.type} ] ${sensor.name}
[ \"=\" ${value...} ]";
public static final ConfigKey<EntityValueToSet> SENSOR =
ConfigKeys.newConfigKey(EntityValueToSet.class, "sensor");
public static final ConfigKey<Object> VALUE =
ConfigKeys.newConfigKey(Object.class, "value");
- public static final ConfigKey<DslPredicates.DslPredicate> REQUIRE =
ConfigKeys.newConfigKey(DslPredicates.DslPredicate.class, "require");
+ public static final ConfigKey<DslPredicates.DslPredicate> REQUIRE =
ConfigKeys.newConfigKey(DslPredicates.DslPredicate.class, "require",
+ "Require a condition in order to set the value; if the condition
is not satisfied, the sensor is not set");
@Override
public void populateFromShorthand(String expression) {
@@ -84,173 +82,83 @@ public class SetSensorWorkflowStep extends
WorkflowStepDefinition {
EntityValueToSet sensor = context.getInput(SENSOR);
if (sensor==null) throw new IllegalArgumentException("Sensor name is
required");
- String sensorNameFull =
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT,
sensor.name, String.class);
- if (Strings.isBlank(sensorNameFull)) throw new
IllegalArgumentException("Sensor name is required");
-
- List<Object> sensorNameIndexes = MutableList.of();
- String sensorNameBase =
extractSensorNameBaseAndPopulateIndices(sensorNameFull, sensorNameIndexes);
+ Pair<String, List<Object>> sensorNameAndIndices =
WorkflowSettingItemsUtils.resolveNameAndBracketedIndices(context, sensor.name,
false);
+ if (sensorNameAndIndices==null) throw new
IllegalArgumentException("Sensor name is required");
- TypeToken<?> type = context.lookupType(sensor.type, () ->
TypeToken.of(Object.class));
+ final TypeToken<?> type = context.lookupType(sensor.type, () ->
TypeToken.of(Object.class));
final Entity entity = sensor.entity!=null ? sensor.entity :
context.getEntity();
- AttributeSensor<Object> sensorBase = (AttributeSensor<Object>)
Sensors.newSensor(type, sensorNameBase);
- AtomicReference<Object> resolvedValue = new AtomicReference<>();
- Object oldValue;
- Runnable resolve = () -> {
+ Supplier<Object> resolveOnceValueSupplier = Suppliers.memoize(() -> {
// // might need to be careful if defined type is different or more
generic than type specified here;
// // but that should be fine as XML persistence preserves types
// Sensor<?> sd = entity.getEntityType().getSensor(sensorName);
// if (!type.isSupertypeOf(sd.getTypeToken())) {
// ...
// }
+ return context.getInput(VALUE.getName(), type);
+ });
- resolvedValue.set(context.getInput(VALUE.getName(), type));
- };
-
+ AttributeSensor<Object> sensorBase = (AttributeSensor<Object>)
Sensors.newSensor(type, sensorNameAndIndices.getLeft());
DslPredicates.DslPredicate require = context.getInput(REQUIRE);
- if (require==null && sensorNameIndexes.isEmpty()) {
- resolve.run();
- oldValue = entity.sensors().set(sensorBase, resolvedValue.get());
- } else {
- oldValue = entity.sensors().modify(sensorBase, oldBase -> {
- if (require!=null) {
- Object old = oldBase;
- MutableList<Object> indexes =
MutableList.copyOf(sensorNameIndexes);
- while (!indexes.isEmpty()) {
- Object i = indexes.remove(0);
- if (old == null) break;
- if (old instanceof Map) old = ((Map) old).get(i);
- else if (old instanceof Iterable && i instanceof
Integer) {
- int ii = (Integer)i;
- int size = Iterables.size((Iterable) old);
- if (ii==-1) ii = size-1;
- old = (ii<0 || ii>=size) ? null :
Iterables.get((Iterable) old, ii);
- } else {
- throw new IllegalArgumentException("Cannot find
argument '" + i + "' in " + old);
- }
- }
-
- if (old == null && !((AbstractEntity.BasicSensorSupport)
entity.sensors()).contains(sensorBase.getName())) {
- DslPredicates.DslEntityPredicateDefault requireTweaked
= new DslPredicates.DslEntityPredicateDefault();
- requireTweaked.sensor = sensorNameFull;
- requireTweaked.check = WrappedValue.of(require);
- if (!requireTweaked.apply(entity)) {
- throw new SensorRequirementFailedAbsent("Sensor "
+ sensorNameFull + " unset or unavailable when there is a non-absent
requirement");
- }
- } else {
- if (!require.apply(old)) {
- throw new SensorRequirementFailed("Sensor " +
sensorNameFull + " value does not match requirement", old);
- }
- }
- }
-
- resolve.run();
-
- // now set
- Object result;
-
- if (!sensorNameIndexes.isEmpty()) {
- result = oldBase;
-
- // ensure mutable
- result = makeMutable(result, sensorNameIndexes);
-
- Object target = result;
- MutableList<Object> indexes =
MutableList.copyOf(sensorNameIndexes);
- while (!indexes.isEmpty()) {
- Object i = indexes.remove(0);
- boolean isLast = indexes.isEmpty();
- Object nextTarget;
+ AtomicReference<Pair<Object, Object>> oldValues = new
AtomicReference<>();
+ if (!ALWAYS_USE_SENSOR_MODIFY && require == null &&
sensorNameAndIndices.getRight().isEmpty()) {
+ // can do simple if not using 'require' and no indices - though
there is no reason not to
+ // and if there is some special value which does an operation on
the sensor, it's nice if we do
+ Pair<Object, Object> oldValuesP =
WorkflowSettingItemsUtils.setAtIndex(sensorNameAndIndices, true,
+ _oldInner -> resolveOnceValueSupplier.get(),
+ // outer getter
+ _name -> entity.sensors().get(sensorBase),
+ // outer setter
+ (_name, nv) -> entity.sensors().set(sensorBase, nv)
+ );
+ oldValues.set(oldValuesP);
- if (target instanceof Map) {
- nextTarget = ((Map) target).get(i);
- if (nextTarget==null || isLast || !(nextTarget
instanceof MutableMap)) {
- // ensure mutable
- nextTarget = isLast ? resolvedValue.get() :
makeMutable(nextTarget, indexes);
- ((Map) target).put(i, nextTarget);
- }
-
- } else if (target instanceof Iterable && i instanceof
Integer) {
- int ii = (Integer)i;
- int size = Iterables.size((Iterable) target);
- if (ii==-1) ii = size-1;
- boolean outOfBounds = ii < 0 || ii >= size;
- nextTarget = outOfBounds ? null :
Iterables.get((Iterable) target, ii);
-
- if (nextTarget==null || isLast || (!(nextTarget
instanceof MutableMap) && !(nextTarget instanceof MutableSet) && !(nextTarget
instanceof MutableList))) {
- nextTarget = isLast ? resolvedValue.get() :
makeMutable(nextTarget, indexes);
- if (outOfBounds) {
- ((Collection) target).add(nextTarget);
- } else {
- if (!(target instanceof List)) throw new
IllegalStateException("Cannot set numerical position index in a non-list
collection (and was not otherwise known as mutable; e.g. use MutableSet):
"+target);
- ((List) target).set(ii, nextTarget);
+ } else {
+ // with 'require' we contractually need to do it all in the modify
loop,
+ // performing the check (on the inner value if there are indices);
+ // even without, there might be concurrent blocks updating the
same map/list if indexes are supplied
+
+ // see testBeefySensorRequireForAtomicityAndConfigCountsMap for a
thorough test
+ // (look for references to SetSensorWorkflowStep.REQUIRE)
+
+ entity.sensors().modify(sensorBase,
+ (oldOuterX) -> {
+ AtomicReference<Object> newValue = new
AtomicReference<>();
+ Pair<Object, Object> oldValuesP =
WorkflowSettingItemsUtils.setAtIndex(sensorNameAndIndices, true,
+ oldInner -> {
+ if (require!=null) {
+ if (oldInner == null &&
!((BasicSensorSupport) entity.sensors()).contains(sensorBase.getName())) {
+ DslEntityPredicateDefault
requireTweaked = new DslEntityPredicateDefault();
+ requireTweaked.sensor =
sensorNameAndIndices.getLeft();
+ requireTweaked.check =
WrappedValue.of(require);
+ if (!requireTweaked.apply(entity))
{
+ throw new
SensorRequirementFailedAbsent("Sensor " + sensorNameAndIndices.getLeft() + "
unset or unavailable when there is a non-absent requirement");
+ }
+ } else {
+ if (!require.apply(oldInner)) {
+ throw new
SensorRequirementFailed("Sensor " + sensorNameAndIndices.getLeft() + " value
does not match requirement", oldInner);
+ }
+ }
+ }
+
+ return resolveOnceValueSupplier.get();
+ },
+ // outer getter
+ _name -> oldOuterX,
+ // outer setter
+ (_name, nv) -> {
+ newValue.set(nv);
+ return oldOuterX;
}
- }
-
- } else {
- throw new IllegalArgumentException("Cannot find
argument '" + i + "' in " + target);
- }
- target = nextTarget;
- }
- } else {
- result = resolvedValue.get();
- }
-
- return Maybe.of(result);
- });
+ );
+ oldValues.set(oldValuesP);
+ return Maybe.of(newValue.get());
+ });
}
- context.noteOtherMetadata("Value set", resolvedValue.get());
- if (oldValue!=null) context.noteOtherMetadata("Previous value",
oldValue);
-
+ WorkflowSettingItemsUtils.noteValueSetNestedMetadata(context,
sensorNameAndIndices, resolveOnceValueSupplier.get(), oldValues.get());
return context.getPreviousStepOutput();
}
- static String extractSensorNameBaseAndPopulateIndices(String
sensorNameFull, List<Object> sensorNameIndexes) {
- int bracket = sensorNameFull.indexOf('[');
- String sensorNameBase;
- if (bracket > 0) {
- sensorNameBase = sensorNameFull.substring(0, bracket);
- String brackets = sensorNameFull.substring(bracket);
- while (!brackets.isEmpty()) {
- if (!brackets.startsWith("[")) throw new
IllegalArgumentException("Expected '[' for sensor index");
- brackets = brackets.substring(1).trim();
- int bi = brackets.indexOf(']');
- if (bi<0) throw new IllegalArgumentException("Mismatched ']'
in sensor name");
- String bs = brackets.substring(0, bi).trim();
- if (bs.startsWith("\"") || bs.startsWith("\'")) bs =
StringEscapes.BashStringEscapes.unwrapBashQuotesAndEscapes(bs);
- else if (bs.matches("-? *[0-9]+")) {
- sensorNameIndexes.add(Integer.parseInt(bs));
- bs = null;
- }
- if (bs!=null) {
- sensorNameIndexes.add(bs);
- }
- brackets = brackets.substring(bi+1).trim();
- }
- } else if (bracket == 0) {
- throw new IllegalArgumentException("Sensor name cannot start with
'['");
- } else {
- sensorNameBase = sensorNameFull;
- }
- return sensorNameBase;
- }
-
- static Object makeMutable(Object x, List<Object> indexesRemaining) {
- if (x==null) {
- // look ahead to see if it should be a list
- if (indexesRemaining!=null && !indexesRemaining.isEmpty() &&
indexesRemaining.get(0) instanceof Integer) return MutableList.of();
- return MutableMap.of();
- }
-
- if (x instanceof Set) {
- if (!(x instanceof MutableSet)) return MutableSet.copyOf((Set) x);
- return x;
- }
- if (x instanceof Map && !(x instanceof MutableMap)) return
MutableMap.copyOf((Map) x);
- else if (x instanceof Iterable && !(x instanceof MutableList)) return
MutableList.copyOf((Iterable) x);
- return x;
- }
-
@Override protected Boolean isDefaultIdempotent() { return true; }
}
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 f363261e1f..2aad2bbeeb 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
@@ -27,6 +27,7 @@ import
org.apache.brooklyn.core.workflow.WorkflowExecutionContext;
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.core.workflow.utils.WorkflowSettingItemsUtils;
import org.apache.brooklyn.util.collections.*;
import org.apache.brooklyn.util.core.flags.TypeCoercions;
import org.apache.brooklyn.util.core.predicates.DslPredicates;
@@ -38,6 +39,7 @@ import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.time.Duration;
import org.apache.brooklyn.util.time.Timestamp;
import org.apache.commons.lang3.tuple.MutablePair;
+import org.apache.commons.lang3.tuple.Pair;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -104,52 +106,23 @@ public class SetVariableWorkflowStep extends
WorkflowStepDefinition {
Object resolvedValue = new
ConfigurableInterpolationEvaluation(context, type, unresolvedValue,
context.getInputOrDefault(INTERPOLATION_MODE),
context.getInputOrDefault(INTERPOLATION_ERRORS)).evaluate();
- Object oldValue = setWorkflowScratchVariableDotSeparated(context,
name, resolvedValue);
- // these are easily inferred from workflow vars
-// context.noteOtherMetadata("Value set", resolvedValue);
-// if (oldValue!=null) context.noteOtherMetadata("Previous value",
oldValue);
+ setWorkflowScratchVariableDotSeparated(context, name, resolvedValue,
+ // metadata is easily inferred from workflow vars, and they
can use a lot of persistence space, so skip
+ false);
return context.getPreviousStepOutput();
}
- static Object
setWorkflowScratchVariableDotSeparated(WorkflowStepInstanceExecutionContext
context, String name, Object resolvedValue) {
- Object oldValue;
- if (name.contains(".")) {
- String[] names = name.split("\\.");
- String names0 = names[0];
- if ("output".equals(names0)) throw new
IllegalArgumentException("Cannot set subfield in output"); // catch common
error
- Object h =
context.getWorkflowExectionContext().getWorkflowScratchVariables().get(names0);
- if (!(h instanceof Map)) throw new
IllegalArgumentException("Cannot set " + name + " because " + names0 + " is " +
(h == null ? "unset" : "not a map"));
- for (int i = 1; i < names.length - 1; i++) {
- Object hi = ((Map<?, ?>) h).get(names[i]);
- if (hi == null) {
- hi = MutableMap.of();
- ((Map) h).put(names[i], hi);
- } else if (!(hi instanceof Map))
- throw new IllegalArgumentException("Cannot set " + name +
" because " + names[i] + " is not a map");
- h = hi;
- }
- oldValue = ((Map) h).put(names[names.length - 1], resolvedValue);
- } else if (name.contains("[")) {
- String[] names = name.split("((?<=\\[|\\])|(?=\\[|\\]))");
- if (names.length != 4 || !"[".equals(names[1]) ||
!"]".equals(names[3])) {
- throw new IllegalArgumentException("Invalid list index
specifier " + name);
- }
- String listName = names[0];
- int listIndex = Integer.parseInt(names[2]);
- Object o =
context.getWorkflowExectionContext().getWorkflowScratchVariables().get(listName);
- if (!(o instanceof List))
- throw new IllegalArgumentException("Cannot set " + name + "
because " + listName + " is " + (o == null ? "unset" : "not a list"));
-
- List l = MutableList.copyOf(((List)o));
- if (listIndex < 0 || listIndex >= l.size()) {
- throw new IllegalArgumentException("Invalid list index " +
listIndex);
- }
- oldValue = l.set(listIndex, resolvedValue);
-
context.getWorkflowExectionContext().updateWorkflowScratchVariable(listName, l);
- } else {
- oldValue =
context.getWorkflowExectionContext().updateWorkflowScratchVariable(name,
resolvedValue);
+ static Pair<Object,Object>
setWorkflowScratchVariableDotSeparated(WorkflowStepInstanceExecutionContext
context, String nameFull, Object resolvedValue, boolean noteMetadata) {
+ Pair<String, List<Object>> np =
WorkflowSettingItemsUtils.extractNameAndDotOrBracketedIndices(nameFull);
+
+ Pair<Object, Object> result = WorkflowSettingItemsUtils.setAtIndex(np,
true, (_prevValue) -> resolvedValue,
+ (k) ->
context.getWorkflowExectionContext().getWorkflowScratchVariables().get(k),
+ (k, v) ->
context.getWorkflowExectionContext().updateWorkflowScratchVariable(k, v));
+ if (noteMetadata) {
+ // not usually done, see call sites of this method
+ WorkflowSettingItemsUtils.noteValueSetNestedMetadata(context, np,
resolvedValue, result);
}
- return oldValue;
+ return result;
}
private enum LetMergeMode { NONE, SHALLOW, DEEP }
diff --git
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformSetWorkflowVariable.java
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformSetWorkflowVariable.java
index 73cc122ec2..10d7d80ab2 100644
---
a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformSetWorkflowVariable.java
+++
b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformSetWorkflowVariable.java
@@ -39,7 +39,7 @@ public class TransformSetWorkflowVariable extends
WorkflowTransformDefault {
@Override
public Object apply(Object v) {
String nameToSet =
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING,
name, String.class);
-
SetVariableWorkflowStep.setWorkflowScratchVariableDotSeparated(stepContext,
nameToSet, v);
+
SetVariableWorkflowStep.setWorkflowScratchVariableDotSeparated(stepContext,
nameToSet, v, false);
return v;
}
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 7f7c2f8254..09907ffe38 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
@@ -18,6 +18,16 @@
*/
package org.apache.brooklyn.core.workflow.steps.variables;
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+
import com.google.common.collect.Iterables;
import com.google.common.reflect.TypeToken;
import org.apache.brooklyn.api.mgmt.ManagementContext;
@@ -42,16 +52,6 @@ import org.apache.brooklyn.util.time.Duration;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import javax.annotation.Nullable;
-import java.util.Arrays;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.function.Function;
-import java.util.function.Predicate;
-import java.util.function.Supplier;
-import java.util.stream.Collectors;
-
import static
org.apache.brooklyn.core.workflow.WorkflowExecutionContext.STEP_TARGET_NAME_FOR_END;
import static
org.apache.brooklyn.core.workflow.steps.variables.SetVariableWorkflowStep.setWorkflowScratchVariableDotSeparated;
@@ -218,12 +218,7 @@ public class TransformVariableWorkflowStep extends
WorkflowStepDefinition {
if (setVariableAtEnd) {
if (varName==null) throw new IllegalStateException("Variable name
not specified when setting variable"); // shouldn't happen
- setWorkflowScratchVariableDotSeparated(context, varName, v);
- // easily inferred from workflow vars, now that updates are stored
separately
-// Object oldValue =
-// <above> setWorkflowScratchVariableDotSeparated(context,
varName, v);
-// context.noteOtherMetadata("Value set", v);
-// if (oldValue != null) context.noteOtherMetadata("Previous
value", oldValue);
+ setWorkflowScratchVariableDotSeparated(context, varName, v, false);
if (context.getOutput()!=null) throw new
IllegalStateException("Transform that produces output results cannot be used
when setting a variable");
if (STEP_TARGET_NAME_FOR_END.equals(context.next)) throw new
IllegalStateException("Return transform cannot be used when setting a
variable");
diff --git
a/core/src/main/java/org/apache/brooklyn/core/workflow/utils/WorkflowSettingItemsUtils.java
b/core/src/main/java/org/apache/brooklyn/core/workflow/utils/WorkflowSettingItemsUtils.java
new file mode 100644
index 0000000000..10095c660e
--- /dev/null
+++
b/core/src/main/java/org/apache/brooklyn/core/workflow/utils/WorkflowSettingItemsUtils.java
@@ -0,0 +1,312 @@
+/*
+ * 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.utils;
+
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiFunction;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Supplier;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import
org.apache.brooklyn.core.workflow.WorkflowExpressionResolution.WorkflowExpressionStage;
+import org.apache.brooklyn.core.workflow.WorkflowStepInstanceExecutionContext;
+import
org.apache.brooklyn.core.workflow.utils.ExpressionParserImpl.CharactersCollectingParseMode;
+import org.apache.brooklyn.core.workflow.utils.ExpressionParserImpl.ParseNode;
+import
org.apache.brooklyn.core.workflow.utils.ExpressionParserImpl.ParseNodeOrValue;
+import org.apache.brooklyn.core.workflow.utils.ExpressionParserImpl.ParseValue;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.commons.lang3.tuple.Pair;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** convenience class for the complexities of setting values, especially
nested values */
+public class WorkflowSettingItemsUtils {
+
+ /**
+ * variable indices are not obvious -- currently you should write
x['${index}'] for maps and x[${index}] for lists.
+ * but the quotes on the former might suggest it should be a literal value.
+ * if you want a literal (unresolved) key of the form ${...}
+ * you have to put that into another variable, or use $brooklyn:literal.
+ *
+ * furthermore x[index] is also ambiguous -- probably that should be the
way to imply using index as a variable.
+ * but currently (and historially) we have allowed it as a string literal
"index".
+ * we now warn on that usage, but we might want to switch it, and respect
its type as a string or integer (for map or list).
+ *
+ * this constant can be used to find usages.
+ */
+ public static final boolean TODO_ALLOW_VARIABLES_IN_INDICES = false;
+
+ public static final String VALUE_SET = "Value set";
+ public static final String PREVIOUS_VALUE = "Previous value";
+ public static final String PREVIOUS_VALUE_OUTER = "Previous value (outer)";
+
+ private static final Logger log =
LoggerFactory.getLogger(WorkflowSettingItemsUtils.class);
+
+ public static Pair<String, List<Object>>
resolveNameAndBracketedIndices(WorkflowStepInstanceExecutionContext context,
String expression, boolean treatDotAsSubkeySeparator) {
+ if (TODO_ALLOW_VARIABLES_IN_INDICES) {
+ // this should be a more sophisicated resolution, using
ExpressionParser
+ // values before a bracket should be taken as literals,
interpolated expressions resolved, maybe quotes unquoted without resolving;
+ // and more importantly in the brackets unquoted values should be
taken as expressions and resolved;
+ // not sure what to do for quotes, there is an argument to allow
"count_${n}" but also to a quote as a literal.
+ throw new UnsupportedOperationException();
+ }
+ String resolved = context.resolve(WorkflowExpressionStage.STEP_INPUT,
expression, String.class);
+ return expressionParseNameAndIndices(resolved,
treatDotAsSubkeySeparator);
+ }
+
+ public static Pair<String, List<Object>>
extractNameAndDotOrBracketedIndices(String nameFull) {
+ if (TODO_ALLOW_VARIABLES_IN_INDICES) {
+ // callers to this need to be updated
+ throw new UnsupportedOperationException();
+ }
+ return expressionParseNameAndIndices(nameFull, true);
+ }
+
+ public static Pair<String, List<Object>>
expressionParseNameAndIndices(String nameFull, boolean
treatDotAsSubkeySeparator) {
+ CharactersCollectingParseMode DOT = new
CharactersCollectingParseMode("dot", '.');
+ ExpressionParserImpl ep = ExpressionParser
+ .newDefaultAllowingUnquotedLiteralValues()
+
.includeGroupingBracketsAtUsualPlaces(ExpressionParser.SQUARE_BRACKET);
+
+ if (treatDotAsSubkeySeparator)
ep.includeAllowedTopLevelTransition(DOT);
+
+ ParseNode p = ep.parse(nameFull).get();
+ Iterator<ParseNodeOrValue> contents = p.getContents().iterator();
+ if (!contents.hasNext()) throw new IllegalArgumentException("Initial
identifier is required");
+
+ ParseNodeOrValue nameBaseC = contents.next();
+ if (nameBaseC.isParseNodeMode(ExpressionParser.INTERPOLATED,
ExpressionParser.SQUARE_BRACKET))
+ throw new IllegalArgumentException("Initial part of identifier
cannot be an expression or reference");
+
+ String nameBase = ExpressionParser.getUnquoted(nameBaseC).trim();
+ List<Object> indices = MutableList.of();
+
+ while (contents.hasNext()) {
+ ParseNodeOrValue t = contents.next();
+ if (t.isParseNodeMode(DOT)) {
+ if (!contents.hasNext()) throw new
IllegalArgumentException("Cannot end with a dot");
+ ParseNodeOrValue next = contents.next();
+ if (next.isParseNodeMode(ExpressionParser.SQUARE_BRACKET)) {
+ // continue to next block; allow foo.['x'].[adfsads]
+ t = next;
+ } else if (next.isParseNodeMode(ParseValue.MODE)) {
+ // only a simple value is allowed
+ indices.add(((ParseValue)next).getContents());
+ t = null;
+
+ } else {
+ throw new IllegalArgumentException("Cannot contain this
type of object: "+next);
+ }
+ }
+ if (t!=null && t.isParseNodeMode(ExpressionParser.SQUARE_BRACKET))
{
+ List<ParseNodeOrValue> nest = ExpressionParser.trimWhitespace(
((ParseNode) t).getContents() );
+ Object index;
+ if (nest.size()>1) throw new
IllegalArgumentException("Bracketed expression must contain a single string or
number");
+ if (nest.isEmpty()) index = "";
+ else {
+ ParseNodeOrValue n = nest.get(0);
+
+ if (n instanceof ParseValue) {
+ String v = ((ParseValue) n).getContents().trim();
+ index = asInteger(v).asType(Object.class).or(() -> {
+ if (TODO_ALLOW_VARIABLES_IN_INDICES) {
+ // resolve?
+ throw new UnsupportedOperationException();
+ } else {
+ log.warn("Index to " + nameFull + " should be
quoted; allowing unquoted for legacy compatibility");
+ }
+ return v;
+ });
+ } else if
(n.isParseNodeMode(ExpressionParser.SINGLE_QUOTE,
ExpressionParser.DOUBLE_QUOTE)) {
+ index = ExpressionParser.getUnquoted(n);
+ } else {
+ throw new IllegalArgumentException("Cannot contain
this type of object bracketed: " + n);
+ }
+ }
+ indices.add(index);
+ }
+ }
+
+ return Pair.of(nameBase, indices);
+ }
+
+ public static Maybe<Integer> asInteger(Object x) {
+ if (x instanceof Integer) return Maybe.of((Integer)x);
+ if (x instanceof String) {
+ if (((String)x).matches("-? *[0-9]+")) return
Maybe.of(Integer.parseInt((String)x));
+ }
+ return Maybe.absent("Cannot make an integer out of: "+x);
+ }
+
+ public static <T> Maybe<T> ensureMutable(T x) {
+ return makeMutable(x, false);
+ }
+ public static <T> Maybe<T> makeMutableCopy(T x) {
+ return makeMutable(x, true);
+ }
+ private static <T> Maybe<T> makeMutable(T x, boolean
alwaysCopyEvenIfMutable) {
+ Object result = makeMutable(x, alwaysCopyEvenIfMutable, () ->
Maybe.absent("Cannot make a mutable object out of null"), (y) ->
Maybe.absent("Cannot make a mutable object out of " + y.getClass()));
+ if (result instanceof Maybe) return (Maybe)result;
+ return Maybe.of((T)result);
+ }
+ public static Object makeMutableOrUnchangedDefaultingToMap(Object x) {
+ return makeMutable(x, false, () -> MutableMap.of(), v -> v);
+ }
+ public static Maybe<Object> makeMutableOrUnchangedForIndex(Object x,
boolean alwaysCopyEvenIfMutable, Object index) {
+ return Maybe.ofDisallowingNull(makeMutable(x, alwaysCopyEvenIfMutable,
() -> {
+ // number or empty string means list
+ if (index instanceof Integer || "".equals(index)) return
MutableList.of();
+ // string is a map
+ if (index instanceof String) return MutableMap.of();
+ // other things not supported
+ return null;
+ }, v -> v));
+ }
+ public static Object makeMutable(@Nullable Object x, boolean
alwaysCopyEvenIfMutable, @Nonnull Supplier<Object> ifNull, @Nonnull
Function<Object,Object> ifNotIterableOrMap) {
+ if (x==null) {
+ return ifNull.get();
+ }
+
+ if (x instanceof Set) return (!alwaysCopyEvenIfMutable && x instanceof
MutableSet) ? x : MutableSet.copyOf((Set) x);
+ if (x instanceof Map) return (!alwaysCopyEvenIfMutable && x instanceof
MutableMap) ? x : MutableMap.copyOf((Map) x);
+ if (x instanceof Iterable) return (!alwaysCopyEvenIfMutable && x
instanceof MutableList) ? x : MutableList.copyOf((Iterable) x);
+ return ifNotIterableOrMap.apply(x);
+ }
+
+ /** returns pair containing outermost updated object and innermost updated
object.
+ * will be the same if there are no indices. */
+ public static Pair<Object,Object> setAtIndex(Pair<String,List<Object>>
nameAndIndices, boolean allowToCreateIntermediate, Function<Object,Object>
valueModifierOrSupplier, Function<String, Object> getter0, BiFunction<String,
Object, Object> setter0) {
+
+ String name = nameAndIndices.getLeft();
+ List<Object> indices = nameAndIndices.getRight();
+
+ Object key = name;
+ List<Object> oldValuesReplacedOutermostFirst = MutableList.of();
+
+ if (indices!=null && !indices.isEmpty()) {
+ if ("output".equals(name))
+ throw new IllegalArgumentException("It is not permitted to set
a subfield of the output"); // catch common error
+ }
+
+ String path = "";
+ Function<Object, Object> getter = (Function) getter0;
+ Function<Object,Consumer<Object>> setterCurried = k -> v -> {
+ oldValuesReplacedOutermostFirst.add(0, setter0.apply((String)k,
v));
+ };
+ Consumer<Object> setter = null;
+
+ Object last = null;
+ for (Object index : MutableList.<Object>of(name).appendAll(indices)) {
+ path += (path.length()>0 ? "/" : "") + index;
+ setter = setterCurried.apply(index);
+ final String pathF = path;
+ final Consumer<Object> prevSetter = setter;
+ final Object next = getter.apply(index);
+ last = next;
+ if (next == null) {
+ if (!allowToCreateIntermediate) {
+ throw new IllegalArgumentException("Cannot set index '" +
index + "' at '" + pathF + "' because that is undefined");
+ } else {
+ // create
+ getter = k -> null;
+ setterCurried = k -> v -> {
+ Object target = makeMutableOrUnchangedForIndex(null,
true, k).orThrow(
+ () -> new IllegalArgumentException("Cannot set
index '" + k + "' at '" + pathF + "' because that is undefined and key type
unknown"));
+ if (k instanceof Integer && (((Integer)k)<-1 ||
((Integer)k)>0)) {
+ throw new IllegalArgumentException("Cannot set
index '" + k + "' at '" + pathF + "' because that is undefined and key type out
of range");
+ }
+ Object oldV = target instanceof List ?
((List)target).add(v) : ((Map)target).put(k, v);
+ oldValuesReplacedOutermostFirst.add(0, oldV);
+ prevSetter.accept(target);
+ };
+ }
+ } else if (next instanceof Map) {
+ getter = ((Map)next)::get;
+ setterCurried = k -> v -> {
+ Map target = makeMutableCopy((Map)next).get();
+ Object oldV = target.put(k, v);
+ // we could be stricter and block this, in case they
thought it was a list?
+// if (oldV==null) {
+// if (!(k instanceof String))
+// throw new IllegalArgumentException("Cannot set
non-string index '" + k + "' in map at '" + pathF + "' unless it is replacing
(map insertion only supported for string keys)");
+// }
+ oldValuesReplacedOutermostFirst.add(0, oldV);
+ prevSetter.accept(target);
+ };
+ } else if (next instanceof List) {
+ getter = k -> {
+ k = asInteger(k).asType(Object.class).or(k);
+ if ("".equals(k)) return null; // empty string means to
append
+ if (!(k instanceof Integer))
+ throw new IllegalArgumentException("Cannot get index
'" + k + "' at '" + pathF + "' because that is a list");
+ Integer kn = (Integer) k;
+ if (kn==-1 || kn==((List)next).size()) return null; // -1
or N means to append
+ return ((List) next).get((Integer) k);
+ };
+ setterCurried = k -> v -> {
+ List target = makeMutableCopy((List)next).get();
+ Integer kn = asInteger(k).or(() -> {
+ if ("".equals(k)) return -1;
+ throw new IllegalArgumentException("Cannot set index
'" + k + "' at '" + pathF + "' because that is a list");
+ });
+ // -1 or size appends, or empty string
+
+ // (might be nice for negative numbers to reference from
the end also -
+ // but the freemarker getter doesn't support that, so it
would cause an odd asymmetry;
+ // however use of the empty string, or size, to add gives
easy ways to add that don't need this)
+ oldValuesReplacedOutermostFirst.add(0, kn==-1 ||
kn==target.size() ? target.add(v) : target.set(kn, v));
+ prevSetter.accept(target);
+ };
+ } else {
+ getter = k -> {
+ throw new IllegalArgumentException("Cannot set sub-index
at '" + k + "' at '" + pathF +"' because that is a " + next.getClass());
+ };
+ setterCurried = null;
+ }
+ }
+
+ setter.accept(valueModifierOrSupplier.apply(last));
+ return Pair.of(oldValuesReplacedOutermostFirst.get(0),
oldValuesReplacedOutermostFirst.get(oldValuesReplacedOutermostFirst.size()-1));
+ }
+
+ public static void
noteValueSetMetadata(WorkflowStepInstanceExecutionContext context, Object
newValue, Object oldValue) {
+ context.noteOtherMetadata(WorkflowSettingItemsUtils.VALUE_SET,
newValue);
+ if (oldValue!=null) {
+
context.noteOtherMetadata(WorkflowSettingItemsUtils.PREVIOUS_VALUE, oldValue);
+ }
+ }
+ public static void
noteValueSetNestedMetadata(WorkflowStepInstanceExecutionContext context,
Pair<String, List<Object>> nameAndIndices, Object newNestedValue, Pair<Object,
Object> oldOuterAndInnerValues) {
+ noteValueSetMetadata(context, newNestedValue,
oldOuterAndInnerValues.getRight());
+ if (!nameAndIndices.getRight().isEmpty()) {
+ Object oldValueOuter = oldOuterAndInnerValues.getLeft();
+ if (oldValueOuter != null) {
+
context.noteOtherMetadata(WorkflowSettingItemsUtils.PREVIOUS_VALUE_OUTER,
oldValueOuter);
+ }
+ }
+ }
+
+}
diff --git
a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowMapAndListTest.java
b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowMapAndListTest.java
index 0d9616c1d9..c42bd46223 100644
---
a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowMapAndListTest.java
+++
b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowMapAndListTest.java
@@ -18,20 +18,31 @@
*/
package org.apache.brooklyn.core.workflow;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
import com.google.common.collect.ImmutableMap;
import org.apache.brooklyn.api.entity.EntityLocal;
import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.core.config.ConfigKeys;
+import org.apache.brooklyn.core.entity.Dumper;
+import org.apache.brooklyn.core.entity.EntityAsserts;
+import org.apache.brooklyn.core.sensor.Sensors;
import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
+import org.apache.brooklyn.core.workflow.steps.appmodel.SetSensorWorkflowStep;
import org.apache.brooklyn.entity.stock.BasicApplication;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.core.config.ConfigBag;
-import org.testng.Assert;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import org.testng.annotations.Test;
-import java.util.List;
+public class WorkflowMapAndListTest extends BrooklynMgmtUnitTestSupport {
-public class WorkflowMapAndListTest extends BrooklynMgmtUnitTestSupport {
+ private static final Logger log =
LoggerFactory.getLogger(WorkflowMapAndListTest.class);
private BasicApplication app;
@@ -49,7 +60,7 @@ public class WorkflowMapAndListTest extends
BrooklynMgmtUnitTestSupport {
}
@Test
- public void testMapDirect() {
+ public void testSetWorkflowInMapWithDot() {
Object result = runSteps(MutableList.of(
"let map myMap = {}",
"let myMap.a = 1",
@@ -57,62 +68,224 @@ public class WorkflowMapAndListTest extends
BrooklynMgmtUnitTestSupport {
));
Asserts.assertEquals(result, "1");
}
+ @Test
+ public void testSetWorkflowInMapWithBrackets() {
+ Object result = runSteps(MutableList.of(
+ "let map myMap = {}",
+ "let myMap['a'] = 1",
+ "return ${myMap.a}"
+ ));
+ Asserts.assertEquals(result, "1");
+ }
+ @Test
+ public void testSetWorkflowInListWithType() {
+ Object result = runSteps(MutableList.of(
+ "let list myList = []",
+ "let int myList[0] = 1",
+ "return ${myList[0]}"
+ ));
+ Asserts.assertEquals(result, 1);
+ }
+
+ @Test
+ public void testSetWorkflowCreateMapAndList() {
+ Object result = runSteps(MutableList.of(
+ "let myMap['a'] = 1",
+ "return ${myMap.a}"
+ ));
+ Asserts.assertEquals(result, "1");
+
+ result = runSteps(MutableList.of(
+ "let int myList[0] = 1",
+ "return ${myList}"
+ ));
+ Asserts.assertEquals(result, Arrays.asList(1));
+ }
+
+ @Test
+ public void testBasicSensorAndConfig() {
+ runSteps(MutableList.of(
+ "set-config my_config['a'] = 1",
+ "set-sensor int my_sensor['a'] = 1"
+ ));
+ EntityAsserts.assertConfigEquals(app,
ConfigKeys.newConfigKey(Object.class, "my_config"), MutableMap.of("a", "1"));
+ EntityAsserts.assertAttributeEquals(app,
Sensors.newSensor(Object.class, "my_sensor"), MutableMap.of("a", 1));
+ }
+
+ @Test
+ public void testMapBracketsMore() {
+ Object result = runSteps(MutableList.of(
+ "set-sensor service.problems['latency'] = \"too slow\"",
+ "let x = ${entity.sensor['service.problems']}",
+ "return ${x}"
+ ));
+ Asserts.assertEquals(result, ImmutableMap.of("latency", "too slow"));
+
+ result = runSteps(MutableList.of(
+ "let map myMap = {}",
+ "let myMap['a'] = 1",
+ "return ${myMap.a}"
+ ));
+ Asserts.assertEquals(result, "1");
+ }
@Test
- public void testSetSensorMap() {
+ public void testMapBracketsUnquotedSyntaxLegacy() {
+ // this will warn but will work, for legacy compatibility reasons
+
Object result = runSteps(MutableList.of(
"set-sensor service.problems[latency] = \"too slow\"",
"let x = ${entity.sensor['service.problems']}",
"return ${x}"
));
Asserts.assertEquals(result, ImmutableMap.of("latency", "too slow"));
+
+ result = runSteps(MutableList.of(
+ "let map myMap = {}",
+ "let myMap[a] = 1",
+ "return ${myMap.a}"
+ ));
+ Asserts.assertEquals(result, "1");
}
@Test
- public void testListIndex() {
+ public void testMoreListByIndexInsertionCreationAndErrors() {
Object result = runSteps(MutableList.of(
"let list mylist = [1, 2, 3]",
"let mylist[1] = 4",
"return ${mylist[1]}"
));
Asserts.assertEquals(result, "4");
+
+ // 0 allowed to create (as does -1, further below)
+ result = runSteps(MutableList.of(
+ "let mylist[0] = 4",
+ "return ${mylist[0]}"
+ ));
+ Asserts.assertEquals(result, "4");
+
+ // other numbers not allowed to create
+ Asserts.assertFailsWith(() -> runSteps(MutableList.of(
+ "let mylist[123] = 4"
+ )), Asserts.expectedFailureContainsIgnoreCase("cannot set index",
"123", "mylist", "undefined"));
+
+ // -1 puts at end
+ result = runSteps(MutableList.of(
+ "let list mylist = [1, 2, 3]",
+ "let mylist[-1] = 4",
+ "return ${mylist[3]}"
+ ));
+ Asserts.assertEquals(result, "4");
+
+ // also works if we insert
+ result = runSteps(MutableList.of(
+ "let list mylist = [1, 2, 3]",
+ "let mylist[-1]['a'][-1] = 4",
+ "return ${mylist[3]['a'][0]}"
+ ));
+ Asserts.assertEquals(result, "4");
+ // as does empty string
+ result = runSteps(MutableList.of(
+ "let list mylist = [1, 2, 3]",
+ "let mylist[]['a'][] = 4",
+ "return ${mylist[3]['a'][0]}"
+ ));
+ Asserts.assertEquals(result, "4");
+
+ // note: getting -1 is not supported
+ Asserts.assertFailsWith(() -> runSteps(MutableList.of(
+ "let list mylist = [1,2,3]",
+ "return ${mylist[-1]}"
+ )), Asserts.expectedFailureContainsIgnoreCase("invalid", "reference",
"-1", "mylist"));
}
@Test
public void testUndefinedList() {
- Object result = null;
- try {
- result = runSteps(MutableList.of(
- "let list mylist = [1, 2, 3]",
- "let anotherList[1] = 4",
- "return ${anotherList[1]}"
- ));
- } catch (Exception e) {
- // We can't use expectedExceptionsMessageRegExp as the error
message is in the `Cause` exception
- if (e.getCause() == null ||
!e.getCause().getMessage().contains("Cannot set anotherList[1] because
anotherList is unset")) {
- Assert.fail("Expected cause exception to contain 'Cannot set
anotherList[1] because anotherList is unset'");
- }
- return;
- }
- Assert.fail("Expected IllegalArgumentException");
+ Asserts.assertFailsWith(() -> runSteps(MutableList.of(
+ "let anotherList[123] = 4"
+ )),
+ e -> Asserts.expectedFailureContainsIgnoreCase(e.getCause(),
+ "cannot", "index", "anotherList", "123", "undefined"));
}
@Test
public void testInvalidListSpecifier() {
- Object result = null;
- try {
- result = runSteps(MutableList.of(
- "let list mylist = [1, 2, 3]",
- "let mylist[1 = 4",
- "return ${mylist[1]}"
- ));
- } catch (Exception e) {
- // We can't use expectedExceptionsMessageRegExp as the error
message is in the `Cause` exception
- if (e.getCause() == null ||
!e.getCause().getMessage().contains("Invalid list index specifier mylist[1")) {
- Assert.fail("Expected cause exception to contain 'Invalid list
index specifier mylist[1'");
- }
- return;
+ Asserts.assertFailsWith(() -> runSteps(MutableList.of(
+ "let list mylizt = [1, 2, 3]",
+ "let mylizt['abc'] = 4",
+ "return ${mylizt[1]}"
+ )),
+ e -> Asserts.expectedFailureContainsIgnoreCase(e.getCause(),
"cannot", "mylizt", "abc", "list"));
+ }
+
+ @Test(groups = "Integration", invocationCount = 20)
+ public void
testBeefySensorRequireForAtomicityAndConfigCountsMapManyTimes() {
+ testBeefySensorRequireForAtomicityAndConfigCountsMap();
+ }
+
+ @Test
+ public void testBeefySensorRequireForAtomicityAndConfigCountsMap() {
+ runSteps(MutableList.of(
+ "set-sensor sum['total'] = 0", //needed for the 'last' check
+ "set-sensor map counts = {}", //needed for the 'last' check
+ MutableMap.of(
+ "step", "foreach n in 1..20",
+ "concurrency", 10,
+ "steps", MutableList.of(
+ "let last = ${entity.sensor['sum']['total']}",
+
+ // keep counts using sensors and config -
observe config might get mismatches but no errors,
+ // and sensors update perfectly atomically
+
+ "let ck = count_${n}",
+ "let last_count = ${entity.sensor.counts[ck]}
?? 0",
+ "let count = ${last_count} + 1",
+ // races in setting config known; we might
miss some here (see check)
+ "set-config counts['${ck}'] = ${count}",
+ // setting sensors however will be mutexed on
the sensor (assuming it exists or is defined)
+ "set-sensor counts['${ck}'] = ${count}",
+ // append to a list
+ "set-config ${ck}[] = ${count}",
+
+ "let next = ${last} + ${n}",
+
+ "log trying to set ${n} to ${next}, attempt
${count}",
+ // also check require with retries
+ MutableMap.of(
+ "step", "set-sensor sum['total']",
+ // reference so we can easily find
this test
+
SetSensorWorkflowStep.REQUIRE.getName(), "${last}",
+ "value", "${next}",
+ "on-error", "retry from start" // if
condition not met, will replay
+ ),
+ "log succeeded setting ${n} to ${next},
attempt ${count}"
+ )
+ )));
+
+ Dumper.dumpInfo(app);
+ EntityAsserts.assertAttributeEquals(app,
Sensors.newSensor(Object.class, "sum"), MutableMap.of("total", 210));
+
+ Map counts;
+ counts = app.sensors().get(Sensors.newSensor(Map.class, "counts"));
+ Asserts.assertEquals(counts.size(), 20);
+ Asserts.assertThat(counts.get("count_15"), x -> ((Integer) x) >= 1);
+
+ counts = app.config().get(ConfigKeys.newConfigKey(Map.class,
"counts"));
+ Asserts.assertThat(counts.size(), x -> x >= 15); // we don't
guarantee concurrency on config map writes, so might lose a few
+ if (counts.size()==20) {
+ log.warn("ODD!!! config for counts updated perfectly"); // doesn't
usually happen, unless slow machine
+ }
+ int nn = 0;
+ for (int n=1; n<=20; n++) {
+ List count_n =
app.config().get(ConfigKeys.newConfigKey(List.class, "count_"+n));
+ Asserts.assertThat(count_n.size(), x -> x >= 1);
+ for (int i = 0; i < count_n.size(); i++)
+ Asserts.assertEquals(count_n.get(i), i + 1);
+ nn += count_n.size();
+ }
+ if (nn<=20) {
+ log.warn("ODD!!! no retries"); // per odd comment above
}
- Assert.fail("Expected IllegalArgumentException");
}
+
}
diff --git
a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowNestedAndCustomExtensionTest.java
b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowNestedAndCustomExtensionTest.java
index 9df6e26cc7..36667e971d 100644
---
a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowNestedAndCustomExtensionTest.java
+++
b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowNestedAndCustomExtensionTest.java
@@ -37,6 +37,7 @@ import
org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
import org.apache.brooklyn.core.test.entity.TestApplication;
import org.apache.brooklyn.core.test.entity.TestEntity;
import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
+import org.apache.brooklyn.core.workflow.steps.appmodel.SetSensorWorkflowStep;
import org.apache.brooklyn.core.workflow.steps.flow.LogWorkflowStep;
import org.apache.brooklyn.core.workflow.store.WorkflowRetentionAndExpiration;
import
org.apache.brooklyn.core.workflow.store.WorkflowStatePersistenceViaSensors;
@@ -424,7 +425,7 @@ public class WorkflowNestedAndCustomExtensionTest extends
RebindTestFixture<Test
" - let count = ${entity.parent.sensor.count}",
" - let inc = ${count} + 1",
" - step: set-sensor count = ${inc}",
- " require: ${count}",
+ " "+ SetSensorWorkflowStep.REQUIRE.getName()+": ${count}",
" sensor:",
" entity: ${entity.parent}",
" on-error:",
