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/06/20 02:04:36 UTC
[brooklyn-server] branch master updated (1a13c62650 -> 7d5c247e82)
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 1a13c62650 Merge branch 'workflow-size-optimizations'
new 043cb18444 tidy up tests, add some notes on gaps
new a88fc13dc8 replace transform recognizes empty word at end now, quoted properly, and is non-greedy for glob all
new 4edec7a9b2 handle quotes in transform arguments
new eb8dea1528 add support for equals boolean check, and better errror/logging on ternary
new 46267ef0d3 fix recording of errors in step in subworkflow in some cases
new 15374e4f5b workflow tidies
new da5e8360e5 use weak map to access more tasks in memory
new 050f79317d let DslPredicate use wrapped values so we can inject supplied values into this
new e59f4f2cb1 add json shorthand deserializer
new 7d5c247e82 comment or fix places that need deeply
The 10 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:
.../brooklyn/CustomTypeConfigYamlRebindTest.java | 5 +-
.../brooklyn/spi/dsl/DslPredicateYamlTest.java | 4 +-
.../brooklyn/spi/dsl/DslSerializationTest.java | 83 ++++-
.../core/resolve/jackson/BeanWithTypeUtils.java | 26 +-
.../resolve/jackson/JsonShorthandDeserializer.java | 177 +++++++++++
.../jackson/ObjectReferencingSerialization.java | 7 +-
.../jackson/WrappedValuesSerialization.java | 8 +-
.../brooklyn/core/workflow/WorkflowEffector.java | 2 +-
.../core/workflow/WorkflowExecutionContext.java | 113 ++++---
.../workflow/WorkflowExpressionResolution.java | 152 ++++++---
.../brooklyn/core/workflow/WorkflowPolicy.java | 2 +-
.../core/workflow/WorkflowStepDefinition.java | 2 +-
.../WorkflowStepInstanceExecutionContext.java | 12 +-
.../core/workflow/steps/CustomWorkflowStep.java | 4 +-
.../steps/appmodel/SetConfigWorkflowStep.java | 2 +-
.../steps/appmodel/SetSensorWorkflowStep.java | 3 +-
.../workflow/steps/external/HttpWorkflowStep.java | 2 +-
.../workflow/steps/external/SshWorkflowStep.java | 2 +-
.../steps/variables/SetVariableWorkflowStep.java | 67 ++--
.../workflow/steps/variables/TransformReplace.java | 8 +-
.../variables/TransformVariableWorkflowStep.java | 13 +-
.../brooklyn/util/core/flags/TypeCoercions.java | 1 +
.../util/core/predicates/DslPredicates.java | 112 ++++---
.../util/core/task/BasicExecutionManager.java | 23 +-
...klynRegisteredTypeJacksonSerializationTest.java | 7 +-
.../JacksonJsonShorthandDeserializerTest.java | 189 +++++++++++
.../jackson/WrappedValuesSerializationTest.java | 11 +-
.../WorkflowNestedAndCustomExtensionTest.java | 16 +
.../core/workflow/WorkflowOperandsTest.java | 8 +
.../workflow/WorkflowPersistReplayErrorsTest.java | 76 ++++-
.../brooklyn/core/workflow/WorkflowRetryTest.java | 1 +
.../core/workflow/WorkflowTransformTest.java | 347 ++++++++-------------
.../util/core/predicates/DslPredicateTest.java | 5 +-
.../launcher/blueprints/SimpleBlueprintTest.java | 11 +
.../tasks/kubectl/ContainerWorkflowStep.java | 2 +-
.../brooklyn/location/winrm/WinrmWorkflowStep.java | 2 +-
.../brooklyn/util/javalang/coerce/TryCoercer.java | 3 +-
37 files changed, 1074 insertions(+), 434 deletions(-)
create mode 100644 core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonShorthandDeserializer.java
create mode 100644 core/src/test/java/org/apache/brooklyn/core/resolve/jackson/JacksonJsonShorthandDeserializerTest.java
[brooklyn-server] 07/10: use weak map to access more tasks in memory
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 da5e8360e58371334635dd9329793c135fea0392
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Sun Jun 18 01:45:07 2023 +0100
use weak map to access more tasks in memory
sometimes tasks are brooklyn GC'd, ie removed from the map,
but still referenced elsewhere, giving a lot more "task GC'd" messages than necessary.
by using weak refs, if there are still child references, a direct request will find the task
even if brooklyn has determined it can be GC'd
---
.../util/core/task/BasicExecutionManager.java | 23 +++++++++++-----------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java
index 005ba07336..e2cde9a050 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java
@@ -21,16 +21,9 @@ package org.apache.brooklyn.util.core.task;
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.collect.Iterables;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
+
+import java.lang.ref.WeakReference;
+import java.util.*;
import java.util.concurrent.Callable;
import java.util.concurrent.CancellationException;
import java.util.concurrent.ConcurrentHashMap;
@@ -50,6 +43,8 @@ import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import java.util.stream.Collectors;
+
+import com.google.common.collect.MapMaker;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.internal.BrooklynLoggingCategories;
import org.apache.brooklyn.api.mgmt.ExecutionContext;
@@ -270,7 +265,8 @@ public class BasicExecutionManager implements ExecutionManager {
//NB CopyOnWriteArraySet is a perf bottleneck, and the simple map makes it easier to remove when a tag is empty
private Map<Object, Set<Task<?>>> tasksByTag = new HashMap<Object, Set<Task<?>>>();
- private ConcurrentMap<String, Task<?>> tasksById = new ConcurrentHashMap<String, Task<?>>();
+ private Map<String, Task<?>> tasksById = new ConcurrentHashMap<String, Task<?>>();
+ private Map<String, Task<?>> tasksByIdWeak = Collections.synchronizedMap(new MapMaker().weakValues().makeMap());
private ConcurrentMap<Object, TaskScheduler> schedulerByTag = new ConcurrentHashMap<Object, TaskScheduler>();
@@ -593,7 +589,9 @@ public class BasicExecutionManager implements ExecutionManager {
@Override
public Task<?> getTask(String id) {
- return tasksById.get(id);
+ Task<?> result = tasksById.get(id);
+ if (result==null) result = tasksByIdWeak.get(id);
+ return result;
}
/**
@@ -1123,6 +1121,7 @@ public class BasicExecutionManager implements ExecutionManager {
}
tasksById.put(task.getId(), task);
+ tasksByIdWeak.put(task.getId(), task);
totalTaskCount.incrementAndGet();
}
[brooklyn-server] 06/10: workflow tidies
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 15374e4f5b551e0ca95f4f52e3d472a745480b6f
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Sun Jun 18 01:03:59 2023 +0100
workflow tidies
better names for tasks, better rest api error reporting, remove value set other metadata as now in workflowScratchUpdates
---
.../apache/brooklyn/core/workflow/WorkflowEffector.java | 2 +-
.../core/workflow/WorkflowExecutionContext.java | 17 ++++++++++++++---
.../apache/brooklyn/core/workflow/WorkflowPolicy.java | 2 +-
.../workflow/WorkflowStepInstanceExecutionContext.java | 5 +++++
.../workflow/steps/appmodel/SetConfigWorkflowStep.java | 2 +-
.../steps/variables/SetVariableWorkflowStep.java | 5 +++--
.../steps/variables/TransformVariableWorkflowStep.java | 5 +++--
.../core/workflow/WorkflowPersistReplayErrorsTest.java | 2 +-
.../launcher/blueprints/SimpleBlueprintTest.java | 11 +++++++++++
9 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowEffector.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowEffector.java
index 5844ac02e1..ca85db81e7 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowEffector.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowEffector.java
@@ -104,7 +104,7 @@ public class WorkflowEffector extends AddEffectorInitializerAbstract implements
WorkflowExecutionContext.WorkflowContextType.EFFECTOR, effector.getName() + " (workflow effector)", ConfigBag.newInstance(this.definitionParams),
effector.getParameters().stream().map(Effectors::asConfigKey).collect(Collectors.toSet()),
invocationParams,
- getFlagsForTaskInvocationAt(entity, effector, invocationParams));
+ getFlagsForTaskInvocationAt(entity, effector, invocationParams), effector.getName());
Task<Object> task = w.getTask(true).get();
if (parentInitializer!=null) {
// allow the parent to record the child workflow _before_ the child workflow gets persisted
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
index ec05fcb380..015d9e8e10 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
@@ -261,6 +261,12 @@ public class WorkflowExecutionContext {
public static WorkflowExecutionContext newInstanceUnpersistedWithParent(BrooklynObject entityOrAdjunctWhereRunning, WorkflowExecutionContext parent,
WorkflowContextType wcType, String name, ConfigBag paramsDefiningWorkflow,
Collection<ConfigKey<?>> extraConfigKeys, ConfigBag extraInputs, Map<String, Object> optionalTaskFlags) {
+ return newInstanceUnpersistedWithParent(entityOrAdjunctWhereRunning, parent, wcType, name, paramsDefiningWorkflow, extraConfigKeys, extraInputs, optionalTaskFlags, null);
+ }
+
+ public static WorkflowExecutionContext newInstanceUnpersistedWithParent(BrooklynObject entityOrAdjunctWhereRunning, WorkflowExecutionContext parent,
+ WorkflowContextType wcType, String name, ConfigBag paramsDefiningWorkflow,
+ Collection<ConfigKey<?>> extraConfigKeys, ConfigBag extraInputs, Map<String, Object> optionalTaskFlags, String optionalTaskName) {
// parameter defs
Map<String,ConfigKey<?>> parameters = MutableMap.of();
@@ -289,7 +295,7 @@ public class WorkflowExecutionContext {
input,
paramsDefiningWorkflow.get(WorkflowCommonConfig.OUTPUT),
WorkflowReplayUtils.updaterForReplayableAtWorkflow(paramsDefiningWorkflow, wcType == WorkflowContextType.NESTED_WORKFLOW),
- optionalTaskFlags);
+ optionalTaskFlags, optionalTaskName);
w.getStepsResolved(); // ensure steps resolve at this point; should be true even if condition doesn't apply (though input might not be valid without condition)
@@ -311,6 +317,11 @@ public class WorkflowExecutionContext {
protected WorkflowExecutionContext(BrooklynObject entityOrAdjunctWhereRunning, WorkflowExecutionContext parent, String name,
List<Object> stepsDefinition, Map<String,Object> input, Object output,
Consumer<WorkflowExecutionContext> replayableInitializer, Map<String, Object> optionalTaskFlags) {
+ this(entityOrAdjunctWhereRunning, parent, name, stepsDefinition, input, output, replayableInitializer, optionalTaskFlags, null);
+ }
+ protected WorkflowExecutionContext(BrooklynObject entityOrAdjunctWhereRunning, WorkflowExecutionContext parent, String name,
+ List<Object> stepsDefinition, Map<String,Object> input, Object output,
+ Consumer<WorkflowExecutionContext> replayableInitializer, Map<String, Object> optionalTaskFlags, String optionalTaskName) {
initParent(parent);
this.name = name;
this.adjunct = entityOrAdjunctWhereRunning instanceof Entity ? null : entityOrAdjunctWhereRunning;
@@ -321,7 +332,7 @@ public class WorkflowExecutionContext {
this.outputDefinition = output;
if (replayableInitializer!=null) replayableInitializer.accept(this);
- TaskBuilder<Object> tb = Tasks.builder().dynamic(true);
+ TaskBuilder<Object> tb = Tasks.builder().displayName(optionalTaskName).dynamic(true);
if (optionalTaskFlags!=null) tb.flags(optionalTaskFlags);
if (Strings.isBlank(tb.getDisplayName())) tb.displayName(name);
task = tb.body(new Body()).build();
@@ -449,7 +460,7 @@ public class WorkflowExecutionContext {
}
if (task==null) {
- if (taskId !=null) {
+ if (taskId!=null) {
task = (Task<Object>) getManagementContext().getExecutionManager().getTask(taskId);
}
if (task==null) {
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java
index 88324b3d9b..e54b4b0b89 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowPolicy.java
@@ -145,7 +145,7 @@ public class WorkflowPolicy<T> extends AbstractPolicy {
Set<PollConfig> pollConfigs = MutableSet.of(pc);
poller.schedulePoll(this, pollConfigs, new WorkflowSensor.WorkflowPollCallable(
- getDisplayName() + " (workflow)", config().getBag(), this), new PolicyNoOpPollHandler());
+ getDisplayName() + " (policy)", config().getBag(), this), new PolicyNoOpPollHandler());
if (!isSuspended()) resume();
}
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 94c2da19ee..2e421cddcf 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
@@ -18,8 +18,10 @@
*/
package org.apache.brooklyn.core.workflow;
+import com.fasterxml.jackson.annotation.JsonGetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonSetter;
import com.google.common.reflect.TypeToken;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.mgmt.ManagementContext;
@@ -28,6 +30,7 @@ import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal;
import org.apache.brooklyn.core.mgmt.BrooklynTaskTags;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.exceptions.Exceptions;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -71,6 +74,8 @@ public class WorkflowStepInstanceExecutionContext {
/** set if the step is in an error handler context, containing the error being handled,
* or if the step had an error that was not handled */
Throwable error;
+ @JsonGetter("error") String getErrorForJson() { return Exceptions.collapseText(error); }
+ @JsonSetter("error") void setErrorFromJaon(String error) { this.error = new RuntimeException(error); }
/** set if there was an error handled locally */
String errorHandlerTaskId;
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 b17757f41a..958f94adb4 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
@@ -45,13 +45,13 @@ public class SetConfigWorkflowStep extends WorkflowStepDefinition {
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");
+ // 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);
- // see note on type in SetSensorWorkflowStep
context.noteOtherMetadata("Value set", resolvedValue);
if (oldValue!=null) context.noteOtherMetadata("Previous value", oldValue);
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 63f2a893d0..a9b112a520 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
@@ -104,8 +104,9 @@ 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);
- context.noteOtherMetadata("Value set", resolvedValue);
- if (oldValue!=null) context.noteOtherMetadata("Previous value", oldValue);
+ // these are easily inferred from workflow vars
+// context.noteOtherMetadata("Value set", resolvedValue);
+// if (oldValue!=null) context.noteOtherMetadata("Previous value", oldValue);
return context.getPreviousStepOutput();
}
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 390dbcae96..e014bb661c 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
@@ -147,8 +147,9 @@ public class TransformVariableWorkflowStep extends WorkflowStepDefinition {
if (name!=null) {
Object oldValue = setWorkflowScratchVariableDotSeparated(context, name, v);
- context.noteOtherMetadata("Value set", v);
- if (oldValue != null) context.noteOtherMetadata("Previous value", oldValue);
+ // these are easily inferred from workflow vars
+// context.noteOtherMetadata("Value set", v);
+// if (oldValue != null) context.noteOtherMetadata("Previous value", oldValue);
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/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
index 8afc16f79d..7f3843b79c 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
@@ -634,7 +634,7 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl
m -> m.matches("Starting step .*-1 in task .*"),
m -> m.matches("Error in step '1 - invoke-effector does-not-exist'; rethrowing: No effector matching 'does-not-exist'"),
m -> m.matches("Error in workflow 'myWorkflow .workflow effector.' around step .*-1, running error handler"),
- m -> m.matches("Encountered error in workflow .*/.* 'myWorkflow .workflow effector.' .handler present.: No effector matching 'does-not-exist'"),
+ m -> m.matches("Encountered error in workflow .*/.* 'myWorkflow' .handler present.: No effector matching 'does-not-exist'"),
m -> m.matches("Starting .*-error-handler with 1 step in task .*"),
m -> m.matches("Starting .*-error-handler-1 in task .*"),
m -> m.matches("Completed handler .*-error-handler; no next step indicated so proceeding to default next step"),
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/blueprints/SimpleBlueprintTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/blueprints/SimpleBlueprintTest.java
index 7adcc91d5b..54ce84e739 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/blueprints/SimpleBlueprintTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/blueprints/SimpleBlueprintTest.java
@@ -19,13 +19,24 @@
package org.apache.brooklyn.launcher.blueprints;
import org.apache.brooklyn.api.entity.Application;
+import org.apache.brooklyn.api.mgmt.ManagementContext;
import org.apache.brooklyn.core.entity.Dumper;
+import org.apache.brooklyn.core.workflow.WorkflowBasicTest;
import org.apache.brooklyn.entity.stock.BasicEntity;
+import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
/** This does rebind. See SimpleBlueprintNonRebindTest for an example with rebind disabled. */
public class SimpleBlueprintTest extends AbstractBlueprintTest {
+ @Override
+ protected ManagementContext decorateManagementContext(ManagementContext mgmt) {
+ mgmt = super.decorateManagementContext(mgmt);
+ // make workflow step types available
+ WorkflowBasicTest.addWorkflowStepTypes(mgmt);
+ return mgmt;
+ }
+
@Override
protected boolean isViewerEnabled() {
return true;
[brooklyn-server] 02/10: replace transform recognizes empty word at end now, quoted properly, and is non-greedy for glob all
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 a88fc13dc868188091fb7094a06b282d8dfa056b
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Jun 15 11:19:19 2023 +0100
replace transform recognizes empty word at end now, quoted properly, and is non-greedy for glob all
---
.../core/workflow/steps/variables/TransformReplace.java | 8 ++++----
.../apache/brooklyn/core/workflow/WorkflowTransformTest.java | 11 +++++++----
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformReplace.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformReplace.java
index 330b51098e..16f8b491e3 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformReplace.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformReplace.java
@@ -39,7 +39,7 @@ public class TransformReplace extends WorkflowTransformDefault {
@Override
protected void initCheckingDefinition() {
Maybe<Map<String, Object>> maybeResult = new ShorthandProcessor(SHORTHAND)
- .withFinalMatchRaw(true)
+ .withFinalMatchRaw(false)
.withFailOnMismatch(true)
.process(transformDef);
@@ -80,7 +80,7 @@ public class TransformReplace extends WorkflowTransformDefault {
: input.replaceFirst(patternToMatch, replacement);
}
if (glob) {
- String globToRegex = convertGlobToRegex(patternToMatch);
+ String globToRegex = convertGlobToRegex(patternToMatch, !all);
return all ? input.replaceAll(globToRegex, replacement)
: input.replaceFirst(globToRegex, replacement);
@@ -103,7 +103,7 @@ public class TransformReplace extends WorkflowTransformDefault {
* https://jakarta.apache.org/oro/
*
*/
- private String convertGlobToRegex(String pattern) {
+ private String convertGlobToRegex(String pattern, boolean isGreedy) {
StringBuilder sb = new StringBuilder(pattern.length());
int inGroup = 0;
int inClass = 0;
@@ -133,7 +133,7 @@ public class TransformReplace extends WorkflowTransformDefault {
break;
case '*':
if (inClass == 0)
- sb.append(".*");
+ sb.append(".*"+(isGreedy ? "" : "?"));
else
sb.append('*');
break;
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
index 457030d84e..a07d3e2ce7 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
@@ -99,8 +99,11 @@ public class WorkflowTransformTest extends BrooklynMgmtUnitTestSupport {
// greedy
Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace regex 'c.*d' XXX"), "abXXX");
Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace all regex 'c.*d' XXX"), "abXXX");
- // TODO this fails
-// Asserts.assertEquals(transform("value 'abc def ghi' | replace regex 'c d' ''"), "abef ghi");
+ // non-greedy qualifier
+ Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace regex 'c.*?d' XXX"), "abXXXef ghi c2d");
+ Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace all regex 'c.*?d' XXX"), "abXXXef ghi XXX");
+
+ Asserts.assertEquals(transform("value 'abc def ghi' | replace regex 'c d' ''"), "abef ghi");
}
@Test
@@ -113,9 +116,9 @@ public class WorkflowTransformTest extends BrooklynMgmtUnitTestSupport {
@Test
public void testTransformGlob() throws Exception {
Asserts.assertEquals(transform("value 'abc def ghi' | replace glob c*e XXX"), "abXXXf ghi");
- // TODO glob is greedy, so all has no effect
+ // glob is greedy, unless all is specified where it is not
Asserts.assertEquals(transform("value 'abc def ghi c2e' | replace glob c*e XXX"), "abXXX");
- Asserts.assertEquals(transform("value 'abc def ghi c2e' | replace all glob c*e XXX"), "abXXX");
+ Asserts.assertEquals(transform("value 'abc def ghi c2e' | replace all glob c*e XXX"), "abXXXf ghi XXX");
}
@Test
[brooklyn-server] 09/10: add json shorthand deserializer
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 e59f4f2cb1f91b2cc78df4124814066aa8dc3ffc
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Tue Jun 20 01:56:45 2023 +0100
add json shorthand deserializer
and make sure it correctly instantiates things that are converted shallowly
---
.../brooklyn/spi/dsl/DslSerializationTest.java | 83 ++++++++-
.../resolve/jackson/JsonShorthandDeserializer.java | 177 +++++++++++++++++++
.../jackson/WrappedValuesSerialization.java | 8 +-
.../JacksonJsonShorthandDeserializerTest.java | 189 +++++++++++++++++++++
.../jackson/WrappedValuesSerializationTest.java | 11 +-
5 files changed, 452 insertions(+), 16 deletions(-)
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslSerializationTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslSerializationTest.java
index 3ee4bda07c..f331bc588e 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslSerializationTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslSerializationTest.java
@@ -18,11 +18,11 @@
*/
package org.apache.brooklyn.camp.brooklyn.spi.dsl;
+import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.google.common.base.Optional;
-import java.util.Map;
-import java.util.function.Supplier;
-
+import com.google.common.reflect.TypeToken;
import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.entity.EntitySpec;
import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
@@ -30,11 +30,10 @@ import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest;
import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.BrooklynDslCommon;
import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslComponent;
import org.apache.brooklyn.camp.brooklyn.spi.dsl.methods.DslComponent.Scope;
-import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils;
-import org.apache.brooklyn.core.resolve.jackson.MapperTestFixture;
-import org.apache.brooklyn.core.resolve.jackson.WrappedValue;
-import org.apache.brooklyn.core.resolve.jackson.WrappedValuesSerializationTest;
+import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
+import org.apache.brooklyn.core.resolve.jackson.*;
import org.apache.brooklyn.core.resolve.jackson.WrappedValuesSerializationTest.ObjectWithWrappedValueString;
+import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
import org.apache.brooklyn.core.workflow.WorkflowBasicTest;
import org.apache.brooklyn.entity.stock.BasicStartable;
import org.apache.brooklyn.test.Asserts;
@@ -45,6 +44,10 @@ import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
import org.testng.Assert;
import org.testng.annotations.Test;
+import java.util.Map;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
@Test
public class DslSerializationTest extends AbstractYamlTest implements MapperTestFixture {
@@ -229,4 +232,70 @@ public class DslSerializationTest extends AbstractYamlTest implements MapperTest
impl = deser(json("asu: { type: "+BasicStartable.class.getName()+" }"), ObjectWithWrappedValueSpec.class);
Asserts.assertNotNull(impl.asu);
}
+
+
+ static class ObjectWithTypedMaps {
+ Map<String,String> s;
+ Map<String,WrappedValue<String>> w;
+ Map<String,Object> o;
+ }
+
+ @Test
+ public void testDeserializeDslToMap() throws Exception {
+ String unwrappedDesiredValue = "foo";
+ String dslExpressionString = "$brooklyn:literal(" +
+ JavaStringEscapes.wrapJavaString(unwrappedDesiredValue)
+ + ")";
+ Object dslExpressionSupplier = DslUtils.resolveBrooklynDslValue(dslExpressionString, null, mgmt(), null).get();
+
+ ObjectWithTypedMaps impl;
+
+ MutableMap<String, MutableMap<String, Object>> dslMap = MutableMap.of("w", MutableMap.of("x", dslExpressionSupplier));
+
+ impl = BeanWithTypeUtils.convertShallow(mgmt(), dslMap, TypeToken.of(ObjectWithTypedMaps.class), true, null, true);
+ Asserts.assertInstanceOf(impl.w.get("x").getSupplier(), BrooklynDslDeferredSupplier.class);
+
+ impl = BeanWithTypeUtils.convertDeeply(mgmt(), dslMap, TypeToken.of(ObjectWithTypedMaps.class), true, null, true);
+ Asserts.assertInstanceOf(impl.w.get("x").getSupplier(), BrooklynDslDeferredSupplier.class);
+ }
+
+
+ // from JsonShorthandDeserializer
+ public static class MapT<T> {
+ @JsonDeserialize(contentUsing = JsonShorthandDeserializer.class)
+ Map<String,T> map;
+ void setMap(Map<String,T> map) { this.map = map; }
+ }
+
+ public static class MapX extends MapT<X> {}
+
+ public static class X {
+ WrappedValue<Object> w;
+
+ @JsonShorthandDeserializer.JsonShorthandInstantiator
+ public static X fromShorthand(WrappedValue<Object> o) {
+ X x = new X();
+ x.w = o;
+ return x;
+ }
+ }
+
+ @Test
+ public void convertShorthandShallowOrDeep() throws JsonProcessingException {
+ Consumer<MapX> check = r -> {
+ Asserts.assertInstanceOf(r.map.get("a").w.getSupplier(), BrooklynDslDeferredSupplier.class);
+ //Asserts.assertEquals(r.map.get("a").w.get(), "vv"); // not in an entity; above check is good enough, will fail if the map doesn't actually hold the wrapped value
+ };
+
+ String unwrappedDesiredValue = "foo";
+ String dslExpressionString = "$brooklyn:literal(" +
+ JavaStringEscapes.wrapJavaString(unwrappedDesiredValue)
+ + ")";
+ Object ws = DslUtils.resolveBrooklynDslValue(dslExpressionString, null, mgmt(), null).get();
+ MapX r;
+ r = BeanWithTypeUtils.convertDeeply(mgmt(), MutableMap.of("map", MutableMap.of("a", ws)), TypeToken.of(MapX.class), true, null, true);
+ check.accept(r);
+ r = BeanWithTypeUtils.convertShallow(mgmt(), MutableMap.of("map", MutableMap.of("a", ws)), TypeToken.of(MapX.class), true, null, true);
+ check.accept(r);
+ }
}
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonShorthandDeserializer.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonShorthandDeserializer.java
new file mode 100644
index 0000000000..6bd5d8e4a4
--- /dev/null
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonShorthandDeserializer.java
@@ -0,0 +1,177 @@
+/*
+ * 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.resolve.jackson;
+
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.BeanProperty;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JavaType;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import com.fasterxml.jackson.databind.deser.ContextualDeserializer;
+import com.fasterxml.jackson.databind.jsontype.TypeDeserializer;
+import com.fasterxml.jackson.databind.type.MapLikeType;
+import com.fasterxml.jackson.databind.util.TokenBuffer;
+import com.google.common.reflect.TypeToken;
+import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.core.flags.TypeCoercions;
+import org.apache.brooklyn.util.guava.Maybe;
+import org.apache.brooklyn.util.javalang.Reflections;
+import org.apache.brooklyn.util.javalang.coerce.TryCoercer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.*;
+
+/** Deserializer which can be set on a field in a bean to indicate that a special
+ * {@link JsonShorthandInstantiator}-annotated static 1-arg method should be used to instantiate the object
+ * from another type if primary instantiation does not succeed.
+ */
+public class JsonShorthandDeserializer extends JsonDeserializer<Object> implements ContextualDeserializer {
+
+ @Target({ElementType.METHOD})
+ @Retention(RetentionPolicy.RUNTIME)
+ public @interface JsonShorthandInstantiator {}
+
+ private static final Logger LOG = LoggerFactory.getLogger(JsonShorthandDeserializer.class);
+ private final JavaType type;
+
+ public JsonShorthandDeserializer() {
+ type = null;
+ }
+
+ public JsonShorthandDeserializer(JavaType type) {
+ this.type = type;
+ }
+
+ // inefficient, and unnecessary normally as the shorthand is only from json, and it is quite flexible
+// static {
+// TypeCoercions.registerAdapter("80-json-shorthand", new TryCoercer() {
+// @Override
+// public <T> Maybe<T> tryCoerce(Object input, TypeToken<T> type) {
+// Optional<Method> shorthand = Arrays.stream(type.getRawType().getMethods()).filter(m -> m.getAnnotation(JsonShorthandInstantiator.class) != null).findFirst();
+// if (!shorthand.isPresent()) return null;
+// try {
+// return Maybe.of((T) shorthand.get().invoke(null, input));
+// } catch (Exception e) {
+// return Maybe.absent("Unable to coerce "+input.getClass()+" to "+type+" using json shorthand");
+// }
+// }
+// });
+// }
+
+ @Override
+ public JsonDeserializer<?> createContextual(DeserializationContext ctxt, BeanProperty property) {
+ JavaType t;
+ if (property!=null) {
+ t = property.getType();
+ } else {
+ t = ctxt.getContextualType();
+ }
+ if (t==null) {
+ // shorthand not supported - could warn
+ return this;
+ }
+ if (t instanceof MapLikeType) {
+ return new JsonShorthandDeserializer(t.getContentType() );
+ }
+ if (Collection.class.isAssignableFrom(t.getRawClass())) {
+ return new JsonShorthandDeserializer(t.getContentType() );
+ }
+ return new JsonShorthandDeserializer(t);
+ }
+
+ // TODO do these methods need the BJSU routines?
+
+ @Override
+ public Object deserializeWithType(JsonParser p, DeserializationContext ctxt, TypeDeserializer typeDeserializer) throws IOException {
+ TokenBuffer b = new TokenBuffer(p, ctxt);
+ b.copyCurrentStructure(p);
+ Object r1 = null;
+ try {
+ r1 = super.deserializeWithType(b.asParserOnFirstToken(), ctxt, typeDeserializer);
+ } catch (Exception e) {
+ // ignore if "with type" deserialization didn't work here; it was probably incompatible for assigning _to_ X
+ // the type will be re-read and instantiated, and assigned to the _value_ of X
+ }
+ if (r1!=null && type.getRawClass().isInstance(r1)) return r1;
+ return deserialize(b.asParserOnFirstToken(), ctxt);
+ }
+
+ @Override
+ public Object deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
+ if (type==null) {
+ // shorthand only supported where specified on a property and type is determinable from context
+ return ctxt.findNonContextualValueDeserializer(ctxt.constructType(Object.class)).deserialize(p, ctxt);
+ }
+
+ List<Exception> exceptions = MutableList.of();
+ try {
+// TokenBuffer b = BrooklynJacksonSerializationUtils.createBufferForParserCurrentObject(p, ctxt);
+// // above might be better, then access with createParserFromTokenBufferAndParser
+ TokenBuffer b = new TokenBuffer(p, ctxt);
+ b.copyCurrentStructure(p);
+
+ try {
+ Object r1 = ctxt.findNonContextualValueDeserializer(type).deserialize(b.asParserOnFirstToken(), ctxt);
+ if (r1!=null && type.getRawClass().isInstance(r1)) return r1;
+ } catch (Exception e) {
+ exceptions.add(e);
+ }
+
+ try {
+ Method inst = Arrays.stream(type.getRawClass().getMethods())
+ .filter(m -> m.getAnnotation(JsonShorthandInstantiator.class) != null).findAny()
+// .orElseThrow(() -> new IllegalStateException("No public method annotated @JsonShorthandInstantiator"));
+ .orElseThrow(() -> {
+ return new IllegalStateException("No public method annotated @JsonShorthandInstantiator");
+ });
+ if ((inst.getModifiers() & Modifier.STATIC)==0) throw new IllegalStateException("@JsonShorthandInstantiator method must be static: "+inst);
+ if (inst.getParameterCount()!=1) throw new IllegalStateException("@JsonShorthandInstantiator method should take a single argument: "+inst);
+ Object v = ctxt.findRootValueDeserializer(ctxt.constructType(inst.getParameters()[0].getParameterizedType())).deserialize(b.asParserOnFirstToken(), ctxt);
+ if (v instanceof Map || (v instanceof WrappedValue && ((WrappedValue)v).getSupplier()==null && ((WrappedValue)v).get() instanceof Map)) {
+ // possibly we are unable to instantiate the type using the above; most of the time it is preferred, but in some cases we need the below,
+ // eg to handle DSL expressions in wrapped values (but many times the below fails). see JacksonJsonShorthandDeserializerTest failures when either is disallowed.
+ Object v2 = ctxt.findNonContextualValueDeserializer(ctxt.constructType(inst.getParameters()[0].getParameterizedType())).deserialize(b.asParserOnFirstToken(), ctxt);
+ if (!(v2 instanceof Map)) {
+ v = v2;
+ }
+ }
+ return inst.invoke(null, v);
+ } catch (Exception e) {
+ exceptions.add(e);
+ }
+
+ throw new IllegalStateException("Cannot instantiate as longhand or shorthand: " + exceptions, exceptions.stream().findFirst().orElse(null));
+ } finally {
+ if (!exceptions.isEmpty() && LOG.isTraceEnabled()) {
+ LOG.trace("Exceptions encountered while deserializing: " + exceptions);
+ exceptions.forEach(e -> LOG.trace("- ", e));
+ }
+ }
+
+ }
+
+}
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/WrappedValuesSerialization.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/WrappedValuesSerialization.java
index 8a05b79a64..4fe02fd157 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/WrappedValuesSerialization.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/WrappedValuesSerialization.java
@@ -93,7 +93,10 @@ public class WrappedValuesSerialization {
Object deserializeWithTypeUnwrapped(JsonParser p, DeserializationContext ctxt, TypeDeserializer typeDeserializer) throws IOException {
List<Exception> exceptions = MutableList.of();
+
try {
+// TokenBuffer b = new TokenBuffer(p, ctxt);
+// b.copyCurrentStructure(p);
TokenBuffer b = BrooklynJacksonSerializationUtils.createBufferForParserCurrentObject(p, ctxt);
JavaType genericType = null;
try {
@@ -108,6 +111,8 @@ public class WrappedValuesSerialization {
try {
// this uses our type deserializer, will try type instantiation from a declared type and/or expected type of the generics
return ctxt.findRootValueDeserializer(genericType).deserialize(
+ // createParserFromTokenBufferAndParser(b, p)
+ // should we use line above instead of line below, which we use several lines further below?
b.asParserOnFirstToken(), ctxt);
} catch (Exception e) {
exceptions.add(e);
@@ -118,7 +123,8 @@ public class WrappedValuesSerialization {
try {
// this does _not_ use our type deserializer; will try type instantiation from the expected type of the generics however
return ctxt.findNonContextualValueDeserializer(genericType).deserialize(
- createParserFromTokenBufferAndParser(b, p), ctxt);
+ createParserFromTokenBufferAndParser(b, p),
+ ctxt);
} catch (Exception e) {
exceptions.add(e);
}
diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/JacksonJsonShorthandDeserializerTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/JacksonJsonShorthandDeserializerTest.java
new file mode 100644
index 0000000000..a74f9c3133
--- /dev/null
+++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/JacksonJsonShorthandDeserializerTest.java
@@ -0,0 +1,189 @@
+/*
+ * 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.resolve.jackson;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.google.common.reflect.TypeToken;
+import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
+import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
+import org.apache.brooklyn.test.Asserts;
+import org.apache.brooklyn.util.collections.Jsonya;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.core.task.DeferredSupplier;
+import org.apache.brooklyn.util.text.Strings;
+import org.apache.brooklyn.util.yaml.Yamls;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+import java.util.Map;
+import java.util.Objects;
+import java.util.function.Consumer;
+
+public class JacksonJsonShorthandDeserializerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(JacksonJsonShorthandDeserializerTest.class);
+
+ static class X0 {
+ @JsonDeserialize(using = JsonShorthandDeserializer.class)
+ X x;
+ }
+
+ static class MapT<T> {
+ @JsonDeserialize(contentUsing = JsonShorthandDeserializer.class)
+ Map<String,T> map;
+ void setMap(Map<String,T> map) { this.map = map; }
+ }
+
+ static class MapX extends MapT<X> {}
+
+ static class X {
+
+// @JsonCreator
+ X() { o = "noa"; }
+// @JsonCreator
+// X(String x) { v = x; o = "str"; }
+// @JsonCreator
+// X(Integer x) { v = x; o = "int"; }
+
+ Object v;
+ String o = "orig";
+
+ @JsonShorthandDeserializer.JsonShorthandInstantiator
+ public static X fromShorthand(Object o) {
+ X x = new X();
+ x.v = o;
+ x.o = "sho";
+ return x;
+ }
+ }
+
+ static class V {
+ static V of(int v) {
+ V result = new V();
+ result.v = v;
+ return result;
+ }
+ Object v;
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ V v1 = (V) o;
+ return Objects.equals(v, v1.v);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(v);
+ }
+ }
+
+ @Test
+ public void testX() throws JsonProcessingException {
+ check("foo", "foo", "sho");
+ check(""+1, 1, "sho");
+ check("{ type: "+ V.class.getName()+", v: 2 }", V.of(2), "sho"); // or obj?
+ check("{ v: 3 }", 3, "noa");
+ check("{ u: 4 }", MutableMap.of("u", 4), "sho");
+ check("{ type: "+X.class.getName()+", v: 5 }", 5, "noa"); // or obj?
+ }
+
+ private static void check(String xJson, Object v, String o) throws JsonProcessingException {
+ X0 x0 = BeanWithTypeUtils.newMapper(null, false, null, true).readerFor(Object.class).readValue(
+ Jsonya.newInstance().add(Yamls.parseAll(Strings.lines(
+ "type: "+X0.class.getName(),
+ "x: "+xJson
+ )).iterator().next()).toString()
+ );
+
+ Assert.assertEquals(x0.x.v, v);
+ Assert.assertEquals(x0.x.o, o);
+
+
+ MapX xm = BeanWithTypeUtils.newMapper(null, false, null, true).readerFor(MapX.class).readValue(
+ Jsonya.newInstance().add(Yamls.parseAll(Strings.lines(
+ "map:",
+ " x: "+xJson
+ )).iterator().next()).toString()
+ );
+
+ Assert.assertEquals(xm.map.get("x").v, v);
+ Assert.assertEquals(xm.map.get("x").o, o);
+ }
+
+ public static class DS implements DeferredSupplier<String> {
+ String s;
+ public String get() { return s; }
+ }
+
+ public static class W {
+ WrappedValue<String> w;
+
+ @JsonShorthandDeserializer.JsonShorthandInstantiator
+ public static W fromShorthand(WrappedValue<String> o) {
+ W w = new W();
+ w.w = o;
+ return w;
+ }
+ }
+
+ public static class MWH {
+ MapT<W> holder;
+ }
+
+ @Test
+ public void convertShallowOrDeep() throws JsonProcessingException {
+ Consumer<MapX> check = r -> {
+ Asserts.assertEquals(r.map.get("a").v, "vv");
+ Asserts.assertEquals(r.map.get("a").o, "sho");
+ };
+
+ MapX r;
+
+ r = BeanWithTypeUtils.convertDeeply(null, MutableMap.of("map", MutableMap.of("a", "vv")), TypeToken.of(MapX.class), false, null, false);
+ check.accept(r);
+ r = BeanWithTypeUtils.convertShallow(null, MutableMap.of("map", MutableMap.of("a", "vv")), TypeToken.of(MapX.class), false, null, false);
+ check.accept(r);
+
+ DS ds = new DS(); ds.s = "vv";
+ ObjectMapper mapper = BeanWithTypeUtils.newMapper(null, false, null, true);
+ MapT<W> mw = new MapT<>();
+ W w = W.fromShorthand(WrappedValue.of(ds));
+ mw.setMap(MutableMap.of("a", w));
+ MWH h = new MWH();
+ h.holder = mw;
+ String hs = mapper.writeValueAsString(h);
+ Asserts.assertEquals(hs, "{\"holder\":{\"map\":{\"a\":{\"w\":{\"type\":\"org.apache.brooklyn.core.resolve.jackson.JacksonJsonShorthandDeserializerTest$DS\",\"s\":\"vv\"}}}}}");
+ MWH h2 = mapper.readValue(hs, MWH.class);
+ Asserts.assertEquals(h2.holder.map.get("a").w.get(), "vv");
+
+ LocalManagementContext mgmt = LocalManagementContextForTests.newInstance();
+ MapT<W> r2;
+ r2 = BeanWithTypeUtils.convertShallow(mgmt, MutableMap.of("map", MutableMap.of("a", ds)), new TypeToken<MapT<W>>() {}, true, null, true);
+ Asserts.assertEquals(r2.map.get("a").w.get(), "vv");
+
+ r2 = BeanWithTypeUtils.convertDeeply(mgmt, MutableMap.of("map", MutableMap.of("a", ds)), new TypeToken<MapT<W>>() {}, true, null, true);
+ Asserts.assertEquals(r2.map.get("a").w.get(), "vv");
+ }
+}
diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/WrappedValuesSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/WrappedValuesSerializationTest.java
index db6f362ef1..b28fbb2754 100644
--- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/WrappedValuesSerializationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/WrappedValuesSerializationTest.java
@@ -19,16 +19,11 @@
package org.apache.brooklyn.core.resolve.jackson;
import com.fasterxml.jackson.databind.ObjectMapper;
+
import java.util.function.Supplier;
import org.apache.brooklyn.core.resolve.jackson.WrappedValue.WrappedValuesInitialized;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.util.core.flags.TypeCoercions;
-import org.apache.brooklyn.util.core.flags.TypeCoercions.BrooklynCommonAdaptorTypeCoercions;
-import org.apache.brooklyn.util.core.task.BasicExecutionContext;
-import org.apache.brooklyn.util.core.task.BasicExecutionManager;
-import org.apache.brooklyn.util.core.task.Tasks;
-import org.apache.brooklyn.util.guava.Maybe;
-import org.apache.brooklyn.util.javalang.JavaClassNames;
import org.testng.Assert;
import org.testng.annotations.Test;
@@ -152,8 +147,8 @@ public class WrappedValuesSerializationTest implements MapperTestFixture {
}
@Test
- public void testDeserializeDsl() throws Exception {
- // test in CAMP where DSL is registered
+ public void testDeserializeUnrecognizedDsl() throws Exception {
+ // tests in CAMP DslDeserializationTest for processing the DSL
String dslLiteralFoo = "$brooklyn:literal(\"foo\")";
ObjectWithWrappedValueString impl = deser(json("x: " + dslLiteralFoo), ObjectWithWrappedValueString.class);
Asserts.assertNotNull(impl.x);
[brooklyn-server] 08/10: let DslPredicate use wrapped values so we can inject supplied values into this
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 050f79317d3b2b243ffe54c5adf9e4878f435455
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Mon Jun 19 17:48:05 2023 +0100
let DslPredicate use wrapped values so we can inject supplied values into this
workflow uses this so that conditions can be parsed once and processed subsequently.
if too messy we might scratch that, always do late resolution.
primarily an issue for foreach and custom where the condition needs to be evaluated
after scratch variables are inserted. (that could be handled another way which might be simpler.)
but the abilities introduced here, for conditions to be more dynamic are useful.
however they currently store the context which is not good.
---
.../brooklyn/CustomTypeConfigYamlRebindTest.java | 5 +-
.../brooklyn/spi/dsl/DslPredicateYamlTest.java | 4 +-
.../core/resolve/jackson/BeanWithTypeUtils.java | 24 +++-
.../jackson/ObjectReferencingSerialization.java | 7 +-
.../core/workflow/WorkflowExecutionContext.java | 58 ++++++----
.../workflow/WorkflowExpressionResolution.java | 125 ++++++++++++++++-----
.../core/workflow/WorkflowStepDefinition.java | 2 +-
.../WorkflowStepInstanceExecutionContext.java | 4 +-
.../core/workflow/steps/CustomWorkflowStep.java | 4 +-
.../steps/appmodel/SetSensorWorkflowStep.java | 3 +-
.../workflow/steps/external/HttpWorkflowStep.java | 2 +-
.../workflow/steps/external/SshWorkflowStep.java | 2 +-
.../steps/variables/SetVariableWorkflowStep.java | 2 +-
.../util/core/predicates/DslPredicates.java | 112 +++++++++++-------
.../WorkflowNestedAndCustomExtensionTest.java | 16 +++
.../util/core/predicates/DslPredicateTest.java | 5 +-
.../tasks/kubectl/ContainerWorkflowStep.java | 2 +-
.../brooklyn/location/winrm/WinrmWorkflowStep.java | 2 +-
.../brooklyn/util/javalang/coerce/TryCoercer.java | 3 +-
19 files changed, 270 insertions(+), 112 deletions(-)
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlRebindTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlRebindTest.java
index ea464d641e..433d80abc5 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlRebindTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/CustomTypeConfigYamlRebindTest.java
@@ -142,7 +142,10 @@ public class CustomTypeConfigYamlRebindTest extends AbstractYamlRebindTest {
() ->
child.config().set((ConfigKey) EntityWithCustomTypeConfig.CUSTOM_TYPE_KEY.subKey("i2"),
MutableMap.of("x", DslUtils.parseBrooklynDsl(mgmt(), "$brooklyn:attributeWhenReady(\"set-later\")"))),
- e -> Asserts.expectedFailureContains(e, "Cannot deserialize value", "String", "from Object"));
+ e -> Asserts.expectedFailureContains(e,
+ //"Cannot deserialize value", "String", "from Object" <- deeply returns this
+ "Cannot coerce or set", "x=$brooklyn:attributeWhenReady(\"set-later\")", "customTypeKey.i2"
+ ));
// but is allowed at map level (is additive)
child.config().set((ConfigKey) EntityWithCustomTypeConfig.CUSTOM_TYPE_KEY,
diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java
index 0059b22005..f23709d80c 100644
--- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java
+++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/DslPredicateYamlTest.java
@@ -93,12 +93,12 @@ public class DslPredicateYamlTest extends AbstractYamlTest {
// this is simpler and more efficient, although it might be surprising
app.config().set(ConfigKeys.newStringConfigKey("expected"), "y");
Asserts.assertFalse( predicate.apply(app) );
- Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)predicate).equals, "x" );
+ Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)predicate).equals.get(), "x" );
// per above, if we re-retrieve the predicate it should work fine
predicate = app.config().get(TestEntity.CONF_PREDICATE);
Asserts.assertTrue( predicate.apply(app) );
- Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)predicate).equals, "y" );
+ Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)predicate).equals.get(), "y" );
}
static class PredicateAndSpec {
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
index ff5902eee1..a920995cac 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
@@ -169,11 +169,28 @@ public class BeanWithTypeUtils {
try {
stack.push(mapOrListToSerializeThenDeserialize);
- return convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes);
+ // prefer this because (a) it's cheaper, and (b) it supports deferred values more nicely;
+ // ObjectReferencingSerialization.deserializeWrapper will do type coercion so very few things if any should need deep coercion now
+ T result = convertShallow(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes);
+
+// T result2 = null;
+// try {
+// result2 = convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes);
+// } catch (Exception e2) {
+// Exceptions.propagateIfFatal(e2);
+// // otherwise ignore
+// }
+// if (!Objects.equals(result, result2)) {
+// // legacy preferred convert deeply; in a few places this mattered.
+// // need to investigate when/why
+// return result2;
+// }
+
+ return result;
} catch (Exception e) {
try {
- return convertShallow(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes);
+ return convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes);
} catch (Exception e2) {
throw Exceptions.propagate(Arrays.asList(e, e2));
}
@@ -186,7 +203,8 @@ public class BeanWithTypeUtils {
@Beta
public static <T> T convertShallow(ManagementContext mgmt, Object mapOrListToSerializeThenDeserialize, TypeToken<T> type, boolean allowRegisteredTypes, BrooklynClassLoadingContext loader, boolean allowJavaTypes) throws JsonProcessingException {
- // try with complex types are saved as objects rather than serialized, but won't work if special deserialization is wanted to apply to a map inside a complex type
+ // try with complex types are saved as objects rather than serialized; might not work if special deserialization is wanted to apply to a map inside a complex type,
+ // but type coercions might mean that it does actually work but doing a nested convert shallow
ObjectMapper mapper = YAMLMapper.builder().build();
mapper = BeanWithTypeUtils.applyCommonMapperConfig(mapper, mgmt, allowRegisteredTypes, loader, allowJavaTypes);
mapper = new ObjectReferencingSerialization().useAndApplytoMapper(mapper);
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/ObjectReferencingSerialization.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/ObjectReferencingSerialization.java
index e703f3a868..9d3af1c773 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/ObjectReferencingSerialization.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/ObjectReferencingSerialization.java
@@ -178,7 +178,12 @@ public class ObjectReferencingSerialization {
// and if the expected type is overly strict, return the original object.
// if the token buffer is used too much we might have lost the alias reference and still end up with a string,
// but so long as this deserializer is preferred which it normally is, losing the alias reference is okay.
- return ((Maybe)TypeCoercions.tryCoerce(result, TypeToken.of(expected))).or(result);
+ Maybe resultCoerced = ((Maybe) TypeCoercions.tryCoerce(result, TypeToken.of(expected)));
+ if (resultCoerced.isAbsent()) {
+ // not uncommon when we are trying to deserialize in a few different ways, or if we are using a string deserializer because the json input is a string
+// if (LOG.isDebugEnabled()) LOG.debug("Reference to "+result+" when deserialization could not be coerced to expected type "+expected+"; proceeding but might cause errors");
+ }
+ return resultCoerced.or(result);
} else {
LOG.debug("Odd - what looks like a reference was received but not found: "+v);
}
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
index 015d9e8e10..33c7722a50 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
@@ -18,7 +18,10 @@
*/
package org.apache.brooklyn.core.workflow;
-import com.fasterxml.jackson.annotation.*;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonSetter;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.type.TypeFactory;
@@ -137,7 +140,7 @@ public class WorkflowExecutionContext {
@JsonDeserialize(contentUsing = JsonPassThroughDeserializer.class)
List<Object> stepsDefinition;
- DslPredicates.DslPredicate condition;
+ Object condition;
@JsonInclude(JsonInclude.Include.NON_EMPTY)
Map<String,Object> input = MutableMap.of();
@@ -307,7 +310,7 @@ public class WorkflowExecutionContext {
WorkflowStepResolution.resolveSubSteps(w.getManagementContext(), "error handling", WorkflowErrorHandling.wrappedInListIfNecessaryOrNullIfEmpty(w.onError));
// some fields need to be resolved at setting time, in the context of the workflow
- w.setCondition(w.resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage.WORKFLOW_STARTING_POST_INPUT, paramsDefiningWorkflow.getStringKey(WorkflowCommonConfig.CONDITION.getName()), WorkflowCommonConfig.CONDITION.getTypeToken()));
+ w.setCondition(paramsDefiningWorkflow.getStringKey(WorkflowCommonConfig.CONDITION.getName()));
// finished -- checkpoint noting this has been created but not yet started
w.updateStatus(WorkflowStatus.STAGED);
@@ -392,7 +395,7 @@ public class WorkflowExecutionContext {
return stepsWithExplicitId;
}
- public void setCondition(DslPredicates.DslPredicate condition) {
+ public void setCondition(Object condition) {
this.condition = condition;
}
@@ -442,23 +445,29 @@ public class WorkflowExecutionContext {
return retryRecords;
}
- @JsonIgnore
- public Object getConditionTarget() {
- if (getWorkflowScratchVariables()!=null) {
- Object v = getWorkflowScratchVariables().get("target");
- // should we also set the entity? otherwise it will take from the task. but that should only apply
- // in a task where the context entity is set, so for now rely on that.
- if (v!=null) return v;
+ public Maybe<Task<Object>> getTask(boolean checkCondition) {
+ if (checkCondition) return getTaskCheckingConditionWithTarget(getEntityOrAdjunctWhereRunning());
+ else return getTaskSkippingCondition();
+ }
+ DslPredicates.DslPredicate resolveCondition(Object condition) {
+ if (condition==null) return null;
+ // condition is resolved wrapped for two reasons:
+ // - target cannot be a fully resolved string unless it is something like 'children', and that constant should be
+ // different to a var ${x} (even if x evaluates to children)
+ // - some tests allow things to throw errors and check for error, so e.g. an expression that doesn't resolve isn't necessarily a problem
+ return resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, condition, TypeToken.of(DslPredicates.DslPredicate.class),
+ WorkflowExpressionResolution.WrappingMode.WRAPPED_RESULT_DEFER_THROWING_ERROR_BUT_NO_RETRY);
+ }
+ public Maybe<Task<Object>> getTaskCheckingConditionWithTarget(Object conditionTarget) {
+ DslPredicates.DslPredicate conditionResolved = resolveCondition(condition);
+ if (conditionResolved != null) {
+ if (!conditionResolved.apply(conditionTarget))
+ return Maybe.absent(new IllegalStateException("This workflow cannot be run at present: condition not satisfied"));
}
- return getEntityOrAdjunctWhereRunning();
+ return getTaskSkippingCondition();
}
-
@JsonIgnore
- public Maybe<Task<Object>> getTask(boolean checkCondition) {
- if (checkCondition && condition!=null) {
- if (!condition.apply(getConditionTarget())) return Maybe.absent(new IllegalStateException("This workflow cannot be run at present: condition not satisfied"));
- }
-
+ public Maybe<Task<Object>> getTaskSkippingCondition() {
if (task==null) {
if (taskId!=null) {
task = (Task<Object>) getManagementContext().getExecutionManager().getTask(taskId);
@@ -672,22 +681,23 @@ public class WorkflowExecutionContext {
/** resolution of ${interpolation} and $brooklyn:dsl and deferred suppliers, followed by type coercion.
* if the type is a string, null is not permitted, otherwise it is. */
public <T> T resolve(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) {
- return new WorkflowExpressionResolution(this, stage, false, false).resolveWithTemplates(expression, type);
+ return new WorkflowExpressionResolution(this, stage, false, WorkflowExpressionResolution.WrappingMode.NONE).resolveWithTemplates(expression, type);
}
public <T> T resolveCoercingOnly(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) {
- return new WorkflowExpressionResolution(this, stage, false, false).resolveCoercingOnly(expression, type);
+ return new WorkflowExpressionResolution(this, stage, false, WorkflowExpressionResolution.WrappingMode.NONE).resolveCoercingOnly(expression, type);
}
- /** as {@link #resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, TypeToken)}, but returning DSL/supplier for values (so the indication of their dynamic nature is preserved, even if the value returned by it is resolved;
+ /** as {@link #resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, TypeToken)},
+ * but returning DSL/supplier for values (so the indication of their dynamic nature is preserved, even if the value returned by it is resolved;
* this is needed e.g. for conditions which treat dynamic expressions differently to explicit values) */
- public <T> T resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) {
- return new WorkflowExpressionResolution(this, stage, false, true).resolveWithTemplates(expression, type);
+ public <T> T resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type, WorkflowExpressionResolution.WrappingMode wrappingMode) {
+ return new WorkflowExpressionResolution(this, stage, false, wrappingMode).resolveWithTemplates(expression, type);
}
/** as {@link #resolve(WorkflowExpressionResolution.WorkflowExpressionStage, Object, TypeToken)}, but waiting on any expressions which aren't ready */
public <T> T resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) {
- return new WorkflowExpressionResolution(this, stage, true, false).resolveWithTemplates(expression, type);
+ return new WorkflowExpressionResolution(this, stage, true, WorkflowExpressionResolution.WrappingMode.NONE).resolveWithTemplates(expression, type);
}
/** resolution of ${interpolation} and $brooklyn:dsl and deferred suppliers, followed by type coercion */
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 b3e6c0589f..5c207c8b6c 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
@@ -18,6 +18,7 @@
*/
package org.apache.brooklyn.core.workflow;
+import com.google.common.annotations.Beta;
import com.google.common.reflect.TypeToken;
import freemarker.template.TemplateHashModel;
import freemarker.template.TemplateModel;
@@ -70,18 +71,55 @@ public class WorkflowExpressionResolution {
private static final Logger log = LoggerFactory.getLogger(WorkflowExpressionResolution.class);
private final WorkflowExecutionContext context;
private final boolean allowWaiting;
- private final boolean useWrappedValue;
private final WorkflowExpressionStage stage;
private final TemplateProcessor.InterpolationErrorMode errorMode;
+ private final WrappingMode wrappingMode;
+
+ public static class WrappingMode {
+ public final boolean wrapResolvedStrings;
+ public final boolean deferThrowingError;
+ public final boolean deferAndRetryErroneousExpressions;
+ public final boolean deferBrooklynDsl;
+ public final boolean deferInterpolation;
+
+ protected WrappingMode(boolean wrapResolvedStrings, boolean deferThrowingError, boolean deferAndRetryErroneousExpressions, boolean deferBrooklynDsl, boolean deferInterpolation) {
+ this.wrapResolvedStrings = wrapResolvedStrings;
+ this.deferThrowingError = deferThrowingError;
+ this.deferAndRetryErroneousExpressions = deferAndRetryErroneousExpressions;
+ this.deferBrooklynDsl = deferBrooklynDsl;
+ this.deferInterpolation = deferInterpolation;
+ }
+
+ /** do not re-evaluate anything, but if there is an error don't throw it until accessed; useful for conditions that should be evaluated immediately */
+ public final static WrappingMode WRAPPED_RESULT_DEFER_THROWING_ERROR_BUT_NO_RETRY = new WrappingMode(true, true, false, false, false);
+
+ /** no wrapping; everything evaluated immediately, errors thrown immediately */
+ public final static WrappingMode NONE = new WrappingMode(false, false, false, false, false);
+
+ /** this was the old default when wrapping was requested, but was an odd one - wraps error throwing and DSL resolution but not interpolation */
+ @Deprecated @Beta // might re-introduce but for now needs to cache workflow context so discouraged
+ final static WrappingMode OLD_DEFAULT_DEFER_THROWING_ERROR_AND_DSL = new WrappingMode(true, true, false, true, false);
+ /** allow subsequent re-evaluation for things that are not recognized, but evaluate everything else now; cf InterpolationErrorMode.IGNORE */
+ @Deprecated @Beta // might re-introduce but for now needs to cache workflow context so discouraged
+ public final static WrappingMode DEFER_RETRY_ON_ERROR_ONLY = new WrappingMode(false, false, true, false, false);
+ /** defer the evaluation of all vars (but evaluate now so if string is static it can be returned as a static) */
+ @Deprecated @Beta // might re-introduce but for now needs to cache workflow context so discouraged
+ public final static WrappingMode ALL_NON_STATIC = new WrappingMode(true /* no effect here */, true /* no effect here */, true, true, true);
+
+ public WrappingMode wrappingModeWhenResolving() {
+ // this works for our current use cases, which is conditions; other uses might want it not to throw something deferred however
+ return WRAPPED_RESULT_DEFER_THROWING_ERROR_BUT_NO_RETRY;
+ }
+ }
- public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, boolean wrapExpressionValues) {
+ public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, WrappingMode wrapExpressionValues) {
this(context, stage, allowWaiting, wrapExpressionValues, TemplateProcessor.InterpolationErrorMode.FAIL);
}
- public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, boolean wrapExpressionValues, TemplateProcessor.InterpolationErrorMode errorMode) {
+ public WorkflowExpressionResolution(WorkflowExecutionContext context, WorkflowExpressionStage stage, boolean allowWaiting, WrappingMode wrapExpressionValues, TemplateProcessor.InterpolationErrorMode errorMode) {
this.context = context;
this.stage = stage;
this.allowWaiting = allowWaiting;
- this.useWrappedValue = wrapExpressionValues;
+ this.wrappingMode = wrapExpressionValues == null ? WrappingMode.NONE : wrapExpressionValues;
this.errorMode = errorMode;
}
@@ -120,7 +158,7 @@ public class WorkflowExpressionResolution {
return ifNoMatches();
}
- Object candidate;
+ Object candidate = null;
if (stage.after(WorkflowExpressionStage.STEP_PRE_INPUT)) {
//somevar -> workflow.current_step.output.somevar
@@ -134,7 +172,9 @@ public class WorkflowExpressionResolution {
//somevar -> workflow.current_step.input.somevar
try {
- candidate = currentStep.getInput(key, Object.class);
+ if (currentStep!=null) {
+ candidate = currentStep.getInput(key, Object.class);
+ }
} catch (Throwable t) {
Exceptions.propagateIfFatal(t);
if (stage==WorkflowExpressionStage.STEP_INPUT && WorkflowVariableResolutionStackEntry.isStackForSettingVariable(RESOLVE_STACK.getAll(true), key) && Exceptions.getFirstThrowableOfType(t, WorkflowVariableRecursiveReference.class)!=null) {
@@ -458,11 +498,11 @@ public class WorkflowExpressionResolution {
public static class AllowBrooklynDslMode {
public static AllowBrooklynDslMode ALL = new AllowBrooklynDslMode(true, null);
- static { ALL.next = () -> ALL; }
+ static { ALL.next = Maybe.of(ALL); }
public static AllowBrooklynDslMode NONE = new AllowBrooklynDslMode(false, null);
- static { NONE.next = () -> NONE; }
- public static AllowBrooklynDslMode CHILDREN_BUT_NOT_HERE = new AllowBrooklynDslMode(false, ()->ALL);
- //public static AllowBrooklynDslMode HERE_BUT_NOT_CHILDREN = new AllowBrooklynDslMode(true, ()->NONE);
+ static { NONE.next = Maybe.of(NONE); }
+ public static AllowBrooklynDslMode CHILDREN_BUT_NOT_HERE = new AllowBrooklynDslMode(false, Maybe.of(ALL));
+ //public static AllowBrooklynDslMode HERE_BUT_NOT_CHILDREN = new AllowBrooklynDslMode(true, Maybe.of(NONE));
private Supplier<AllowBrooklynDslMode> next;
private boolean allowedHere;
@@ -530,22 +570,15 @@ public class WorkflowExpressionResolution {
public Object processTemplateExpressionString(String expression, AllowBrooklynDslMode allowBrooklynDsl) {
if (expression==null) return null;
if (expression.startsWith("$brooklyn:") && allowBrooklynDsl.isAllowedHere()) {
-
+ if (wrappingMode.deferBrooklynDsl) {
+ return WrappedUnresolvedExpression.ofExpression(expression, this, allowBrooklynDsl);
+ }
Object expressionTemplateResolved = processTemplateExpressionString(expression, AllowBrooklynDslMode.NONE);
+ // resolve interpolation before brooklyn DSL, so brooklyn DSL can be passed interpolated vars like workflow scratch;
+ // this means $brooklyn bits that return interpolated strings do not have their interpolation evaluated, which is probably sensible;
+ // and $brooklyn cannot be used inside an interpolated string, which is okay.
Object expressionTemplateAndDslResolved = resolveDsl(expressionTemplateResolved);
return expressionTemplateAndDslResolved;
-
- // previous to 2023-03-30, instead of above, we resolved DSL first. this meant DSL expressions that contained workflow expressions were allowed,
- // which might be useful but probably shouldn't be supported; and furthermore you couldn't pass workflow vars to DSL expressions which should be supported.
-// if (!Objects.equals(e2, expression)) {
-// if (e2 instanceof String) {
-// // proceed to below
-// expression = (String) e2;
-// } else {
-// return processTemplateExpression(e2);
-// }
-// }
-
}
TemplateHashModel model = new WorkflowFreemarkerModel();
@@ -556,10 +589,13 @@ public class WorkflowExpressionResolution {
result = TemplateProcessor.processTemplateContents("workflow", expression, model, true, false, errorMode);
} catch (Exception e) {
Exception e2 = e;
+ if (wrappingMode.deferAndRetryErroneousExpressions) {
+ return WrappedUnresolvedExpression.ofExpression(expression, this, allowBrooklynDsl);
+ }
if (!allowWaiting && Exceptions.isCausedByInterruptInAnyThread(e)) {
e2 = new IllegalArgumentException("Expression value '"+expression+"' unavailable and not permitted to wait: "+ Exceptions.collapseText(e), e);
}
- if (useWrappedValue) {
+ if (wrappingMode.deferThrowingError) {
// in wrapped value mode, errors don't throw until accessed, and when used in conditions they can be tested as absent
return WrappedResolvedExpression.ofError(expression, new ResolutionFailureTreatedAsAbsent.ResolutionFailureTreatedAsAbsentDefaultException(e2));
} else {
@@ -570,12 +606,19 @@ public class WorkflowExpressionResolution {
}
if (!expression.equals(result)) {
- if (useWrappedValue) {
+ // not a static string
+ if (wrappingMode.deferInterpolation) {
+ return WrappedUnresolvedExpression.ofExpression(expression, this, allowBrooklynDsl);
+ }
+ if (wrappingMode.deferBrooklynDsl) {
+ return new WrappedResolvedExpression<Object>(expression, result);
+ }
+ // we try, but don't guarantee, that DSL expressions aren't re-resolved, ie $brooklyn:literal("$brooklyn:literal(\"x\")") won't return x;
+ // this block will return a supplier
+ result = processDslComponents(result);
+
+ if (wrappingMode.wrapResolvedStrings) {
return new WrappedResolvedExpression<Object>(expression, result);
- } else {
- // we try, but don't guarantee, that DSL expressions aren't re-resolved, ie $brooklyn:literal("$brooklyn:literal(\"x\")") won't return x;
- // this block will
- result = processDslComponents(result);
}
}
@@ -636,6 +679,7 @@ public class WorkflowExpressionResolution {
result.error = error;
return result;
}
+
@Override
public T get() {
if (error!=null) {
@@ -651,4 +695,27 @@ public class WorkflowExpressionResolution {
}
}
+ public static class WrappedUnresolvedExpression implements DeferredSupplier<Object> {
+
+ @Deprecated @Beta // might re-introduce but for now needs to cache workflow context -- via resolver -- so discouraged
+ public static WrappedUnresolvedExpression ofExpression(String expression, WorkflowExpressionResolution resolver, AllowBrooklynDslMode dslMode) {
+ return new WrappedUnresolvedExpression(expression, resolver, dslMode);
+ }
+ protected WrappedUnresolvedExpression(String expression, WorkflowExpressionResolution resolver, AllowBrooklynDslMode dslMode) {
+ this.expression = expression;
+ this.resolver = resolver;
+ this.dslMode = dslMode;
+ }
+
+ String expression;
+ WorkflowExpressionResolution resolver;
+ AllowBrooklynDslMode dslMode;
+
+ public Object get() {
+ WorkflowExpressionResolution resolverNow = new WorkflowExpressionResolution(resolver.context, resolver.stage, resolver.allowWaiting,
+ resolver.wrappingMode.wrappingModeWhenResolving(), resolver.errorMode);
+ return resolverNow.processTemplateExpression(expression, dslMode);
+ }
+ }
+
}
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 e7ac95b261..f6a2cdc54a 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
@@ -96,7 +96,7 @@ public abstract class WorkflowStepDefinition {
@JsonIgnore
public DslPredicates.DslPredicate getConditionResolved(WorkflowStepInstanceExecutionContext context) {
try {
- return context.resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, getConditionRaw(), TypeToken.of(DslPredicates.DslPredicate.class));
+ return context.context.resolveCondition(getConditionRaw());
} catch (Exception e) {
throw Exceptions.propagateAnnotated("Unresolveable condition (" + getConditionRaw() + ")", e);
}
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 2e421cddcf..ee2c1379d7 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
@@ -212,8 +212,8 @@ public class WorkflowStepInstanceExecutionContext {
public <T> T resolve(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) {
return context.resolve(stage, expression, type);
}
- public <T> T resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) {
- return context.resolveWrapped(stage, expression, type);
+ public <T> T resolveWrapped(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type, WorkflowExpressionResolution.WrappingMode wrappingMode) {
+ return context.resolveWrapped(stage, expression, type, wrappingMode);
}
public <T> T resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage stage, Object expression, TypeToken<T> type) {
return context.resolveWaiting(stage, expression, type);
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java
index 1a4d5a9200..0fdd8ab981 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java
@@ -246,7 +246,9 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl
AtomicInteger index = new AtomicInteger(0);
((Iterable<?>) targetR).forEach(t -> {
WorkflowExecutionContext nw = newWorkflow(context, t, wasList ? index.getAndIncrement() : null);
- Maybe<Task<Object>> mt = nw.getTask(true);
+ // workflow expressions are accessible in the condition, and the condition was resolveWrapped so will be looked up in the context of the invocation;
+ // thus ${target} can be used anywhere in it and should work
+ Maybe<Task<Object>> mt = nw.getTaskCheckingConditionWithTarget(t);
String targetS = wasList || t !=null ? " for target '"+t+"'" : "";
if (mt.isAbsent()) {
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 93af80a119..72d7a11ebb 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
@@ -25,6 +25,7 @@ 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.resolve.jackson.WrappedValue;
import org.apache.brooklyn.core.sensor.Sensors;
import org.apache.brooklyn.core.workflow.WorkflowExpressionResolution;
import org.apache.brooklyn.core.workflow.WorkflowStepDefinition;
@@ -132,7 +133,7 @@ public class SetSensorWorkflowStep extends WorkflowStepDefinition {
if (old == null && !((AbstractEntity.BasicSensorSupport) entity.sensors()).contains(sensorBase.getName())) {
DslPredicates.DslEntityPredicateDefault requireTweaked = new DslPredicates.DslEntityPredicateDefault();
requireTweaked.sensor = sensorNameFull;
- requireTweaked.check = require;
+ requireTweaked.check = WrappedValue.of(require);
if (!requireTweaked.apply(entity)) {
throw new SensorRequirementFailedAbsent("Sensor " + sensorNameFull + " unset or unavailable when there is a non-absent requirement");
}
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 1c93c96d5c..b262f1efb2 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
@@ -177,7 +177,7 @@ public class HttpWorkflowStep extends WorkflowStepDefinition {
protected void checkExitCode(Integer code, Predicate<Integer> exitcode) {
if (exitcode==null) return;
if (exitcode instanceof DslPredicates.DslPredicateBase) {
- Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEquals;
+ Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEqualsUnwrapped();
if (implicit!=null) {
if ("any".equalsIgnoreCase(""+implicit)) {
// if any is supplied as the implicit value, we accept; e.g. user says "exit_code: any"
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/SshWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/SshWorkflowStep.java
index cb62a4ec53..32743191a1 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/SshWorkflowStep.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/external/SshWorkflowStep.java
@@ -119,7 +119,7 @@ public class SshWorkflowStep extends WorkflowStepDefinition {
}
if (exitcode instanceof DslPredicates.DslPredicateBase) {
- Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEquals;
+ Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEqualsUnwrapped();
if (implicit!=null) {
if ("any".equalsIgnoreCase(""+implicit)) {
// if any is supplied as the implicit value, we accept; e.g. user says "exit_code: any"
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 a9b112a520..2cb1bfd108 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
@@ -216,7 +216,7 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
}
<T> T resolveSubPart(Object v, TypeToken<T> type) {
- return new WorkflowExpressionResolution(context.getWorkflowExectionContext(), WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, false, false, errorMode)
+ return new WorkflowExpressionResolution(context.getWorkflowExectionContext(), WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, false, WorkflowExpressionResolution.WrappingMode.NONE, errorMode)
.resolveWithTemplates(v, type);
}
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 8ed0c3533f..f3d6673095 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
@@ -42,6 +42,7 @@ 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.resolve.jackson.JsonSymbolDependentDeserializer;
+import org.apache.brooklyn.core.resolve.jackson.WrappedValue;
import org.apache.brooklyn.core.sensor.Sensors;
import org.apache.brooklyn.util.JavaGroovyEquivalents;
import org.apache.brooklyn.util.collections.MutableList;
@@ -59,6 +60,7 @@ import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.guava.SerializablePredicate;
import org.apache.brooklyn.util.javalang.Boxing;
import org.apache.brooklyn.util.javalang.Reflections;
+import org.apache.brooklyn.util.javalang.coerce.TryCoercer;
import org.apache.brooklyn.util.text.NaturalOrderComparator;
import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.text.WildcardGlobs;
@@ -87,12 +89,21 @@ public class DslPredicates {
TypeCoercions.registerAdapter(java.util.function.Predicate.class, DslEntityPredicate.class, DslEntityPredicateAdapter::new);
TypeCoercions.registerAdapter(java.util.function.Predicate.class, DslPredicate.class, DslPredicateAdapter::new);
- // subsumed in above
-// TypeCoercions.registerAdapter(com.google.common.base.Predicate.class, DslEntityPredicate.class, DslEntityPredicateAdapter::new);
-// TypeCoercions.registerAdapter(com.google.common.base.Predicate.class, DslPredicate.class, DslPredicateAdapter::new);
- // TODO could use json shorthand instead?
+ // could use json shorthand instead, but this is simpler
TypeCoercions.registerAdapter(String.class, DslPredicate.class, DslPredicates::implicitlyEqualTo);
+
+// TypeCoercions.registerAdapter(DeferredSupplier.class, DslPredicate.class, DslPredicates::implicitlyEqualTo);
+// TypeCoercions.registerAdapter(WorkflowExpressionResolution.WrappedUnresolvedExpression.class, DslPredicate.class, DslPredicates::implicitlyEqualTo);
+ // not sure why above don't work, but below does
+ TypeCoercions.registerAdapter("60-expression-to-predicate", new TryCoercer() {
+ @Override
+ public <T> Maybe<T> tryCoerce(Object input, TypeToken<T> type) {
+ if (!(input instanceof DeferredSupplier)) return null;
+ if (!DslPredicate.class.isAssignableFrom(type.getRawType())) return null;
+ return (Maybe) Maybe.of(type.getRawType().cast(implicitlyEqualTo(input)));
+ }
+ });
}
static {
init();
@@ -114,6 +125,20 @@ public class DslPredicates {
/** always returns false */ NEVER,
}
+ static <T> T unwrapped(WrappedValue<T> t) {
+ return WrappedValue.get(t);
+ }
+
+ static Object unwrappedObject(Object t) {
+ if (t instanceof WrappedValue) return ((WrappedValue)t).get();
+ return t;
+ }
+
+ static Object undeferred(Object t) {
+ if (t instanceof DeferredSupplier) return ((DeferredSupplier)t).get();
+ return t;
+ }
+
public static final boolean coercedEqual(Object a, Object b) {
if (a==null || b==null) return a==null && b==null;
@@ -123,7 +148,7 @@ public class DslPredicates {
if (a.equals(b) || b.equals(a)) return true;
if (a instanceof DeferredSupplier || b instanceof DeferredSupplier)
- return coercedEqual(a instanceof DeferredSupplier ? ((DeferredSupplier)a).get() : a, b instanceof DeferredSupplier ? ((DeferredSupplier)b).get() : b);
+ return coercedEqual(undeferred(a), undeferred(b));
// if classes are equal or one is a subclass of the other, and the above check was false, that is decisive
if (a.getClass().isAssignableFrom(b.getClass())) return false;
@@ -190,7 +215,7 @@ public class DslPredicates {
}
if (a instanceof DeferredSupplier || b instanceof DeferredSupplier)
- return coercedCompare(a instanceof DeferredSupplier ? ((DeferredSupplier)a).get() : a, b instanceof DeferredSupplier ? ((DeferredSupplier)b).get() : b);
+ return coercedCompare(undeferred(a), undeferred(b));
// if classes are equal or one is a subclass of the other, and the above check was false, that is decisive
if (a.getClass().isAssignableFrom(b.getClass()) && b instanceof Comparable) return -((Comparable) b).compareTo(a);
@@ -243,16 +268,16 @@ public class DslPredicates {
@JsonInclude(JsonInclude.Include.NON_NULL)
public static class DslPredicateBase<T> {
- public Object implicitEquals;
- public Object equals;
- public String regex;
- public String glob;
+ public WrappedValue<Object> implicitEquals;
+ public WrappedValue<Object> equals;
+ public WrappedValue<String> regex;
+ public WrappedValue<String> glob;
/** nested check */
- public DslPredicate check;
- public DslPredicate not;
- public List<DslPredicate> any;
- public List<DslPredicate> all;
+ public WrappedValue<DslPredicate> check;
+ public WrappedValue<DslPredicate> not;
+ public List<WrappedValue<DslPredicate>> any;
+ public List<WrappedValue<DslPredicate>> all;
@JsonProperty("assert")
public DslPredicate assertCondition;
@@ -280,19 +305,19 @@ public class DslPredicates {
int checksApplicable = 0;
int checksPassed = 0;
- public <T> void checkTest(T test, java.util.function.Predicate<T> predicateForTest) {
- if (test!=null) {
+ public <T> void checkTest(T testFieldValue, java.util.function.Predicate<T> predicateForTest) {
+ if (testFieldValue!=null) {
checksDefined++;
checksApplicable++;
- if (predicateForTest.test(test)) checksPassed++;
+ if (predicateForTest.test(testFieldValue)) checksPassed++;
}
}
- public <T> void check(T test, Maybe<Object> value, java.util.function.BiPredicate<T,Object> check) {
+ public <T> void check(T testFieldValue, Maybe<Object> value, java.util.function.BiPredicate<T,Object> check) {
if (value.isPresent()) {
- checkTest(test, t -> check.test(t, value.get()));
+ checkTest(testFieldValue, t -> check.test(t, value.get()));
} else {
- if (test!=null) {
+ if (testFieldValue!=null) {
checksDefined++;
}
}
@@ -314,6 +339,8 @@ public class DslPredicates {
}
}
+ public Object implicitEqualsUnwrapped() { return unwrapped(implicitEquals); }
+
public boolean apply(T input) {
Maybe<Object> result = resolveTargetAgainstInput(input);
if (result.isPresent() && result.get() instanceof RetargettedPredicateEvaluation) {
@@ -463,16 +490,24 @@ public class DslPredicates {
public void applyToResolved(Maybe<Object> result, CheckCounts checker) {
if (assertCondition!=null) failOnAssertCondition(result, checker);
- checker.check(implicitEquals, result, (test, value) -> {
+ checker.check(implicitEquals, result, (implicitTestSpec, value) -> {
+
+ // if a condition somehow gets put into the implicit equals, e.g. via an expression returning an expression, then recognize it as a condition
+ Object test = unwrapped(implicitTestSpec);
+ if (test instanceof DslPredicate) {
+ return nestedPredicateCheck((DslPredicate) test, result);
+ }
+
if ((!(test instanceof BrooklynObject) && value instanceof BrooklynObject) ||
(!(test instanceof Iterable) && value instanceof Iterable)) {
- throw new IllegalStateException("Implicit string used for equality check comparing "+test+" with "+value+", which is probably not what was meant. Use explicit 'equals: ...' syntax for this case.");
+ throw new IllegalStateException("Implicit value used for equality check comparing "+test+" with "+value+", which is probably not what was meant. Use explicit 'equals: ...' syntax for this case.");
}
- return DslPredicates.coercedEqual(test, value);
+
+ return DslPredicates.coercedEqual(implicitTestSpec, value);
});
checker.check(equals, result, DslPredicates::coercedEqual);
- checker.check(regex, result, (test, value) -> asStringTestOrFalse(value, v -> Pattern.compile(test, Pattern.DOTALL).matcher(v).matches()));
- checker.check(glob, result, (test, value) -> asStringTestOrFalse(value, v -> WildcardGlobs.isGlobMatched(test, v)));
+ checker.check(regex, result, (test, value) -> asStringTestOrFalse(value, v -> Pattern.compile(unwrapped(test), Pattern.DOTALL).matcher(v).matches()));
+ checker.check(glob, result, (test, value) -> asStringTestOrFalse(value, v -> WildcardGlobs.isGlobMatched(unwrapped(test), v)));
checker.check(inRange, result, (test,value) ->
// current Range only supports Integer, but this code will support any
@@ -503,10 +538,10 @@ public class DslPredicates {
return nestedPredicateCheck(test, Maybe.of(computedSize));
});
- checker.checkTest(not, test -> !nestedPredicateCheck(test, result));
- checker.checkTest(check, test -> nestedPredicateCheck(test, result));
- checker.checkTest(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(p, result)));
- checker.checkTest(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(p, result)));
+ checker.checkTest(not, test -> !nestedPredicateCheck(unwrapped(test), result));
+ checker.checkTest(check, test -> nestedPredicateCheck(unwrapped(test), result));
+ checker.checkTest(any, test -> test.stream().anyMatch(p -> nestedPredicateCheck(unwrapped(p), result)));
+ checker.checkTest(all, test -> test.stream().allMatch(p -> nestedPredicateCheck(unwrapped(p), result)));
checker.check(javaInstanceOf, result, this::checkJavaInstanceOf);
}
@@ -517,7 +552,7 @@ public class DslPredicates {
boolean assertionPassed;
if (assertCondition instanceof DslPredicateBase) {
- Object implicitWhen = ((DslPredicateBase) assertCondition).implicitEquals;
+ Object implicitWhen = ((DslPredicateBase) assertCondition).implicitEqualsUnwrapped();
if (implicitWhen!=null) {
// can assume no other checks, if one is implicit
CheckCounts checker = new CheckCounts();
@@ -579,7 +614,7 @@ public class DslPredicates {
// first check if implicitly equal to a registered type
if (javaInstanceOf instanceof DslPredicateBase) {
- Object implicitRegisteredType = ((DslPredicateBase) javaInstanceOf).implicitEquals;
+ Object implicitRegisteredType = ((DslPredicateBase) javaInstanceOf).implicitEqualsUnwrapped();
if (implicitRegisteredType instanceof String) {
Entity ent = null;
if (value instanceof Entity) ent = (Entity)value;
@@ -673,16 +708,17 @@ public class DslPredicates {
Entity.class, EntityAdjuncts.getEntity(bo, true).orNull()));
}
+ /** default implementation */
@Beta
public static class DslPredicateDefault<T2> extends DslPredicateBase<T2> implements DslPredicate<T2>, Cloneable {
public DslPredicateDefault() {}
// allow a string or int or other common types to be an implicit equality target
- public DslPredicateDefault(String implicitEquals) { this.implicitEquals = implicitEquals; }
- public DslPredicateDefault(Integer implicitEquals) { this.implicitEquals = implicitEquals; }
- public DslPredicateDefault(Double implicitEquals) { this.implicitEquals = implicitEquals; }
- public DslPredicateDefault(Long implicitEquals) { this.implicitEquals = implicitEquals; }
- public DslPredicateDefault(Number implicitEquals) { this.implicitEquals = implicitEquals; } // note: Number is not matched by jackson bean constructor
+ public DslPredicateDefault(String implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); }
+ public DslPredicateDefault(Integer implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); }
+ public DslPredicateDefault(Double implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); }
+ public DslPredicateDefault(Long implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); }
+ public DslPredicateDefault(Number implicitEquals) { this.implicitEquals = WrappedValue.of(implicitEquals); } // note: Number is not matched by jackson bean constructor
// not used by code, but allows clients to store other information
public Object metadata;
@@ -1018,13 +1054,13 @@ public class DslPredicates {
public static DslPredicate equalTo(Object x) {
DslEntityPredicateDefault result = new DslEntityPredicateDefault();
- result.equals = x;
+ result.equals = WrappedValue.of(x);
return result;
}
public static DslPredicate implicitlyEqualTo(Object x) {
DslEntityPredicateDefault result = new DslEntityPredicateDefault();
- result.implicitEquals = x;
+ result.implicitEquals = WrappedValue.of(x);
return result;
}
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 517195896d..9df6e26cc7 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
@@ -901,4 +901,20 @@ public class WorkflowNestedAndCustomExtensionTest extends RebindTestFixture<Test
"steps", MutableList.of("return element-${key}-${value}"))));
Asserts.assertEquals(output, MutableList.of("element-K1-V1", "element-K2-V2"));
}
+
+ @Test
+ public void testForeachCondition() throws Exception {
+ Object output = invokeWorkflowStepsWithLogging(MutableList.of(
+ "let list L = [ a, b, c ]",
+ MutableMap.of("step", "foreach item in ${L}",
+ "steps", MutableList.of("return ${item}"),
+ "condition", MutableMap.of("any", MutableList.of(
+ "a",
+ MutableMap.of("target", MutableList.of("x", "c"),
+ "has-element",
+ "${item}"
+// MutableMap.of("equals", "${item}")
+ ))))));
+ Asserts.assertEquals(output, MutableList.of("a", "c"));
+ }
}
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 fefe1e28fa..08a3c77d86 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
@@ -18,6 +18,7 @@
*/
package org.apache.brooklyn.util.core.predicates;
+import org.apache.brooklyn.core.resolve.jackson.WrappedValue;
import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.util.collections.MutableList;
@@ -533,7 +534,7 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
"tag", "locationTagValueMatched"), DslPredicates.DslPredicate.class);
Asserts.assertInstanceOf(p, DslPredicates.DslPredicateDefault.class);
Asserts.assertInstanceOf( ((DslPredicates.DslPredicateDefault)p).tag, DslPredicates.DslPredicateDefault.class);
- Asserts.assertEquals( ((DslPredicates.DslPredicateDefault) ((DslPredicates.DslPredicateDefault)p).tag).implicitEquals, "locationTagValueMatched");
+ Asserts.assertEquals( ((DslPredicates.DslPredicateDefault) ((DslPredicates.DslPredicateDefault)p).tag).implicitEqualsUnwrapped(), "locationTagValueMatched");
}
@Test
@@ -542,7 +543,7 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
DslPredicates.DslPredicate p = TypeCoercions.coerce(MutableMap.of("target", "location", "equals", kvMap, "config", "x"),
DslPredicates.DslPredicate.class);
Asserts.assertInstanceOf(p, DslPredicates.DslPredicateDefault.class);
- Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)p).equals, kvMap);
+ Asserts.assertEquals( WrappedValue.get( ((DslPredicates.DslPredicateDefault)p).equals ), kvMap);
Asserts.assertEquals( ((DslPredicates.DslPredicateDefault)p).config, "x");
}
diff --git a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java
index 9c4ad3d39e..367e3575da 100644
--- a/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java
+++ b/software/base/src/main/java/org/apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java
@@ -108,7 +108,7 @@ public class ContainerWorkflowStep extends WorkflowStepDefinition {
protected void checkExitCode(ContainerTaskResult ptw, DslPredicates.DslPredicate<Integer> exitcode) {
if (exitcode==null) return;
if (exitcode instanceof DslPredicates.DslPredicateBase) {
- Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEquals;
+ Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEqualsUnwrapped();
if (implicit!=null) {
if ("any".equalsIgnoreCase(""+implicit)) {
// if any is supplied as the implicit value, we accept; e.g. user says "exit-code: any"
diff --git a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java
index ba75c6aed5..087d710fcb 100644
--- a/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java
+++ b/software/winrm/src/main/java/org/apache/brooklyn/location/winrm/WinrmWorkflowStep.java
@@ -106,7 +106,7 @@ public class WinrmWorkflowStep extends WorkflowStepDefinition {
protected void checkExitCode(ProcessTaskWrapper<?> ptw, DslPredicates.DslPredicate<Integer> exitcode) {
if (exitcode==null) return;
if (exitcode instanceof DslPredicates.DslPredicateBase) {
- Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEquals;
+ Object implicit = ((DslPredicates.DslPredicateBase) exitcode).implicitEqualsUnwrapped();
if (implicit!=null) {
if ("any".equalsIgnoreCase(""+implicit)) {
// if any is supplied as the implicit value, we accept; e.g. user says "exit-code: any"
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TryCoercer.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TryCoercer.java
index f410fbc92d..27243a40d6 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TryCoercer.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TryCoercer.java
@@ -18,10 +18,9 @@
*/
package org.apache.brooklyn.util.javalang.coerce;
-import org.apache.brooklyn.util.guava.Maybe;
-
import com.google.common.annotations.Beta;
import com.google.common.reflect.TypeToken;
+import org.apache.brooklyn.util.guava.Maybe;
/**
* A coercer that can be registered, which will try to coerce the given input to the given type.
[brooklyn-server] 01/10: tidy up tests, add some notes on gaps
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 043cb184445a9210de5f0117041defa38cabcb99
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Jun 15 11:12:49 2023 +0100
tidy up tests, add some notes on gaps
---
.../core/workflow/WorkflowTransformTest.java | 345 ++++++++-------------
1 file changed, 122 insertions(+), 223 deletions(-)
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
index 4460761ca6..457030d84e 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
@@ -18,9 +18,12 @@
*/
package org.apache.brooklyn.core.workflow;
+import org.apache.brooklyn.api.entity.Entity;
import org.apache.brooklyn.api.entity.EntityLocal;
import org.apache.brooklyn.api.entity.EntitySpec;
+import org.apache.brooklyn.api.mgmt.ManagementContext;
import org.apache.brooklyn.api.mgmt.Task;
+import org.apache.brooklyn.core.entity.Entities;
import org.apache.brooklyn.core.test.BrooklynMgmtUnitTestSupport;
import org.apache.brooklyn.entity.stock.BasicApplication;
import org.apache.brooklyn.test.Asserts;
@@ -28,10 +31,14 @@ import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.core.config.ConfigBag;
import org.apache.brooklyn.util.text.Strings;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import java.util.Arrays;
import java.util.Map;
import java.util.function.Function;
+import java.util.stream.Collectors;
public class WorkflowTransformTest extends BrooklynMgmtUnitTestSupport {
@@ -39,278 +46,170 @@ public class WorkflowTransformTest extends BrooklynMgmtUnitTestSupport {
WorkflowBasicTest.addWorkflowStepTypes(mgmt);
}
- @Test
- public void testTransformTrim() throws Exception {
- loadTypes();
-
- String untrimmed = "Hello, World! ";
- String trimmed = untrimmed.trim();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
+ BasicApplication app;
- WorkflowEffector eff = new WorkflowEffector(ConfigBag.newInstance()
- .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
- .configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
- .configure(WorkflowEffector.STEPS, MutableList.<Object>of()
- .append("let mystring = '"+untrimmed+"'")
- .append("transform mytrimmedstring = ${mystring} | trim")
- .append("return ${mytrimmedstring}")
- )
- );
- eff.apply((EntityLocal)app);
-
- Task<?> invocation = app.invoke(app.getEntityType().getEffectorByName("myWorkflow").get(), null);
- Object result = invocation.getUnchecked();
- Asserts.assertNotNull(result);
- Asserts.assertEquals(result, trimmed);
- }
-
- @Test
- public void testTransformRegex() throws Exception {
+ @BeforeMethod(alwaysRun = true)
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
-
- WorkflowEffector eff = new WorkflowEffector(ConfigBag.newInstance()
- .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
- .configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
- .configure(WorkflowEffector.STEPS, MutableList.<Object>of()
- .append("transform x = 'silly world' | replace regex l. k")
- .append("return ${x}")
- )
- );
- eff.apply((EntityLocal)app);
-
- Task<?> invocation = app.invoke(app.getEntityType().getEffectorByName("myWorkflow").get(), null);
- Object result = invocation.getUnchecked();
- Asserts.assertNotNull(result);
- Asserts.assertEquals(result, "siky world");
+ app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
}
- @Test
- public void testTransformAllRegex() throws Exception {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
+ @AfterMethod(alwaysRun = true, timeOut = Asserts.THIRTY_SECONDS_TIMEOUT_MS)
+ @Override
+ public void tearDown() throws Exception {
+ if (app!=null) Entities.unmanage(app);
+ super.tearDown();
+ }
- WorkflowEffector eff = new WorkflowEffector(ConfigBag.newInstance()
- .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
- .configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
- .configure(WorkflowEffector.STEPS, MutableList.<Object>of()
- .append("transform x = 'silly world' | replace all regex l. k")
- .append("return ${x}")
- )
- );
- eff.apply((EntityLocal)app);
+ static Object runWorkflowSteps(Entity entity, String ...steps) {
+ return WorkflowBasicTest.runWorkflow(entity, Arrays.asList(steps).stream().map(s -> "- "+Strings.indent(2, s).trim()).collect(Collectors.joining("\n")), "test").getTask(false).get().getUnchecked();
+ }
+ Object runWorkflowSteps(String ...steps) {
+ return runWorkflowSteps(app, steps);
+ }
- Task<?> invocation = app.invoke(app.getEntityType().getEffectorByName("myWorkflow").get(), null);
- Object result = invocation.getUnchecked();
- Asserts.assertNotNull(result);
- Asserts.assertEquals(result, "siky work");
+ Object transform(String args) {
+ return runWorkflowSteps(app, "transform "+args);
}
@Test
- public void testTransformRegexWithBackslash() throws Exception {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
-
- WorkflowEffector eff = new WorkflowEffector(ConfigBag.newInstance()
- .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
- .configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
- .configure(WorkflowEffector.STEPS, MutableList.<Object>of()
- .append("transform x = 'abc/def/ghi' | replace regex 'c/d' XXX")
- .append("return ${x}")
- )
- );
- eff.apply((EntityLocal)app);
+ public void testTransformTrim() throws Exception {
+ String untrimmed = "Hello, World! ";
+ String trimmed = untrimmed.trim();
- Task<?> invocation = app.invoke(app.getEntityType().getEffectorByName("myWorkflow").get(), null);
- Object result = invocation.getUnchecked();
- Asserts.assertNotNull(result);
- Asserts.assertEquals(result, "abXXXef/ghi");
+ Asserts.assertEquals(
+ runWorkflowSteps(
+ "let mystring = '"+untrimmed+"'",
+ "transform mytrimmedstring = ${mystring} | trim",
+ "return ${mytrimmedstring}"),
+ trimmed);
}
@Test
- public void testTransformRegexWithSpace() throws Exception {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
-
- WorkflowEffector eff = new WorkflowEffector(ConfigBag.newInstance()
- .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
- .configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
- .configure(WorkflowEffector.STEPS, MutableList.<Object>of()
- .append("transform x = 'abc def ghi' | replace regex 'c d' XXX")
- .append("return ${x}")
- )
- );
- eff.apply((EntityLocal)app);
-
- Task<?> invocation = app.invoke(app.getEntityType().getEffectorByName("myWorkflow").get(), null);
- Object result = invocation.getUnchecked();
- Asserts.assertNotNull(result);
- Asserts.assertEquals(result, "abXXXef ghi");
+ public void testTransformRegex() throws Exception {
+ Asserts.assertEquals(transform("value 'silly world' | replace regex l. k"), "siky world");
+ Asserts.assertEquals(transform("value 'silly world' | replace all regex l. k"), "siky work");
+ // with slash
+ Asserts.assertEquals(transform("value 'abc/def/ghi' | replace regex 'c/d' XXX"), "abXXXef/ghi");
+ // with space
+ Asserts.assertEquals(transform("value 'abc def ghi' | replace regex 'c d' XXX"), "abXXXef ghi");
+
+ // greedy
+ Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace regex 'c.*d' XXX"), "abXXX");
+ Asserts.assertEquals(transform("value 'abc def ghi c2d' | replace all regex 'c.*d' XXX"), "abXXX");
+ // TODO this fails
+// Asserts.assertEquals(transform("value 'abc def ghi' | replace regex 'c d' ''"), "abef ghi");
}
@Test
public void testTransformLiteral() throws Exception {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
-
- WorkflowEffector eff = new WorkflowEffector(ConfigBag.newInstance()
- .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
- .configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
- .configure(WorkflowEffector.STEPS, MutableList.<Object>of()
- .append("transform x = 'abc.*def ghi' | replace literal c.*d XXX")
- .append("return ${x}")
- )
- );
- eff.apply((EntityLocal)app);
-
- Task<?> invocation = app.invoke(app.getEntityType().getEffectorByName("myWorkflow").get(), null);
- Object result = invocation.getUnchecked();
- Asserts.assertNotNull(result);
- Asserts.assertEquals(result, "abXXXef ghi");
+ Asserts.assertEquals(transform("value 'abc def ghi' | replace literal c.*d XXX"), "abc def ghi");
+ Asserts.assertEquals(transform("value 'abc.*def ghi c.*d' | replace literal c.*d XXX"), "abXXXef ghi c.*d");
+ Asserts.assertEquals(transform("value 'abc.*def ghi c.*d' | replace all literal c.*d XXX"), "abXXXef ghi XXX");
}
@Test
public void testTransformGlob() throws Exception {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
-
- WorkflowEffector eff = new WorkflowEffector(ConfigBag.newInstance()
- .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
- .configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
- .configure(WorkflowEffector.STEPS, MutableList.<Object>of()
- .append("transform x = 'abc.*def ghi' | replace glob c*e XXX")
- .append("return ${x}")
- )
- );
- eff.apply((EntityLocal)app);
-
- Task<?> invocation = app.invoke(app.getEntityType().getEffectorByName("myWorkflow").get(), null);
- Object result = invocation.getUnchecked();
- Asserts.assertNotNull(result);
- Asserts.assertEquals(result, "abXXXf ghi");
+ Asserts.assertEquals(transform("value 'abc def ghi' | replace glob c*e XXX"), "abXXXf ghi");
+ // TODO glob is greedy, so all has no effect
+ Asserts.assertEquals(transform("value 'abc def ghi c2e' | replace glob c*e XXX"), "abXXX");
+ Asserts.assertEquals(transform("value 'abc def ghi c2e' | replace all glob c*e XXX"), "abXXX");
}
@Test
public void testMapDirect() {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
-
- WorkflowEffector eff = new WorkflowEffector(ConfigBag.newInstance()
- .configure(WorkflowEffector.EFFECTOR_NAME, "myWorkflow")
- .configure(WorkflowEffector.EFFECTOR_PARAMETER_DEFS, MutableMap.of("p1", MutableMap.of("defaultValue", "p1v")))
- .configure(WorkflowEffector.STEPS, MutableList.<Object>of()
- .append("let map myMap = {a: 1}")
- .append("let myMap.a = 2")
- .append("return ${myMap.a}")
- )
- );
- eff.apply((EntityLocal)app);
-
- Task<?> invocation = app.invoke(app.getEntityType().getEffectorByName("myWorkflow").get(), null);
- Object result = invocation.getUnchecked();
- Asserts.assertNotNull(result);
- Asserts.assertEquals(result, "2");
+ Asserts.assertEquals(runWorkflowSteps(
+ "let map myMap = {a: 1}",
+ "let myMap.a = 2",
+ "return ${myMap.a}"),
+ "2");
}
@Test
public void testReturnTransformWithMapYaml() {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
- Object result = WorkflowBasicTest.runWorkflow(app, Strings.lines(
- "- step: let s",
- " value: |",
- " bogus",
- " - not valid: yaml: here",
- " that's: okay",
- " ---",
- " key: value",
- "- transform s | yaml | return",
- "- return should not come here",
- ""), "test").getTask(false).get().getUnchecked();
- Asserts.assertInstanceOf(result, Map.class);
- Asserts.assertEquals(result, MutableMap.of("key", "value"));
+ Asserts.assertEquals(WorkflowBasicTest.runWorkflow(app, Strings.lines(
+ "- step: let s",
+ " value: |",
+ " bogus",
+ " - not valid: yaml: here",
+ " that's: okay",
+ " ---",
+ " key: value",
+ "- transform s | yaml | return",
+ "- return should not come here",
+ ""), "test").getTask(false).get().getUnchecked(),
+ MutableMap.of("key", "value"));
}
@Test
public void testSetVarTransform() {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
- Object result = WorkflowBasicTest.runWorkflow(app, Strings.lines(
- "- step: let s",
- " value: \"key: Value\"",
- "- transform s | yaml | set y",
- "- transform y.key2 = ${output.key} | to_upper_case",
- "- transform output.key | to_lower_case", // output should still be the yaml map transformed from ${s}
- "- transform output | set y.key3", // output passed in here will be 'value' from previous step
- "- transform value true | set y.key4",
- "- transform boolean value true | set y.key5",
- "- return ${y}",
- ""), "test").getTask(false).get().getUnchecked();
- Asserts.assertInstanceOf(result, Map.class);
- Asserts.assertEquals(result, MutableMap.of("key", "Value", "key2", "VALUE", "key3", "value", "key4", "true", "key5", true));
+ Asserts.assertEquals(WorkflowBasicTest.runWorkflow(app, Strings.lines(
+ "- step: let s",
+ " value: \"key: Value\"",
+ "- transform s | yaml | set y",
+ "- transform y.key2 = ${output.key} | to_upper_case",
+ "- transform output.key | to_lower_case", // output should still be the yaml map transformed from ${s}
+ "- transform output | set y.key3", // output passed in here will be 'value' from previous step
+ "- transform value true | set y.key4",
+ "- transform boolean value true | set y.key5",
+ "- return ${y}",
+ ""), "test").getTask(false).get().getUnchecked(),
+ MutableMap.of("key", "Value", "key2", "VALUE", "key3", "value", "key4", "true", "key5", true));
}
@Test
public void testResolveTransform() {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
- Object result = WorkflowBasicTest.runWorkflow(app, Strings.lines(
- "- let a = b",
- "- let b = c",
- "- let x = \"${\" ${a} \"}\"",
- "- transform x | resolve_expression | return",
- ""), "test").getTask(false).get().getUnchecked();
- Asserts.assertEquals(result, "c");
-
- result = WorkflowBasicTest.runWorkflow(app, Strings.lines(
- "- let a = b",
- "- let b = c",
- "- transform value {${a}} | prepend $ | resolve_expression | return",
- ""), "test").getTask(false).get().getUnchecked();
- Asserts.assertEquals(result, "c");
+ Asserts.assertEquals(runWorkflowSteps(
+ "let a = b",
+ "let b = c",
+ "let x = \"${\" ${a} \"}\"",
+ "transform x | resolve_expression | return"
+ ), "c");
+
+ Asserts.assertEquals(runWorkflowSteps(
+ "let a = b",
+ "let b = c",
+ "transform value {${a}} | prepend $ | resolve_expression | return"
+ ), "c");
}
@Test
public void testSliceRemoveAndAppendTransform() {
- loadTypes();
-
- BasicApplication app = mgmt.getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
-
- Function<String,Object> transform = tx -> WorkflowBasicTest.runWorkflow(app, Strings.lines("- "+tx), "test").getTask(false).get().getUnchecked();
-
// slice list
- Asserts.assertEquals(transform.apply("transform value ['a','bb','ccc'] | type list | slice 1"), MutableList.of("bb", "ccc"));
- Asserts.assertEquals(transform.apply("transform value ['a','bb','ccc'] | type list | slice 1 -1"), MutableList.of("bb"));
- Asserts.assertEquals(transform.apply("transform value ['a','bb','ccc'] | type list | slice -1"), MutableList.of("ccc"));
+ Asserts.assertEquals(transform("value ['a','bb','ccc'] | type list | slice 1"), MutableList.of("bb", "ccc"));
+ Asserts.assertEquals(transform("value ['a','bb','ccc'] | type list | slice 1 -1"), MutableList.of("bb"));
+ Asserts.assertEquals(transform("value ['a','bb','ccc'] | type list | slice -1"), MutableList.of("ccc"));
// slice string
- Asserts.assertEquals(transform.apply("transform value abc | slice 1"), "bc");
+ Asserts.assertEquals(transform("value abc | slice 1"), "bc");
+
+ // append and prepend list
+ Asserts.assertEquals(transform("value ['a','bb'] | type list | append ccc"), MutableList.of("a", "bb", "ccc"));
+ Asserts.assertEquals(runWorkflowSteps(
+ "transform value ['ccc'] | type list | set c",
+ "transform value ['a','bb'] | type list | append ${c}"
+ ), MutableList.of("a", "bb", MutableList.of("ccc")));
+ Asserts.assertEquals(transform("value ['a','bb'] | type list | prepend ccc"), MutableList.of("ccc", "a", "bb"));
- // append list
- Asserts.assertEquals(transform.apply("transform value ['a','bb'] | type list | append ccc"), MutableList.of("a", "bb", "ccc"));
- Asserts.assertEquals(transform.apply("transform value ['ccc'] | type list | set c\n" +
- "- transform value ['a','bb'] | type list | append ${c}"), MutableList.of("a", "bb", MutableList.of("ccc")));
+ // append and prepend string
+ Asserts.assertEquals(transform("value hello | append _world"), "hello_world");
+ // TODO this fails
+// Asserts.assertEquals(transform("value hello | append \" world\""), "hello world");
// remove list
- Asserts.assertEquals(transform.apply("transform value ['a','bb','ccc'] | type list | remove 1"), MutableList.of("a", "ccc"));
+ Asserts.assertEquals(transform("value ['a','bb','ccc'] | type list | remove 1"), MutableList.of("a", "ccc"));
// remove map
- Asserts.assertEquals(transform.apply("step: transform\n" +
- " transform: type map | remove b\n" +
- " value:\n" +
- " a: 1\n" +
- " b: 2"), MutableMap.of("a", 1));
+ Asserts.assertEquals(
+ runWorkflowSteps(
+ "step: transform\n" +
+ "transform: type map | remove b\n" +
+ "value:\n" +
+ " a: 1\n" +
+ " b: 2"),
+ MutableMap.of("a", 1));
}
}
[brooklyn-server] 04/10: add support for equals boolean check, and better errror/logging on ternary
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 eb8dea1528335449f28179c6354843dfa1e3a85a
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Sat Jun 17 00:20:11 2023 +0100
add support for equals boolean check, and better errror/logging on ternary
---
.../steps/variables/SetVariableWorkflowStep.java | 46 ++++++++++++----------
.../core/workflow/WorkflowOperandsTest.java | 8 ++++
2 files changed, 34 insertions(+), 20 deletions(-)
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 7da45ca46f..63f2a893d0 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
@@ -232,14 +232,14 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
wordsByQuote = qst.remainderAsList();
}
// then look for operators etc
- return process(wordsByQuote, null);
+ return process(wordsByQuote);
}
QuotedStringTokenizer qst(String input) {
return QuotedStringTokenizer.builder().includeQuotes(true).includeDelimiters(true).expectQuotesDelimited(true).failOnOpenQuote(true).build(input);
}
- Object process(List<String> w, Function<String,TypeToken<?>> explicitType) {
+ Object process(List<String> w) {
// if no tokens, treat as null
if (w.isEmpty()) return null;
@@ -276,6 +276,7 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
// #6: < <= > >=
result = handleTokenIfPresent(w, false, MutableMap.of(
+ "==", this::handleEquals,
"<", this::handleOrderedLessThan,
"<=", this::handleOrderedLessThanOrEqual,
">", this::handleOrderedGreaterThan,
@@ -332,12 +333,12 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
}
Object handleNullish(List<String> lhs, List<String> rhs) {
- return processMaybe(lhs, null).or(() -> process(rhs, null));
+ return processMaybe(lhs, null).or(() -> process(rhs));
}
Maybe<Object> processMaybe(List<String> lhs, Function<String,TypeToken<?>> explicitType) {
try {
- Object result = process(lhs, explicitType);
+ Object result = process(lhs);
if (result!=null) return Maybe.of(result);
return Maybe.absent("null");
@@ -372,8 +373,8 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
}
Object applyMathOperator(String op, List<String> lhs0, List<String> rhs0, BiFunction<Integer,Integer,Number> ifInt, BiFunction<Double,Double,Number> ifDouble) {
- Object lhs = process(lhs0, null);
- Object rhs = process(rhs0, null);
+ Object lhs = process(lhs0);
+ Object rhs = process(rhs0);
if ("+".equals(op)) {
if (lhs instanceof Duration) {
@@ -426,8 +427,8 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
}
Object applyBooleanOperator(List<String> lhs0, List<String> rhs0, BiFunction<Boolean, Boolean, Boolean> biFn) {
- Object lhs = process(lhs0, null);
- Object rhs = process(rhs0, null);
+ Object lhs = process(lhs0);
+ Object rhs = process(rhs0);
Maybe<Boolean> lhsB = asBoolean(lhs);
Maybe<Boolean> rhsB = asBoolean(rhs);
@@ -437,9 +438,9 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
throw new IllegalArgumentException("Should not come here");
}
- Object applyComparison(List<String> lhs0, List<String> rhs0, Function<Integer, Boolean> test) {
- Object lhs = process(lhs0, null);
- Object rhs = process(rhs0, null);
+ boolean applyComparison(List<String> lhs0, List<String> rhs0, Function<Integer, Boolean> test) {
+ Object lhs = process(lhs0);
+ Object rhs = process(rhs0);
return DslPredicates.coercedCompare(lhs, rhs, test);
}
@@ -492,9 +493,13 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
return applyComparison(lhs, rhs, v -> v<=0);
}
+ boolean handleEquals(List<String> lhs, List<String> rhs) {
+ return DslPredicates.coercedEqual(process(lhs), process(rhs));
+ }
+
Object handleTernaryCondition(List<String> lhs0, List<String> rhs0) {
- log.info(String.format("Ternary Condition 0: [lhs:%s][rhs:%s]", lhs0, rhs0));
- Object lhs = process(lhs0, null);
+ //log.info(String.format("Ternary Condition 0: [lhs:%s][rhs:%s]", lhs0, rhs0));
+ Object lhs = process(lhs0);
Object rhs;
int questionIndex = rhs0.indexOf("?");
int colonIndex = rhs0.indexOf(":");
@@ -506,9 +511,9 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
rhs = handleChainedTernaryRhs(rhs0);
} else {
// non-nested or chained
- rhs = process(rhs0, null);
+ rhs = process(rhs0);
}
- log.info(String.format("Ternary Condition 1: [lhs:%s][rhs:%s]", lhs, rhs));
+ //log.info(String.format("Ternary Condition 1: [lhs:%s][rhs:%s]", lhs, rhs));
if (!(rhs instanceof TernaryArms)) throw new IllegalArgumentException("Mismatched ternary ':' operator");
@@ -517,14 +522,15 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
if (condition.get()) {
// ? left : right -- rhs length is 5 [left, ,:, ,right]
// ? true && true : false || false -- rhs length is ???
- return process( ((TernaryArms)rhs).getLeft(), null );
+ return process( ((TernaryArms)rhs).getLeft());
//throw new IllegalArgumentException("TERNARY CONDITION IS TRUE");
} else {
- return process( ((TernaryArms)rhs).getRight(), null );
+ return process( ((TernaryArms)rhs).getRight());
//throw new IllegalArgumentException("TERNARY CONDITION IS FALSE");
}
}
- throw new IllegalArgumentException("Should not come here");
+
+ throw new IllegalArgumentException("Leading term of ternary '"+lhs+"' does not evaluate to a boolean");
}
public TernaryArms handleNestedTernaryRhs(List<String> rhs) {
@@ -534,7 +540,7 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
}
int firstColonIndex = rhs.indexOf(":");
if (firstColonIndex == lastColonIndex) {
- return (TernaryArms) process(rhs, null);
+ return (TernaryArms) process(rhs);
}
return new TernaryArms(trim(rhs.subList(0, lastColonIndex)), trim(rhs.subList(lastColonIndex + 1, rhs.size())));
}
@@ -554,7 +560,7 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
}
public Object handleTernaryArms(List<String> lhs0, List<String> rhs0) {
- log.info(String.format("Ternary 0: [lhs:%s][rhs:%s]", lhs0, rhs0));
+ //log.info(String.format("Ternary 0: [lhs:%s][rhs:%s]", lhs0, rhs0));
return new TernaryArms(lhs0, rhs0);
}
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowOperandsTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowOperandsTest.java
index 2deb0d3ef8..4f61339c9e 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowOperandsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowOperandsTest.java
@@ -192,5 +192,13 @@ public class WorkflowOperandsTest extends BrooklynMgmtUnitTestSupport {
// chained
assertEvaluated("false ? ignored : true ? \"b\" : \"c\"", "b");
+
+ assertEvaluated("false ? ignored : true ? \"b\" : \"c\"", "b");
+
+ Asserts.assertEquals(runSteps(MutableList.of(
+ "let integer x = 4",
+ "let integer y = ${x} == 4 ? 5 : 6",
+ "return ${y}"
+ )), 5);
}
}
[brooklyn-server] 03/10: handle quotes in transform arguments
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 4edec7a9b285d84e80fa83898ad803c47a4af3d5
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu Jun 15 11:20:18 2023 +0100
handle quotes in transform arguments
---
.../workflow/steps/variables/TransformVariableWorkflowStep.java | 8 +++++++-
.../org/apache/brooklyn/core/workflow/WorkflowTransformTest.java | 3 +--
2 files changed, 8 insertions(+), 3 deletions(-)
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 14c8ff78d8..390dbcae96 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
@@ -34,6 +34,7 @@ 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.QuotedStringTokenizer;
import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.time.Duration;
import org.slf4j.Logger;
@@ -169,7 +170,12 @@ public class TransformVariableWorkflowStep extends WorkflowStepDefinition {
}
WorkflowTransformWithContext getTransform(WorkflowStepInstanceExecutionContext context, String transformDef) {
- List<String> transformWords = Arrays.asList(transformDef.split(" "));
+ List<String> transformWords;
+ // old
+// transformWords = Arrays.asList(transformDef.split(" "));
+ // new, respecting quotes
+ transformWords = QuotedStringTokenizer.builder().includeQuotes(false).includeDelimiters(false).expectQuotesDelimited(true).failOnOpenQuote(true).build(transformDef).remainderAsList();
+
String transformType = transformWords.get(0);
Function t = Maybe.ofDisallowingNull(TRANSFORMATIONS.get(transformType)).map(Supplier::get).orNull();
if (t==null) {
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
index a07d3e2ce7..9f90a6dfcd 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
@@ -198,8 +198,7 @@ public class WorkflowTransformTest extends BrooklynMgmtUnitTestSupport {
// append and prepend string
Asserts.assertEquals(transform("value hello | append _world"), "hello_world");
- // TODO this fails
-// Asserts.assertEquals(transform("value hello | append \" world\""), "hello world");
+ Asserts.assertEquals(transform("value hello | append \" world\""), "hello world");
// remove list
Asserts.assertEquals(transform("value ['a','bb','ccc'] | type list | remove 1"), MutableList.of("a", "ccc"));
[brooklyn-server] 05/10: fix recording of errors in step in subworkflow in some cases
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 46267ef0d35c9be4dc78c69013201af65a97a29f
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Sat Jun 17 15:32:53 2023 +0100
fix recording of errors in step in subworkflow in some cases
---
.../core/workflow/WorkflowExecutionContext.java | 38 ++++++-----
.../WorkflowStepInstanceExecutionContext.java | 3 +-
.../workflow/WorkflowPersistReplayErrorsTest.java | 74 ++++++++++++++++++++++
.../brooklyn/core/workflow/WorkflowRetryTest.java | 1 +
4 files changed, 100 insertions(+), 16 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
index 26fc66df27..ec05fcb380 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java
@@ -83,7 +83,6 @@ import static org.apache.brooklyn.core.workflow.WorkflowReplayUtils.ReplayResume
@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonDeserialize(converter = WorkflowExecutionContext.Converter.class)
public class WorkflowExecutionContext {
-
private static final Logger log = LoggerFactory.getLogger(WorkflowExecutionContext.class);
public static final String LABEL_FOR_ERROR_HANDLER = "error-handler";
@@ -213,7 +212,7 @@ public class WorkflowExecutionContext {
public static class OldStepRecord {
/** count of runs started */
int countStarted = 0;
- /** count of runs completed */
+ /** count of runs completed (without error) */
int countCompleted = 0;
/** context for last _completed_ instance of step */
@@ -1102,6 +1101,9 @@ public class WorkflowExecutionContext {
if (unhandledError != null) {
return () -> endWithError(unhandledError.getLeft(), unhandledError.getRight());
}
+ if (!continueOnErrorHandledOrNextReplay) {
+ updateOnSuccessfulCompletion();
+ }
} catch (Throwable e2) {
// do not propagateIfFatal, we need to handle most throwables
log.debug("Uncaught error in workflow exception handler: "+ e2, e2);
@@ -1500,20 +1502,26 @@ public class WorkflowExecutionContext {
onFinish.accept(step.output, null);
} catch (Exception e) {
- handleErrorAtStep(step, t, onFinish, e);
- }
-
- oldStepInfo.compute(currentStepIndex, (index, old) -> {
- if (old==null) {
- log.warn("Lost old step info for "+this+", step "+index);
- old = new OldStepRecord();
+ try {
+ handleErrorAtStep(step, t, onFinish, e);
+ } catch (Exception e2) {
+ currentStepInstance.error = e2;
+ throw e2;
}
- old.countCompleted++;
- // okay if this gets picked up by accident because we will check the stepIndex it records against the currentStepIndex,
- // and ignore it if different
- old.context = currentStepInstance;
- return old;
- });
+ } finally {
+ // do this whether or not error
+ oldStepInfo.compute(currentStepIndex, (index, old) -> {
+ if (old == null) {
+ log.warn("Lost old step info for " + this + ", step " + index);
+ old = new OldStepRecord();
+ }
+ if (currentStepInstance.error==null) old.countCompleted++;
+ // okay if this gets picked up by accident because we will check the stepIndex it records against the currentStepIndex,
+ // and ignore it if different
+ old.context = currentStepInstance;
+ return old;
+ });
+ }
previousStepTaskId = currentStepInstance.taskId;
previousStepIndex = currentStepIndex;
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 4f80f5da71..94c2da19ee 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
@@ -68,7 +68,8 @@ public class WorkflowStepInstanceExecutionContext {
// replay instructions or a string explicit next step identifier
public Object next;
- /** set if the step is in an error handler context, containing the error being handled */
+ /** set if the step is in an error handler context, containing the error being handled,
+ * or if the step had an error that was not handled */
Throwable error;
/** set if there was an error handled locally */
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
index d95b053402..8afc16f79d 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowPersistReplayErrorsTest.java
@@ -19,6 +19,7 @@
package org.apache.brooklyn.core.workflow;
import com.google.common.base.Stopwatch;
+import com.google.common.collect.Iterables;
import org.apache.brooklyn.api.entity.EntityLocal;
import org.apache.brooklyn.api.entity.EntitySpec;
import org.apache.brooklyn.api.mgmt.ManagementContext;
@@ -56,6 +57,7 @@ import org.testng.annotations.Test;
import java.io.IOException;
import java.io.Serializable;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -1041,4 +1043,76 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl
// and worst case they can be manually deleted)
}
+ @Test
+ public void testErrorInSubWorkflowCaughtUpdatesContextAndStep() throws Exception {
+ app = mgmt().getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
+ WorkflowExecutionContext run = WorkflowBasicTest.runWorkflow(app, Strings.lines(
+ "steps:",
+ "- log 1",
+ "- type: workflow",
+ " steps:",
+ " - log 2-1",
+ " - step: fail message 2-2",
+ " on-error:",
+ " - log 2-2-error",
+ " - fail message 2-2-done",
+ " - log 2-3",
+ " on-error: ",
+ " - return 2-done",
+ "- log 3"
+ ), "test-error-in-subworkflow");
+ Asserts.assertEquals(run.getTask(true).get().getUnchecked(), "2-done");
+
+ WorkflowExecutionContext.OldStepRecord step2 = run.oldStepInfo.get(1);
+ Asserts.assertNotNull(step2);
+ Asserts.assertNotNull(step2.context);
+ Asserts.assertNull(step2.context.error); // should be null because handled
+ Asserts.assertNull(step2.context.errorHandlerTaskId); // should be null because not treated as a step handler, but handler for the workflow - step2sub.errorHandlerTaskId
+
+ BrooklynTaskTags.WorkflowTaskTag step2subTag = Iterables.getOnlyElement(step2.context.getSubWorkflows());
+
+ WorkflowExecutionContext step2sub = new WorkflowStatePersistenceViaSensors(mgmt()).getWorkflows(app).get(step2subTag.getWorkflowId());
+ Asserts.assertEquals(step2sub.getStatus(), WorkflowExecutionContext.WorkflowStatus.SUCCESS);
+ Asserts.assertNotNull(step2sub.errorHandlerTaskId);
+
+ WorkflowExecutionContext.OldStepRecord step22 = step2sub.oldStepInfo.get(1);
+ Asserts.assertNotNull(step22);
+
+ Asserts.assertNotNull(step22);
+ Asserts.assertNotNull(step22.context);
+ Asserts.assertNotNull(step22.context.error); // not null because not handled here
+ Asserts.assertNotNull(step22.context.errorHandlerTaskId);
+ }
+
+ @Test
+ public void testErrorInSubWorkflowUncaughtUpdatesContextAndStep() throws Exception {
+ app = mgmt().getEntityManager().createEntity(EntitySpec.create(BasicApplication.class));
+ WorkflowExecutionContext run = WorkflowBasicTest.runWorkflow(app, Strings.lines(
+ "steps:",
+ "- log 1",
+ "- type: workflow",
+ " steps:",
+ " - log 2-1",
+ " - step: fail message 2-2",
+ " on-error:",
+ " - log 2-2-error",
+ " - fail message 2-2-done",
+ " - log 2-3",
+ "- log 3"
+ ), "test-error-in-subworkflow");
+ run.getTask(true).get().blockUntilEnded();
+ Asserts.assertEquals(run.getStatus(), WorkflowExecutionContext.WorkflowStatus.ERROR);
+
+ WorkflowExecutionContext.OldStepRecord step2 = run.oldStepInfo.get(1);
+ BrooklynTaskTags.WorkflowTaskTag step2subTag = Iterables.getOnlyElement(step2.context.getSubWorkflows());
+
+ WorkflowExecutionContext step2sub = new WorkflowStatePersistenceViaSensors(mgmt()).getWorkflows(app).get(step2subTag.getWorkflowId());
+ Asserts.assertEquals(step2sub.getStatus(), WorkflowExecutionContext.WorkflowStatus.ERROR);
+
+ WorkflowExecutionContext.OldStepRecord step22 = step2sub.oldStepInfo.get(1);
+ Asserts.assertNotNull(step22);
+ Asserts.assertNotNull(step22.context);
+ Asserts.assertNotNull(step22.context.error);
+ Asserts.assertNotNull(step22.context.errorHandlerTaskId);
+ }
}
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 ef3b500f36..03686deb82 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
@@ -337,6 +337,7 @@ public class WorkflowRetryTest extends RebindTestFixture<BasicApplication> {
while (lastInvocation==null) Time.sleep(Duration.millis(10));
EntityAsserts.assertAttributeEventually(app, Sensors.newIntegerSensor("count"), v -> v!=null && v > 1);
Asserts.assertFalse(lastInvocation.isDone());
+ log.info("setting sensor so workflow can proceed");
app.sensors().set(Sensors.newIntegerSensor("no_count"), -1);
lastInvocation.getUnchecked(Duration.ONE_SECOND);
EntityAsserts.assertAttributeEquals(app, Sensors.newIntegerSensor("no_count"), 0);
[brooklyn-server] 10/10: comment or fix places that need deeply
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 7d5c247e82f658bdca39146def7a1348a6725f9a
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Tue Jun 20 02:45:39 2023 +0100
comment or fix places that need deeply
---
.../core/resolve/jackson/BeanWithTypeUtils.java | 2 ++
.../workflow/WorkflowExpressionResolution.java | 27 +++++++++++-----------
.../steps/variables/SetVariableWorkflowStep.java | 16 +++++++++----
.../brooklyn/util/core/flags/TypeCoercions.java | 1 +
...klynRegisteredTypeJacksonSerializationTest.java | 7 ++++--
.../core/workflow/WorkflowTransformTest.java | 4 ++--
6 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
index a920995cac..91835f23ee 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/BeanWithTypeUtils.java
@@ -190,6 +190,8 @@ public class BeanWithTypeUtils {
} catch (Exception e) {
try {
+ // needed for a few things, mainly where a bean has a type field that conflicts with the type here,
+ // tryCoercer 81-wrong-bean uses this
return convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes);
} catch (Exception e2) {
throw Exceptions.propagate(Arrays.asList(e, e2));
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 5c207c8b6c..a18b78d8bd 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
@@ -388,24 +388,25 @@ public class WorkflowExpressionResolution {
/** does not use templates */
public <T> T resolveCoercingOnly(Object expression, TypeToken<T> type) {
- try {
- if (expression==null || (Jsonya.isJsonPrimitiveDeep(expression) && !(expression instanceof Set))) {
+ if (expression==null) return null;
+ if (Jsonya.isJsonPrimitiveDeep(expression) && !(expression instanceof Set)) {
+ try {
// only try yaml coercion, as values are normally set from yaml and will be raw at this stage (but not if they are from a DSL)
// (might be better to always to TC.coerce)
return BeanWithTypeUtils.convert(context.getManagementContext(), expression, type, true,
RegisteredTypes.getClassLoadingContext(context.getEntity()), true /* needed for wrapped resolved holders */);
- } else {
- return TypeCoercions.coerce(expression, type);
- }
- } catch (Exception e) {
- Exceptions.propagateIfFatal(e);
- try {
- // fallback to simple coercion
- return TypeCoercions.coerce(expression, type);
- } catch (Exception e2) {
- Exceptions.propagateIfFatal(e2);
- throw Exceptions.propagate(e);
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ try {
+ // fallback to simple coercion
+ return TypeCoercions.coerce(expression, type);
+ } catch (Exception e2) {
+ Exceptions.propagateIfFatal(e2);
+ throw Exceptions.propagate(e);
+ }
}
+ } else {
+ return TypeCoercions.coerce(expression, type);
}
}
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 2cb1bfd108..fb2d2ebfa1 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
@@ -241,6 +241,10 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
}
Object process(List<String> w) {
+ return process(w, false);
+ }
+
+ Object process(List<String> w, boolean ternaryColonAllowed) {
// if no tokens, treat as null
if (w.isEmpty()) return null;
@@ -251,10 +255,12 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
// not used: ! ~ - + & ++ -- (i.e. unary operators)
// #__: ?: (i.e. ternary)
- result = handleTokenIfPresent(w, false, MutableMap.of(
- "?", this::handleTernaryCondition,
- ":", this::handleTernaryArms
- ));
+ result = handleTokenIfPresent(w, false,
+ ternaryColonAllowed ? MutableMap.of(
+ "?", this::handleTernaryCondition,
+ ":", this::handleTernaryArms)
+ : MutableMap.of("?", this::handleTernaryCondition)
+ );
if (result.isPresent()) return result.get();
@@ -512,7 +518,7 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
rhs = handleChainedTernaryRhs(rhs0);
} else {
// non-nested or chained
- rhs = process(rhs0);
+ rhs = process(rhs0, true);
}
//log.info(String.format("Ternary Condition 1: [lhs:%s][rhs:%s]", lhs, rhs));
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
index d630819a4a..ed2e66f165 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/flags/TypeCoercions.java
@@ -390,6 +390,7 @@ public class TypeCoercions {
return null;
}
try {
+ // if a bean has a type field, we might need to try an explicit conversion to it
Maybe<Map> resultMap = BeanWithTypeUtils.tryConvertOrAbsentUsingContext(Maybe.of(input), new TypeToken<Map>() {}, true);
if (toMap || resultMap.isAbsentOrNull()) return (Maybe<T>) resultMap;
return BeanWithTypeUtils.tryConvertOrAbsentUsingContext(Maybe.cast(resultMap), type);
diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java
index dc9a429904..840371893b 100644
--- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynRegisteredTypeJacksonSerializationTest.java
@@ -223,8 +223,11 @@ public class BrooklynRegisteredTypeJacksonSerializationTest extends BrooklynMgmt
// we have no choice but to fallback to map deserialization
// however we should allow further coercion to Map (in case we read as typed something which should have been a map)
// and also coercion that serializes input if complex type then deserializes to intended type, if the intended type has a field 'type'
- Map redeserMap = TypeCoercions.coerce(deser(deser), Map.class);
- Asserts.assertEquals(redeserMap.get("type"), OtherBean.class.getName());
+
+ // coercing to a map doesn't mean serializing it, that's too silly
+// Map redeserMap = TypeCoercions.coerce(deser(deser), Map.class);
+// Asserts.assertEquals(redeserMap.get("type"), OtherBean.class.getName());
+
SampleBeanWithType redeserObj = TypeCoercions.coerce(deser(deser), SampleBeanWithType.class);
Asserts.assertEquals(redeserObj.x, "hello");
}
diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
index 9f90a6dfcd..9d102cf6f6 100644
--- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowTransformTest.java
@@ -125,9 +125,9 @@ public class WorkflowTransformTest extends BrooklynMgmtUnitTestSupport {
public void testMapDirect() {
Asserts.assertEquals(runWorkflowSteps(
"let map myMap = {a: 1}",
- "let myMap.a = 2",
+ "let myMap.a = ${myMap.a} + 2",
"return ${myMap.a}"),
- "2");
+ 3);
}
@Test