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
The following commit(s) were added to refs/heads/master by this push: new 1d9bb2ec0f improve step parser 1d9bb2ec0f is described below commit 1d9bb2ec0f808eb30ed2eb8fcb34843120d9d389 Author: Alex Heneveld <a...@cloudsoft.io> AuthorDate: Wed Feb 21 15:47:32 2024 +0000 improve step parser more forgiving in some cases, and better errors when cannot forgive, including if value omitted from sensors/config (eg if you say set-sensor x=1) --- .../brooklyn/core/workflow/WorkflowStepResolution.java | 18 ++++++++++++++++-- .../workflow/steps/appmodel/SetConfigWorkflowStep.java | 2 ++ .../workflow/steps/appmodel/SetSensorWorkflowStep.java | 2 ++ .../steps/variables/SetVariableWorkflowStep.java | 1 + 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java index a41eb524b2..8526483ed8 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowStepResolution.java @@ -33,6 +33,7 @@ import org.apache.brooklyn.core.objs.BrooklynObjectInternal; import org.apache.brooklyn.core.resolve.jackson.BeanWithTypeUtils; import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.core.workflow.steps.CustomWorkflowStep; +import org.apache.brooklyn.core.workflow.steps.flow.SubWorkflowStep; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.config.ConfigBag; @@ -88,6 +89,12 @@ public class WorkflowStepResolution { String shorthand = null; Map defM = null; + + if (def instanceof List) { + // list treated as implicit subworkflow, eg step: [ "sleep 1" ] = step: { steps: [ "sleep 1" ] } + def = MutableMap.of(SubWorkflowStep.SHORTHAND_TYPE_NAME_DEFAULT, def); + } + if (def instanceof String) { shorthand = (String) def; defM = MutableMap.of(); @@ -98,10 +105,12 @@ public class WorkflowStepResolution { Object s = defM.remove("step"); if (s == null) s = defM.remove("shorthand"); if (s == null) s = defM.remove("s"); - if (s==null && defM.containsKey("steps")) { + + if (s==null && defM.containsKey(WorkflowCommonConfig.STEPS.getName())) { // if it has steps, but no step or s, assume it is a subworkflow - s = "subworkflow"; + s = SubWorkflowStep.SHORTHAND_TYPE_NAME_DEFAULT; } + if (s == null && defM.size()==1) { // assume the colon caused it accidentally to be a map s = Iterables.getOnlyElement(defM.keySet()); @@ -111,9 +120,14 @@ public class WorkflowStepResolution { s = null; } } + if (s==null) { throw new IllegalArgumentException("Step definition must indicate a `type` or a `step` / `shorthand` / `s` (" + def + ")"); } + if (s instanceof Map && defM.size()==1) { + // allow shorthand to contain a nested map if the shorthand is the only thing in the map, eg { step: { step: "xxx" } } + return resolveStep(mgmt, s); + } if (!(s instanceof String)) { throw new IllegalArgumentException("step shorthand must be a string"); } 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 31c4ddb3fd..b1b788fa76 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 @@ -62,6 +62,8 @@ public class SetConfigWorkflowStep extends WorkflowStepDefinition { Object entityO2 = ObjectUtils.firstNonNull(config.entity, entityO1, context.getEntity()); final Entity entity = WorkflowStepResolution.findEntity(context, entityO2).get(); + if (!context.hasInput(VALUE)) throw new IllegalArgumentException("Value is required"); + Pair<Object, Object> oldValues = WorkflowSettingItemsUtils.setAtIndex(nameAndIndices, true, (_oldValue) -> resolvedValue, name -> entity.config().get((ConfigKey<Object>) ConfigKeys.newConfigKey(type, name)), (name, value) -> entity.config().set((ConfigKey<Object>) ConfigKeys.newConfigKey(type, name), value)); 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 dd570a1af6..6dcff9b435 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 @@ -96,6 +96,8 @@ public class SetSensorWorkflowStep extends WorkflowStepDefinition { Object entityO2 = ObjectUtils.firstNonNull(sensor.entity, entityO1, context.getEntity()); final Entity entity = WorkflowStepResolution.findEntity(context, entityO2).get(); + if (!context.hasInput(VALUE)) throw new IllegalArgumentException("Value is required"); + Supplier<Object> resolveOnceValueSupplier = Suppliers.memoize(() -> { // // might need to be careful if defined type is different or more generic than type specified here; // // but that should be fine as XML persistence preserves types 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 2aad2bbeeb..508d885dfc 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 @@ -102,6 +102,7 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition { if (Strings.isBlank(name)) throw new IllegalArgumentException("Variable name is required"); TypeToken<?> type = context.lookupType(variable.type, () -> null); + if (!context.hasInput(VALUE)) throw new IllegalArgumentException("Value is required"); Object unresolvedValue = input.get(VALUE.getName()); Object resolvedValue = new ConfigurableInterpolationEvaluation(context, type, unresolvedValue, context.getInputOrDefault(INTERPOLATION_MODE), context.getInputOrDefault(INTERPOLATION_ERRORS)).evaluate();