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 b47cbd8107454373ca18002f1697b05a03fdeb52 Author: Alex Heneveld <[email protected]> AuthorDate: Thu Mar 21 13:48:25 2024 +0000 better checking for string/primitive, and better error for workflow input --- .../main/java/org/apache/brooklyn/core/config/Sanitizer.java | 2 +- .../core/mgmt/internal/AbstractManagementContext.java | 2 +- .../brooklyn/core/resolve/jackson/BeanWithTypeUtils.java | 10 ++++------ .../core/workflow/WorkflowStepInstanceExecutionContext.java | 11 +++++++++-- .../core/workflow/steps/external/HttpWorkflowStep.java | 5 ++++- .../brooklyn/core/workflow/steps/variables/TransformJoin.java | 4 ++-- .../core/workflow/steps/variables/TransformPrependAppend.java | 4 ++-- .../org/apache/brooklyn/util/core/flags/TypeCoercions.java | 2 +- .../core/workflow/WorkflowPersistReplayErrorsTest.java | 2 +- .../apache/brooklyn/tasks/kubectl/ContainerWorkflowStep.java | 3 +-- .../java/org/apache/brooklyn/util/collections/Jsonya.java | 5 ++++- .../main/java/org/apache/brooklyn/util/javalang/Boxing.java | 3 +++ 12 files changed, 33 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java b/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java index 4e7fa7b614..e79c220c52 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java @@ -176,7 +176,7 @@ public final class Sanitizer { } public static String suppressJson(Object value, boolean excludeBrooklynDslExpressions) { - if (value==null || Boxing.isPrimitiveOrBoxedObject(value) || value instanceof CharSequence) { + if (value==null || Boxing.isPrimitiveOrStringOrBoxedObject(value) || value instanceof CharSequence) { if (excludeBrooklynDslExpressions && value instanceof String && ((String)value).startsWith("$brooklyn:")) return (String)value; return suppress(value); } diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java index 722f6040c5..7d569dc146 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java @@ -566,7 +566,7 @@ public abstract class AbstractManagementContext implements ManagementContextInte String entityId; if (entityO instanceof Entity) { entity = (Entity) entityO; - } else if (entityO instanceof String || Boxing.isPrimitiveOrBoxedObject(entityO)) { + } else if (Boxing.isPrimitiveOrStringOrBoxedObject(entityO)) { entityId = entityO.toString(); List<Entity> firstGroupOfMatches = AppGroupTraverser.findFirstGroupOfMatches(contextEntity, true, 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 9b2a6dde2b..2c85657c93 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 @@ -120,9 +120,7 @@ public class BeanWithTypeUtils { return isJsonAndOthers(o, oo -> false); } public static boolean isJsonAndOthers(Object o, Predicate<Object> acceptOthers) { - if (o instanceof String) return true; - if (o==null) return true; - if (Boxing.isPrimitiveOrBoxedObject(o)) return true; + if (o==null || Boxing.isPrimitiveOrStringOrBoxedObject(o)) return true; if (o instanceof Collection) { for (Object oo : (Collection<?>) o) { if (!isJsonAndOthers(oo, acceptOthers)) return false; @@ -222,7 +220,7 @@ public class BeanWithTypeUtils { boolean useLonghandObjectWriter = true; if (type.getRawType().equals(Object.class)) useLonghandObjectWriter = false; - else if (mapOrListToSerializeThenDeserialize==null || mapOrListToSerializeThenDeserialize instanceof String || Boxing.isPrimitiveOrBoxedObject(mapOrListToSerializeThenDeserialize)) useLonghandObjectWriter = false; + else if (mapOrListToSerializeThenDeserialize==null || Boxing.isPrimitiveOrStringOrBoxedObject(mapOrListToSerializeThenDeserialize)) useLonghandObjectWriter = false; String serialization = !useLonghandObjectWriter ? mapper.writeValueAsString(mapOrListToSerializeThenDeserialize) : mapper.writerFor(Object.class).writeValueAsString(mapOrListToSerializeThenDeserialize); return mapper.readValue(serialization, BrooklynJacksonType.asJavaType(mapper, type)); @@ -245,7 +243,7 @@ public class BeanWithTypeUtils { if (inputMap.isAbsent()) return (Maybe<T>)inputMap; Object o = inputMap.get(); - if (!(o instanceof Map) && !(o instanceof List) && !Boxing.isPrimitiveOrBoxedObject(o) && !(o instanceof String)) { + if (!(o instanceof Map) && !(o instanceof List) && !Boxing.isPrimitiveOrStringOrBoxedObject(o)) { if (type.isSupertypeOf(o.getClass())) { return (Maybe<T>)inputMap; } else { @@ -337,6 +335,6 @@ public class BeanWithTypeUtils { } // we want some special object. if we have a map or a string or possibly a primitive then conversion might sort us out. - return (t instanceof Map || t instanceof String || Boxing.isPrimitiveOrBoxedObject(t)) ? 1 : -1; + return (t instanceof Map || Boxing.isPrimitiveOrStringOrBoxedObject(t)) ? 1 : -1; } } 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 e1c6df8316..3ede6cc447 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 @@ -33,6 +33,7 @@ import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.exceptions.LossySerializingThrowable; +import org.apache.brooklyn.util.javalang.Boxing; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -179,8 +180,14 @@ public class WorkflowStepInstanceExecutionContext { if (inputResolved.containsKey(key)) return (T)inputResolved.get(key); Object v = input.get(key); - T v2 = WorkflowExpressionResolution.allowingRecursionWhenSetting(context, WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, key, - () -> context.resolve(stage, v, type)); + T v2; + try { + v2 = WorkflowExpressionResolution.allowingRecursionWhenSetting(context, WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, key, + () -> context.resolve(stage, v, type)); + } catch (Exception e) { + throw Exceptions.propagateAnnotated("Cannot resolve input "+ + (Boxing.isPrimitiveOrStringOrBoxedObject(v) ? "'"+v+"'" : v.getClass().getName() + " ("+v+")"), e); + } if (REMEMBER_RESOLVED_INPUT) { if (!Objects.equals(v, v2)) { inputResolved.put(key, v2); 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 b262f1efb2..fa47370772 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 @@ -46,6 +46,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; +import java.net.URI; import java.net.URISyntaxException; import java.nio.charset.Charset; import java.util.Map; @@ -103,7 +104,9 @@ public class HttpWorkflowStep extends WorkflowStepDefinition { new ShellEnvironmentSerializer(context.getManagementContext()).serialize(params) .forEach((k, v) -> urib.addParameter(k, v)); } - httpb.uri(urib.build()); + URI uri = urib.build(); + httpb.uri(uri); + context.noteOtherMetadata("URL", uri.toString()); } catch (URISyntaxException e) { throw Exceptions.propagateAnnotated("Invalid URI: "+endpoint, e); } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformJoin.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformJoin.java index b14e8b77d9..885e45185b 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformJoin.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformJoin.java @@ -40,14 +40,14 @@ public class TransformJoin extends WorkflowTransformDefault { @Override public Object apply(Object v) { Object separatorResolvedO = separator==null ? "" : context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, separator, Object.class); - if (!(separatorResolvedO instanceof String || Boxing.isPrimitiveOrBoxedObject(separatorResolvedO))) { + if (!(Boxing.isPrimitiveOrStringOrBoxedObject(separatorResolvedO))) { throw new IllegalStateException("Argument must be a string or primitive to use as the separator"); } String separatorResolved = ""+separatorResolvedO; if (v instanceof Iterable) { List list = MutableList.copyOf((Iterable)v); return list.stream().map(x -> { - if (!(x instanceof String || Boxing.isPrimitiveOrBoxedObject(x))) { + if (!(Boxing.isPrimitiveOrStringOrBoxedObject(x))) { throw new IllegalStateException("Elements in the list to join must be a strings or primitives"); } return ""+x; diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformPrependAppend.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformPrependAppend.java index 5c06ee9e58..522c750f58 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformPrependAppend.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformPrependAppend.java @@ -44,8 +44,8 @@ public class TransformPrependAppend extends WorkflowTransformDefault { @Override public Object apply(Object v) { Object expressionResolved = context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, expression, Object.class); - if (v instanceof String || Boxing.isPrimitiveOrBoxedObject(v)) { - if (!(expressionResolved instanceof String || Boxing.isPrimitiveOrBoxedObject(expressionResolved))) { + if (Boxing.isPrimitiveOrStringOrBoxedObject(v)) { + if (!(Boxing.isPrimitiveOrStringOrBoxedObject(expressionResolved))) { throw new IllegalStateException("Argument must be a string or primitive to prepend/append"); } if (atEnd) return Strings.toString(v) + expressionResolved; 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 b93ac1b986..3840c0b838 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 @@ -361,7 +361,7 @@ public class TypeCoercions { @Override public <T> Maybe<T> tryCoerce(Object input, TypeToken<T> type) { - if (!(input instanceof Map || input instanceof Collection || Boxing.isPrimitiveOrBoxedObject(input) || input instanceof String)) { + if (!(input instanceof Map || input instanceof Collection || Boxing.isPrimitiveOrStringOrBoxedObject(input))) { return null; } if (BeanWithTypeUtils.isConversionRecommended(Maybe.of(input), type)) { 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 73e242de5f..32a0328970 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 @@ -742,7 +742,7 @@ public class WorkflowPersistReplayErrorsTest extends RebindTestFixture<BasicAppl List<String> msgs = logWatcher.getMessages(); log.info("Error handler output:\n"+msgs.stream().collect(Collectors.joining("\n"))); Asserts.assertEquals(msgs.stream().filter(s -> s.contains("NOT")).findAny().orElse(null), null); - Asserts.assertEquals(msgs.stream().filter(s -> s.contains("created-but-not-logged") && !s.contains("Creating handler")).findAny().orElse(null), null); + Asserts.assertEquals(msgs.stream().filter(s -> s.contains("created-but-not-logged") && !s.contains("error-handler")).findAny().orElse(null), null); Asserts.assertNotNull(msgs.stream().filter(s -> s.contains("1-1")).findAny().orElse(null)); Asserts.assertNotNull(msgs.stream().filter(s -> s.contains("1-2")).findAny().orElse(null)); Asserts.assertNotNull(msgs.stream().filter(s -> s.contains("1-4")).findAny().orElse(null)); 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 1d5938f4ab..56299751fe 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 @@ -103,8 +103,7 @@ public class ContainerWorkflowStep extends WorkflowStepDefinition { } else if (args instanceof Collection) { List<String> result = MutableList.of(); ((Collection)args).forEach(x -> { - if (x instanceof String) result.add((String)x); - else if (Boxing.isPrimitiveOrBoxedObject(x)) result.add(x.toString()); + if (Boxing.isPrimitiveOrStringOrBoxedObject(x)) result.add(x.toString()); else throw new IllegalArgumentException("Argument '"+x+"' not supported; should be a string"); }); tf.arguments(result); diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/collections/Jsonya.java b/utils/common/src/main/java/org/apache/brooklyn/util/collections/Jsonya.java index b79d5ca0ae..e3a84f033d 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/collections/Jsonya.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/collections/Jsonya.java @@ -114,9 +114,12 @@ public class Jsonya { if (x==null) return true; return x instanceof Map || x instanceof Collection || x instanceof String || Boxing.isPrimitiveOrBoxedObject(x); } + public static boolean isJsonPrimitive(Object x) { + return Boxing.isPrimitiveOrStringOrBoxedObject(x); + } public static boolean isJsonPrimitiveDeep(Object x) { if (x==null) return true; - if (x instanceof String || Boxing.isPrimitiveOrBoxedObject(x)) return true; + if (isJsonPrimitive(x)) return true; if (x instanceof Map) return !((Map<?,?>)x).entrySet().stream().anyMatch(ent -> !isJsonPrimitiveDeep(ent.getKey()) || !isJsonPrimitiveDeep(ent.getValue())); if (x instanceof Collection) return !((Collection)x).stream().anyMatch(ent -> !isJsonPrimitiveDeep(ent)); return false; diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/Boxing.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/Boxing.java index 7079fc2167..eda50850c0 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/Boxing.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/Boxing.java @@ -93,6 +93,9 @@ public class Boxing { if (o==null) return false; return isPrimitiveOrBoxedClass(o.getClass()); } + public static boolean isPrimitiveOrStringOrBoxedObject(Object o) { + return o instanceof String || isPrimitiveOrBoxedObject(o); + } public static boolean isPrimitiveOrBoxedClass(Class<?> t) { // TODO maybe better to switch to using Primitives, eg: // return Primitives.allPrimitiveTypes().contains(type) || Primitives.allWrapperTypes().contains(type);
