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/05/26 13:19:05 UTC
[brooklyn-server] branch master updated (d27d1720eb -> 8bdc96f732)
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 d27d1720eb add 'slice' and 'remove' transforms to workflow
new 1d2f6bf3c9 allow use of overridden condition
new 216b485589 more workflow math for time
new dd89910600 stricter coercion to maps
new 4660a30ce0 permit jackson to be used to coerce strings and primitives
new f35185c466 don't use longhand object writer when given a primitive or string input
new 70b08ce04b return all errors from type coercion
new 0096fe41a6 don't accept jackson conversion of string to Predicate, and don't expand primitives when converting
new 8bdc96f732 Merge branch 'misc-fixes'
The 8 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:
.../core/resolve/jackson/BeanWithTypeUtils.java | 33 ++++--
.../resolve/jackson/CommonTypesSerialization.java | 16 +--
.../JacksonBetterDelegatingDeserializer.java | 17 +++
.../jackson/JsonSymbolDependentDeserializer.java | 7 +-
.../core/workflow/WorkflowExecutionContext.java | 5 +-
.../steps/variables/SetVariableWorkflowStep.java | 29 ++++-
.../workflow/steps/variables/TransformMerge.java | 2 +
.../brooklyn/util/core/flags/TypeCoercions.java | 2 +-
.../util/core/predicates/DslPredicates.java | 30 +++--
.../BrooklynMiscJacksonSerializationTest.java | 132 ++++++++++++++++++---
.../util/core/internal/TypeCoercionsTest.java | 100 ++++++++++------
.../util/core/predicates/DslPredicateTest.java | 10 +-
.../coerce/CommonAdaptorTypeCoercions.java | 15 ++-
.../javalang/coerce/TypeCoercerExtensible.java | 34 +++---
.../util/javalang/coerce/TypeCoercionsTest.java | 35 +-----
15 files changed, 325 insertions(+), 142 deletions(-)
[brooklyn-server] 07/08: don't accept jackson conversion of string to Predicate, and don't expand primitives when converting
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 0096fe41a649e5b75d13bda82ca876a1bb3a10e1
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri May 26 12:51:18 2023 +0100
don't accept jackson conversion of string to Predicate, and don't expand primitives when converting
do still allow strings for DslPredicate; but since now we allow strings to convert via jackson more often,
we need to guard against places they go in to basic Predicates and short-circuit other conversion
(eg constraints like nonNull)
related, when given a long, we want to leave it as the long, not expand it to be ["Long", value], as that is
deliberately harder to convert and we rightly i think don't accept that
---
.../resolve/jackson/CommonTypesSerialization.java | 16 +++++-------
.../JacksonBetterDelegatingDeserializer.java | 17 ++++++++++++
.../jackson/JsonSymbolDependentDeserializer.java | 7 ++++-
.../util/core/predicates/DslPredicates.java | 30 ++++++++++++++++------
.../BrooklynMiscJacksonSerializationTest.java | 24 ++++++++++++-----
.../util/core/predicates/DslPredicateTest.java | 16 ++++++++----
6 files changed, 79 insertions(+), 31 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/CommonTypesSerialization.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/CommonTypesSerialization.java
index 679bddec6f..53b7e4bc53 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/CommonTypesSerialization.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/CommonTypesSerialization.java
@@ -30,21 +30,15 @@ import com.fasterxml.jackson.databind.module.SimpleDeserializers;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.databind.ser.BeanPropertyWriter;
import com.fasterxml.jackson.databind.ser.BeanSerializerModifier;
-import com.google.common.base.Predicate;
import com.google.common.reflect.TypeToken;
-import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec;
import org.apache.brooklyn.api.mgmt.ManagementContext;
import org.apache.brooklyn.api.objs.BrooklynObject;
import org.apache.brooklyn.api.objs.BrooklynObjectType;
-import org.apache.brooklyn.api.objs.Configurable;
import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
-import org.apache.brooklyn.core.objs.BrooklynObjectInternal;
-import org.apache.brooklyn.core.typereg.TypePlanTransformers;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.core.flags.BrooklynTypeNameResolution;
import org.apache.brooklyn.util.core.flags.FlagUtils;
-import org.apache.brooklyn.util.core.flags.TypeCoercions;
import org.apache.brooklyn.util.core.predicates.DslPredicates;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.javalang.Boxing;
@@ -225,6 +219,10 @@ public class CommonTypesSerialization {
}
public T deserializeOther(Object value) {
+ if (getType()!=null && getType().isInstance(value))
+ return (T) value; //if using complex types (convertShallow) allow if castable
+ // we could attempt coercion, but no need to go that far, and readObject in deserialize above would probably fail also;
+ // we assume if you have a complex object it is already the right type, or you use coerce
throw new IllegalStateException(getType()+" should be supplied as string or map with 'type' and 'value'; instead had " + value);
}
@@ -471,10 +469,8 @@ public class CommonTypesSerialization {
private final ManagementContext mgmt;
public PredicateSerialization(ManagementContext mgmt) { this.mgmt = mgmt; }
public void apply(SimpleModule m) {
- m.addDeserializer(Predicate.class, (JsonDeserializer) new DslPredicates.DslPredicateJsonDeserializer());
- m.addDeserializer(java.util.function.Predicate.class, (JsonDeserializer) new DslPredicates.DslPredicateJsonDeserializer());
- m.addDeserializer(DslPredicates.DslPredicate.class, (JsonDeserializer) new DslPredicates.DslPredicateJsonDeserializer());
- m.addDeserializer(DslPredicates.DslEntityPredicate.class, (JsonDeserializer) new DslPredicates.DslPredicateJsonDeserializer());
+ DslPredicates.DslPredicateJsonDeserializer.DSL_REGISTERED_CLASSES.forEach(t ->
+ m.addDeserializer(t, new DslPredicates.DslPredicateJsonDeserializer()) );
}
}
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JacksonBetterDelegatingDeserializer.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JacksonBetterDelegatingDeserializer.java
index 611bee0f1b..dd6cc5ab0b 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JacksonBetterDelegatingDeserializer.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JacksonBetterDelegatingDeserializer.java
@@ -167,4 +167,21 @@ public abstract class JacksonBetterDelegatingDeserializer extends DelegatingDese
protected abstract Object deserializeWrapper(JsonParser jp, DeserializationContext ctxt, BiFunctionThrowsIoException<JsonParser, DeserializationContext, Object> nestedDeserialize) throws IOException;
+ @Override
+ public JsonDeserializer<?> createContextual(DeserializationContext ctxt,
+ BeanProperty property)
+ throws JsonMappingException
+ {
+ JavaType vt = ctxt.constructType(_delegatee.handledType());
+
+ //override parent to make this available
+ if (vt==null) vt = ctxt.getContextualType();
+
+ JsonDeserializer<?> del = ctxt.handleSecondaryContextualization(_delegatee,
+ property, vt);
+ if (del == _delegatee) {
+ return this;
+ }
+ return newDelegatingInstance(del);
+ }
}
diff --git a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonSymbolDependentDeserializer.java b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonSymbolDependentDeserializer.java
index 8a6b36b4c4..93d8b7f9d1 100644
--- a/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonSymbolDependentDeserializer.java
+++ b/core/src/main/java/org/apache/brooklyn/core/resolve/jackson/JsonSymbolDependentDeserializer.java
@@ -62,7 +62,10 @@ public abstract class JsonSymbolDependentDeserializer extends JsonDeserializer<O
type = property.getType();
}
if (type==null) {
- // this is normally set during contextualization but not during deserialization (although not if we're the ones contextualizing it)
+ // ctxt.getContextualType() is normally set during primary contextualization and first round of secondary (if known)
+ // but not usually available during deserialization, so do it now;
+ // however it can be suppressed if in a nested secondary contextualization (eg via DelegatingDeserializer),
+ // but our JacksonBetterDelegatingDeserializer attempts to avoid this
type = ctxt.getContextualType();
}
if (isTypeReplaceableByDefault()) {
@@ -74,6 +77,7 @@ public abstract class JsonSymbolDependentDeserializer extends JsonDeserializer<O
protected boolean isTypeReplaceableByDefault() {
if (type==null) return true;
+ if (type.getRawClass().isInterface()) return true;
return false;
}
@@ -139,6 +143,7 @@ public abstract class JsonSymbolDependentDeserializer extends JsonDeserializer<O
return getObjectDeserializer();
}
+ /** deserializes if we know we have an object; if we have a string, it will typically go into deserializeToken */
protected Object deserializeObject(JsonParser p) throws IOException, JsonProcessingException {
return contextualize(getObjectDeserializer()).deserialize(p, ctxt);
}
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 036a65b1aa..8ed0c3533f 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
@@ -677,9 +677,12 @@ public class DslPredicates {
public static class DslPredicateDefault<T2> extends DslPredicateBase<T2> implements DslPredicate<T2>, Cloneable {
public DslPredicateDefault() {}
- // allow a string or int to be an implicit equality target
+ // 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
// not used by code, but allows clients to store other information
public Object metadata;
@@ -887,18 +890,28 @@ public class DslPredicates {
@Beta
public static class DslPredicateJsonDeserializer extends JsonSymbolDependentDeserializer {
- public static final Set<Class> DSL_SUPPORTED_CLASSES = ImmutableSet.<Class>of(
- java.util.function.Predicate.class, Predicate.class,
- DslPredicate.class, DslEntityPredicate.class);
+ public static final Set<Class> DSL_REGISTERED_CLASSES = ImmutableSet.<Class>of(
+ java.util.function.Predicate.class, Predicate.class,
+ DslPredicate.class, DslEntityPredicate.class);
+ public static final Set<Class> DSL_RESTRICTED_CLASSES = ImmutableSet.<Class>of(
+ java.util.function.Predicate.class, Predicate.class);
public JsonDeserializer<?> createContextual(DeserializationContext ctxt, BeanProperty property) throws JsonMappingException {
return super.createContextual(ctxt, property);
}
protected boolean isTypeReplaceableByDefault() {
- if (super.isTypeReplaceableByDefault()) return true;
- if (type.getRawClass().isInterface()) return true;
- return false;
+ if (type!=null && DSL_RESTRICTED_CLASSES.contains(type.getRawClass())) {
+ // don't allow setting our default type if one of these restricted classes is supplied;
+ // if we have a map, then we'll still always come to our deserializeObject, with the requested type;
+ // but if we have a string or other simple token, will never come to our deserializeObject,
+ // instead it goes to deserializeToken (in superclass) which uses bean deserializer ie constructor,
+ // which will only work if we replace the type with our getDefaultType(), and we only want string
+ // implicitEquals to work if a DslPredicate was specified, not for any predicate (as those might be
+ // for constraints, like "nonNull")
+ return false;
+ }
+ return super.isTypeReplaceableByDefault();
}
@Override
@@ -908,6 +921,7 @@ public class DslPredicates {
@Override
protected Object deserializeObject(JsonParser p0) throws IOException {
+
MutableList<Throwable> errors = MutableList.of();
TokenBuffer pb = BrooklynJacksonSerializationUtils.createBufferForParserCurrentObject(p0, ctxt);
@@ -915,7 +929,7 @@ public class DslPredicates {
Object raw = null;
JsonDeserializer<?> deser;
- if (DSL_SUPPORTED_CLASSES.contains(type.getRawClass())) {
+ if (DSL_REGISTERED_CLASSES.contains(type.getRawClass())) {
// just load a map/string, then handle it
deser = ctxt.findRootValueDeserializer(ctxt.constructType(Object.class));
raw = deser.deserialize(pb.asParserOnFirstToken(), ctxt);
diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java
index 0db76c0ea6..c25b3730b8 100644
--- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java
@@ -30,11 +30,7 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLMapper;
import java.io.IOException;
import java.time.Instant;
-import java.util.Date;
-import java.util.GregorianCalendar;
-import java.util.Locale;
-import java.util.Map;
-import java.util.TimeZone;
+import java.util.*;
import java.util.function.BiConsumer;
import com.google.common.reflect.TypeToken;
@@ -205,16 +201,30 @@ public class BrooklynMiscJacksonSerializationTest implements MapperTestFixture {
}
@Test
- public void testInstantFromLong() throws Exception {
+ public void testInstantConversionFromVarious() throws Exception {
mapper = BeanWithTypeUtils.newYamlMapper(null, false, null, true);
long utc = new Date().getTime();
- mapper.readerFor(Instant.class).readValue( mapper.writeValueAsString(utc) );
+ Instant inst = mapper.readerFor(Instant.class).readValue( mapper.writeValueAsString(utc) );
// below known not to work, as long is converted to ["j...Long", utc] which we don't process
//mapper.readerFor(Instant.class).readValue( mapper.writerFor(Object.class).writeValueAsString(utc) );
ManagementContext mgmt = LocalManagementContextForTests.newInstance();
+
BeanWithTypeUtils.convertShallow(mgmt, utc, TypeToken.of(Instant.class), false, null, false);
BeanWithTypeUtils.convertDeeply(mgmt, utc, TypeToken.of(Instant.class), false, null, false);
+
+ BeanWithTypeUtils.convertShallow(mgmt, ""+utc, TypeToken.of(Instant.class), false, null, false);
+ BeanWithTypeUtils.convertDeeply(mgmt, ""+utc, TypeToken.of(Instant.class), false, null, false);
+
+ BeanWithTypeUtils.convertShallow(mgmt, inst, TypeToken.of(Instant.class), false, null, false);
+ BeanWithTypeUtils.convertDeeply(mgmt, inst, TypeToken.of(Instant.class), false, null, false);
+
+ // Date won't convert, either at root or nested; that is deliberate, we assume if you have a complex object it is already the right type,
+ // or you use coerce
+// BeanWithTypeUtils.convertShallow(mgmt, Date.from(inst), TypeToken.of(Instant.class), false, null, false);
+// BeanWithTypeUtils.convertDeeply(mgmt, Date.from(inst), TypeToken.of(Instant.class), false, null, false);
+// BeanWithTypeUtils.convertShallow(mgmt, MutableList.of(Date.from(inst)), new TypeToken<List<Instant>>() {}, false, null, false);
+// BeanWithTypeUtils.convertDeeply(mgmt, MutableList.of(Date.from(inst)), new TypeToken<List<Instant>>() {}, false, null, false);
}
@Test
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 5378589e08..fefe1e28fa 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
@@ -65,11 +65,17 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
}
@Test
- public void testImplicitEqualsAsRawPredicate() {
- // now this is supported, 2023-05, since we can coerce strings to DslPredicates
- Predicate p = TypeCoercions.coerce("x", Predicate.class);
- Asserts.assertTrue(p.test("x"));
- Asserts.assertFalse(p.test("y"));
+ public void testImplicitEqualsAsRawPredicateNotSupported() {
+ Asserts.assertFailsWith(() -> {
+ Predicate p = TypeCoercions.coerce("x", Predicate.class);
+ // we deliberately don't support this, because predicates of the form 'notNull' should resolve in a different way;
+ // but _given_ a predicate which should be DslPredicate, we _do_ want to allow it;
+ // see DslPredicates.JsonDeserializer registered/permitted classes
+ Asserts.fail("Should have failed, instead gave: "+p); // probably implicit equals
+ }, e -> {
+ Asserts.assertStringContainsIgnoreCase(e.toString(), "cannot convert", "given input", " x ", "predicate");
+ return true;
+ });
}
DslPredicates.DslPredicate predicate(String key, Object value) {
[brooklyn-server] 02/08: more workflow math for time
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 216b48558991a32c161cf8d2c43dd3250757cc3e
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu May 25 14:25:25 2023 +0100
more workflow math for time
add and subtract duration, subtract duration from instant, don't run unless it is the right operator
---
.../steps/variables/SetVariableWorkflowStep.java | 29 ++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 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 3e84563740..6531ba0cec 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
@@ -375,8 +375,33 @@ public class SetVariableWorkflowStep extends WorkflowStepDefinition {
Object lhs = process(lhs0, null);
Object rhs = process(rhs0, null);
- if (lhs instanceof Instant) return TypeCoercions.coerce(rhs, Duration.class).addTo((Instant)lhs);
- if (lhs instanceof Date) return new Timestamp((Instant) TypeCoercions.coerce(rhs, Duration.class).addTo( ((Date)lhs).toInstant() ));
+ if ("+".equals(op)) {
+ if (lhs instanceof Duration) {
+ if (rhs instanceof Instant || rhs instanceof Date) {
+ Object newRhs = lhs;
+ lhs = rhs;
+ rhs = newRhs;
+ // fall through to below
+ } else {
+ return TypeCoercions.coerce(rhs, Duration.class).add((Duration) lhs);
+ }
+ }
+ if (lhs instanceof Instant) return TypeCoercions.coerce(rhs, Duration.class).addTo((Instant) lhs);
+ if (lhs instanceof Date)
+ return new Timestamp((Instant) TypeCoercions.coerce(rhs, Duration.class).addTo(((Date) lhs).toInstant()));
+ } else if ("-".equals(op)) {
+ if (lhs instanceof Duration) {
+ return ((Duration)lhs).subtract(TypeCoercions.coerce(rhs, Duration.class));
+ }
+ if (lhs instanceof Instant) {
+ if (rhs instanceof Instant) return Duration.between((Instant)rhs, (Instant)lhs);
+ return TypeCoercions.coerce(rhs, Duration.class).multiply(-1).addTo((Instant) lhs);
+ }
+ if (lhs instanceof Date) {
+ if (rhs instanceof Date) return Duration.between(((Date)rhs).toInstant(), ((Date)lhs).toInstant());
+ return new Timestamp((Instant) TypeCoercions.coerce(rhs, Duration.class).multiply(-1).addTo(((Date) lhs).toInstant()));
+ }
+ }
Maybe<Integer> lhsI = asInteger(lhs);
Maybe<Integer> rhsI = asInteger(rhs);
[brooklyn-server] 08/08: Merge branch 'misc-fixes'
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 8bdc96f7327e1f17d114d215b06e7c90cd612ca7
Merge: d27d1720eb 0096fe41a6
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri May 26 14:18:57 2023 +0100
Merge branch 'misc-fixes'
.../core/resolve/jackson/BeanWithTypeUtils.java | 33 ++++--
.../resolve/jackson/CommonTypesSerialization.java | 16 +--
.../JacksonBetterDelegatingDeserializer.java | 17 +++
.../jackson/JsonSymbolDependentDeserializer.java | 7 +-
.../core/workflow/WorkflowExecutionContext.java | 5 +-
.../steps/variables/SetVariableWorkflowStep.java | 29 ++++-
.../workflow/steps/variables/TransformMerge.java | 2 +
.../brooklyn/util/core/flags/TypeCoercions.java | 2 +-
.../util/core/predicates/DslPredicates.java | 30 +++--
.../BrooklynMiscJacksonSerializationTest.java | 132 ++++++++++++++++++---
.../util/core/internal/TypeCoercionsTest.java | 100 ++++++++++------
.../util/core/predicates/DslPredicateTest.java | 10 +-
.../coerce/CommonAdaptorTypeCoercions.java | 15 ++-
.../javalang/coerce/TypeCoercerExtensible.java | 34 +++---
.../util/javalang/coerce/TypeCoercionsTest.java | 35 +-----
15 files changed, 325 insertions(+), 142 deletions(-)
[brooklyn-server] 06/08: return all errors from type coercion
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 70b08ce04b059492142b173bfaf80572e9b83035
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri May 26 12:10:39 2023 +0100
return all errors from type coercion
as we have a lot of coercers now available, it's useful to know which one(s) failed,
and to have their source sometimes
---
.../javalang/coerce/TypeCoercerExtensible.java | 34 +++++++++-------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensible.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensible.java
index 03926871be..c9c03b1910 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensible.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensible.java
@@ -26,8 +26,11 @@ import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.stream.Collectors;
+import com.google.common.collect.*;
import org.apache.brooklyn.core.validation.BrooklynValidation;
+import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.guava.AnyExceptionSupplier;
import org.apache.brooklyn.util.guava.Maybe;
@@ -43,12 +46,6 @@ import org.slf4j.LoggerFactory;
import com.google.common.annotations.Beta;
import com.google.common.base.Function;
import com.google.common.base.Objects;
-import com.google.common.collect.HashBasedTable;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
-import com.google.common.collect.Table;
import com.google.common.reflect.TypeToken;
/**
@@ -131,7 +128,7 @@ public class TypeCoercerExtensible implements TypeCoercer {
protected <T> Maybe<T> tryCoerceInternal2(Object value, TypeToken<T> targetTypeToken, Class<T> targetType) {
if (value==null) return Maybe.of((T)null);
Maybe<T> result = null;
- Maybe<T> firstError = null;
+ List<Maybe<T>> errors = MutableList.of();
//recursive coercion of parameterized collections and map entries
targetType = TypeTokens.getRawType(targetTypeToken, targetType);
@@ -178,8 +175,7 @@ public class TypeCoercerExtensible implements TypeCoercer {
}
}
- // remember any error if we were first
- if (result!=null && firstError==null) firstError = result;
+ if (result!=null) errors.add(result);
}
//ENHANCEMENT could look in type hierarchy of both types for a conversion method...
@@ -217,7 +213,7 @@ public class TypeCoercerExtensible implements TypeCoercer {
if (resultM.isPresent()) return resultM;
// if couldn't coerce parameterized types then back out of this coercer
// but remember the error if we were first
- if (firstError==null) firstError = resultM;
+ errors.add(resultM);
}
} else {
return Maybe.of(resultT);
@@ -226,15 +222,13 @@ public class TypeCoercerExtensible implements TypeCoercer {
Exceptions.propagateIfFatal(e);
if (log.isDebugEnabled()) {
log.debug("When coercing, registry adapter "+entry+" gave error on "+value+" -> "+targetType+" "
- + (firstError==null ? "(rethrowing)" : "(suppressing as there is already an error)")
+ + (errors.isEmpty() ? "(rethrowing)" : "(adding as secondary error as there is already another)")
+ ": "+e, e);
}
- if (firstError==null) {
- if (e instanceof ClassCoercionException) {
- firstError = Maybe.absent(e);
- } else {
- firstError = Maybe.absent(new ClassCoercionException("Cannot coerce type "+value.getClass().getCanonicalName()+" to "+targetTypeToken+" ("+value+"): registered coercer failed", e));
- }
+ if (e instanceof ClassCoercionException) {
+ errors.add(Maybe.absent(e));
+ } else {
+ errors.add(Maybe.absent(new ClassCoercionException("Cannot coerce type "+value.getClass().getCanonicalName()+" to "+targetTypeToken+" ("+value+"): registered coercer failed", e)));
}
continue;
}
@@ -243,9 +237,9 @@ public class TypeCoercerExtensible implements TypeCoercer {
}
// not found
- if (firstError!=null) {
- // it might be nice to have more than just the first error but for now that's all we remember
- return firstError;
+ if (!errors.isEmpty()) {
+ if (errors.size()==1) return Iterables.getOnlyElement(errors);
+ return Maybe.absent(Exceptions.create(errors.stream().map(Maybe.Absent::getException).collect(Collectors.toList())));
}
if (value instanceof Map) {
if (((Map)value).containsKey("type")) {
[brooklyn-server] 03/08: stricter coercion to maps
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 dd89910600c41b65cc17050bcd27a621da02f9ca
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu May 25 15:27:00 2023 +0100
stricter coercion to maps
previously allows "x" to be the {x: null} map; this has been removed unless there is at least one colon, e.g. 'key: value, key2' still allowed
also tidies where we thought 'key = value' would be parsed as a map but it wasn't due to test artifacts
---
.../workflow/steps/variables/TransformMerge.java | 2 +
.../util/core/internal/TypeCoercionsTest.java | 98 +++++++++++++---------
.../coerce/CommonAdaptorTypeCoercions.java | 15 +++-
.../util/javalang/coerce/TypeCoercionsTest.java | 35 ++------
4 files changed, 80 insertions(+), 70 deletions(-)
diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformMerge.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformMerge.java
index aabc425179..ea15cc6f90 100644
--- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformMerge.java
+++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/variables/TransformMerge.java
@@ -29,6 +29,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.text.QuotedStringTokenizer;
+import org.apache.brooklyn.util.text.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -104,6 +105,7 @@ public class TransformMerge extends WorkflowTransformDefault {
};
for (Object vi : (Iterable) v) {
+ if (vi instanceof String && Strings.isBlank((String)vi)) continue;
try {
vi = wait ? context.resolveWaiting(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, vi, TypeToken.of(type)) :
context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_RUNNING, vi, TypeToken.of(type));
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java b/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java
index 32167362c1..38e40ae234 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java
@@ -20,9 +20,6 @@ package org.apache.brooklyn.util.core.internal;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.util.collections.MutableList;
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNull;
-import static org.testng.Assert.assertTrue;
import java.math.BigDecimal;
import java.math.BigInteger;
@@ -41,7 +38,9 @@ import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.collections.MutableSet;
import org.apache.brooklyn.util.core.ClassLoaderUtils;
import org.apache.brooklyn.util.core.flags.TypeCoercions;
+import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.javalang.coerce.ClassCoercionException;
+import org.apache.brooklyn.util.javalang.coerce.CommonAdaptorTypeCoercions;
import org.apache.brooklyn.util.text.StringPredicates;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -56,10 +55,25 @@ import com.google.common.reflect.TypeToken;
import groovy.lang.GString;
+import static org.testng.Assert.*;
+
public class TypeCoercionsTest {
private static final Logger log = LoggerFactory.getLogger(TypeCoercionsTest.class);
-
+
+ // largely a duplicate of upstream common coerce/TypeCoercionsTest
+ // but with a few extras; ideally would be reconciled
+
+ private static void assertMapsEqual(Map actual, Map expected) {
+ Assert.assertEquals(actual, expected);
+
+ // workaround for bug in testng assert when values are null
+ // PR https://github.com/testng-team/testng/pull/2914 opened, hopefully included in testng 7.9 then this can be removed
+ if (actual instanceof Map && expected instanceof Map) {
+ assertEquals( ((Map)actual).keySet(), ((Map)expected).keySet(), "Keys in map differ: "+actual.keySet()+" / "+expected.keySet() );
+ }
+ }
+
@Test
public void testCoerceCharSequenceToString() {
assertEquals(TypeCoercions.coerce(new StringBuilder("abc"), String.class), "abc");
@@ -156,7 +170,7 @@ public class TypeCoercionsTest {
assertEquals(TypeCoercions.coerce(BigInteger.ONE, int.class), (Integer)1);
assertEquals(TypeCoercions.coerce(BigInteger.valueOf(Long.MAX_VALUE), Long.class), (Long)Long.MAX_VALUE);
assertEquals(TypeCoercions.coerce(BigInteger.valueOf(Long.MAX_VALUE), long.class), (Long)Long.MAX_VALUE);
-
+
assertEquals(TypeCoercions.coerce(BigDecimal.valueOf(0.5), Double.class), 0.5d, 0.00001d);
assertEquals(TypeCoercions.coerce(BigDecimal.valueOf(0.5), double.class), 0.5d, 0.00001d);
}
@@ -195,13 +209,13 @@ public class TypeCoercionsTest {
@Test
public void testListToSetCoercion() {
Set<?> s = TypeCoercions.coerce(ImmutableList.of(1), Set.class);
- Assert.assertEquals(s, ImmutableSet.of(1));
+ assertEquals(s, ImmutableSet.of(1));
}
@Test
public void testSetToListCoercion() {
List<?> s = TypeCoercions.coerce(ImmutableSet.of(1), List.class);
- Assert.assertEquals(s, ImmutableList.of(1));
+ assertEquals(s, ImmutableList.of(1));
}
@Test
@@ -223,41 +237,41 @@ public class TypeCoercionsTest {
public void testListEntryCoercion() {
@SuppressWarnings("serial")
List<?> s = TypeCoercions.coerce(ImmutableList.of("java.lang.Integer", "java.lang.Double"), new TypeToken<List<Class<?>>>() { });
- Assert.assertEquals(s, ImmutableList.of(Integer.class, Double.class));
+ assertEquals(s, ImmutableList.of(Integer.class, Double.class));
}
@Test
public void testListEntryToSetCoercion() {
@SuppressWarnings("serial")
Set<?> s = TypeCoercions.coerce(ImmutableList.of("java.lang.Integer", "java.lang.Double"), new TypeToken<Set<Class<?>>>() { });
- Assert.assertEquals(s, ImmutableSet.of(Integer.class, Double.class));
+ assertEquals(s, ImmutableSet.of(Integer.class, Double.class));
}
@Test
public void testListEntryToCollectionCoercion() {
@SuppressWarnings("serial")
Collection<?> s = TypeCoercions.coerce(ImmutableList.of("java.lang.Integer", "java.lang.Double"), new TypeToken<Collection<Class<?>>>() { });
- Assert.assertEquals(s, ImmutableList.of(Integer.class, Double.class));
+ assertEquals(s, ImmutableList.of(Integer.class, Double.class));
}
@Test
public void testMapValueCoercion() {
@SuppressWarnings("serial")
Map<?,?> s = TypeCoercions.coerce(ImmutableMap.of("int", "java.lang.Integer", "double", "java.lang.Double"), new TypeToken<Map<String, Class<?>>>() { });
- Assert.assertEquals(s, ImmutableMap.of("int", Integer.class, "double", Double.class));
+ assertMapsEqual(s, ImmutableMap.of("int", Integer.class, "double", Double.class));
}
@Test
public void testMapKeyCoercion() {
@SuppressWarnings("serial")
Map<?,?> s = TypeCoercions.coerce(ImmutableMap.of("java.lang.Integer", "int", "java.lang.Double", "double"), new TypeToken<Map<Class<?>, String>>() { });
- Assert.assertEquals(s, ImmutableMap.of(Integer.class, "int", Double.class, "double"));
+ assertMapsEqual(s, ImmutableMap.of(Integer.class, "int", Double.class, "double"));
}
@Test
public void testStringToListCoercion() {
List<?> s = TypeCoercions.coerce("a,b,c", List.class);
- Assert.assertEquals(s, ImmutableList.of("a", "b", "c"));
+ assertEquals(s, ImmutableList.of("a", "b", "c"));
}
@Test
@@ -269,97 +283,105 @@ public class TypeCoercionsTest {
@Test
public void testJsonStringToMapCoercion() {
Map<?,?> s = TypeCoercions.coerce("{ \"a\" : \"1\", b : 2 }", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", 2));
+ assertMapsEqual(s, ImmutableMap.of("a", "1", "b", 2));
}
@Test
public void testJsonStringWithoutQuotesToMapCoercion() {
Map<?,?> s = TypeCoercions.coerce("{ a : 1 }", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", 1));
+ assertMapsEqual(s, ImmutableMap.of("a", 1));
}
@Test
public void testJsonComplexTypesToMapCoercion() {
Map<?,?> s = TypeCoercions.coerce("{ a : [1, \"2\", '\"3\"'], b: { c: d, 'e': \"f\" } }", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", ImmutableList.<Object>of(1, "2", "\"3\""),
+ assertMapsEqual(s, ImmutableMap.of("a", ImmutableList.<Object>of(1, "2", "\"3\""),
"b", ImmutableMap.of("c", "d", "e", "f")));
}
@Test
public void testJsonStringWithoutBracesToMapCoercion() {
Map<?,?> s = TypeCoercions.coerce("a : 1", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", 1));
+ assertMapsEqual(s, ImmutableMap.of("a", 1));
}
@Test
public void testJsonStringWithoutBracesWithMultipleToMapCoercion() {
Map<?,?> s = TypeCoercions.coerce("a : 1, b : 2", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", 1, "b", 2));
+ assertMapsEqual(s, ImmutableMap.of("a", 1, "b", 2));
}
- @Test
+ @Test(enabled = false) //never actually worked, only seemed to because of bug in assertEquals(Map,Map)
public void testKeyEqualsValueStringToMapCoercion() {
+ if (!CommonAdaptorTypeCoercions.PARSE_MAPS_WITH_EQUALS_SYMBOL) Assert.fail("Known to be unsupported");
Map<?,?> s = TypeCoercions.coerce("a=1,b=2", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", "2"));
+ assertMapsEqual(s, ImmutableMap.of("a", "1", "b", "2"));
}
@Test
public void testJsonStringWithoutBracesOrSpaceDisallowedAsMapCoercion() {
- Map<?,?> s = TypeCoercions.coerce("a:1,b:2", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", "2"));
- // NB: snakeyaml 1.17 required spaces after the colon, but 1.21 accepts the above
+ Maybe<Map> s1 = TypeCoercions.tryCoerce("a:1,b:2", Map.class);
+ // NB: snakeyaml 1.17 required spaces after the colon, 1.21 accepts the above, but we explicitly disallow it
+ // (mileage may vary if you do something like a:1,b=2)
+ if (s1.isPresent()) Asserts.fail("Shouldn't allow colon without space when processing map; instead got: "+s1.get());
+
+ Map<?,?> s = TypeCoercions.coerce("a: 1,b: 2", Map.class);
+ assertMapsEqual(s, ImmutableMap.of("a", 1, "b", 2));
}
-
- @Test
- public void testEqualsInBracesMapCoercion() {
+
+ @Test(enabled = false) //never actually worked, only seemed to because of bug in assertEquals(Map,Map)
+ public void testEqualsInBracesMapCoercionLax() {
+ if (!CommonAdaptorTypeCoercions.PARSE_MAPS_WITH_EQUALS_SYMBOL) Assert.fail("Known to be unsupported");
Map<?,?> s = TypeCoercions.coerce("{ a = 1, b = '2' }", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", 1, "b", "2"));
+ assertMapsEqual(s, ImmutableMap.of("a", 1, "b", "2"));
}
- @Test
+ @Test(enabled = false) //never actually worked, only seemed to because of bug in assertEquals(Map,Map)
public void testKeyEqualsOrColonValueWithBracesStringToMapCoercion() {
+ if (!CommonAdaptorTypeCoercions.PARSE_MAPS_WITH_EQUALS_SYMBOL) Assert.fail("Known to be unsupported");
Map<?,?> s = TypeCoercions.coerce("{ a=1, b: 2 }", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", 2));
+ assertMapsEqual(s, ImmutableMap.of("a", "1", "b", 2));
}
- @Test
+ @Test(enabled = false) //never actually worked, only seemed to because of bug in assertEquals(Map,Map)
public void testKeyEqualsOrColonValueWithoutBracesStringToMapCoercion() {
+ if (!CommonAdaptorTypeCoercions.PARSE_MAPS_WITH_EQUALS_SYMBOL) Assert.fail("Known to be unsupported");
Map<?,?> s = TypeCoercions.coerce("a=1, b: 2", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", 2));
+ assertMapsEqual(s, ImmutableMap.of("a", "1", "b", 2));
}
@SuppressWarnings("serial")
@Test
public void testYamlMapsDontGoTooFarWhenWantingListOfString() {
List<?> s = TypeCoercions.coerce("[ a: 1, b: 2 ]", List.class);
- Assert.assertEquals(s, ImmutableList.of(MutableMap.of("a", 1), MutableMap.of("b", 2)));
+ assertEquals(s, ImmutableList.of(MutableMap.of("a", 1), MutableMap.of("b", 2)));
s = TypeCoercions.coerce("[ a: 1, b : 2 ]", new TypeToken<List<String>>() {});
- Assert.assertEquals(s, ImmutableList.of("a: 1", "b : 2"));
+ assertEquals(s, ImmutableList.of("a: 1", "b : 2"));
}
@Test
public void testURItoStringCoercion() {
String s = TypeCoercions.coerce(URI.create("http://localhost:1234/"), String.class);
- Assert.assertEquals(s, "http://localhost:1234/");
+ assertEquals(s, "http://localhost:1234/");
}
@Test
public void testURLtoStringCoercion() throws MalformedURLException {
String s = TypeCoercions.coerce(new URL("http://localhost:1234/"), String.class);
- Assert.assertEquals(s, "http://localhost:1234/");
+ assertEquals(s, "http://localhost:1234/");
}
@Test
public void testAs() {
Integer x = TypeCoercions.coerce(new WithAs("3"), Integer.class);
- Assert.assertEquals(x, (Integer)3);
+ assertEquals(x, (Integer)3);
}
@Test
public void testFrom() {
WithFrom x = TypeCoercions.coerce("3", WithFrom.class);
- Assert.assertEquals(x.value, 3);
+ assertEquals(x.value, 3);
}
@Test
@@ -433,7 +455,7 @@ public class TypeCoercionsTest {
public void testObjectInMapCoercion() {
ClassWithMap r1 =
TypeCoercions.coerce(MutableMap.of("properties", MutableMap.of("x", 1)), ClassWithMap.class);
- Assert.assertEquals(r1.properties.get("x"), 1);
+ assertEquals(r1.properties.get("x"), 1);
r1 = TypeCoercions.coerce(MutableMap.of("properties", MutableMap.of("x", new MyClazz())), ClassWithMap.class);
Asserts.assertInstanceOf(r1.properties.get("x"), MyClazz.class);
diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java
index 1a1ed20928..20c8649b40 100644
--- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java
+++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java
@@ -62,6 +62,11 @@ import com.google.common.reflect.TypeToken;
public class CommonAdaptorTypeCoercions {
+ // It might be nice to support key=value syntax when trying to parse things as maps.
+ // We have never supported this, but a bug in testng made it look like we did.
+ // This flags relevant areas of code and tests
+ public static final boolean PARSE_MAPS_WITH_EQUALS_SYMBOL = false;
+
@Beta public static final double DELTA_FOR_COERCION = 0.000001;
private final TypeCoercerExtensible coercer;
@@ -473,9 +478,15 @@ public class CommonAdaptorTypeCoercions {
};
Maybe<Map<?, ?>> r1 = null;
-
+
+ if (PARSE_MAPS_WITH_EQUALS_SYMBOL) {
+ // implement it here if we support it. could perhaps simply replace with ": " ?
+ // but ideally want more sophisticated quote processing and splitting
+ throw new IllegalStateException("Parsing maps with equals not currently supported");
+ }
+
// first try wrapping in braces if needed
- if (!inputS.trim().startsWith("{")) {
+ if (!inputS.trim().startsWith("{") && (inputS.contains(": "))) {
r1 = parseYaml.apply("{ "+inputS+" }");
if (r1.isPresent()) return (Maybe<T>) r1;
// fall back to parsing without braces, e.g. if it's multiline
diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercionsTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercionsTest.java
index 8f85bf1e33..0a6ebd5105 100644
--- a/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercionsTest.java
+++ b/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercionsTest.java
@@ -38,6 +38,7 @@ import java.util.TimeZone;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.guava.Maybe;
import org.apache.brooklyn.util.text.StringPredicates;
import org.apache.commons.lang3.builder.EqualsBuilder;
import org.slf4j.Logger;
@@ -347,6 +348,9 @@ public class TypeCoercionsTest {
public void testJsonStringToMapCoercion() {
Map<?,?> s = coerce("{ \"a\" : \"1\", b : 2 }", Map.class);
Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", 2));
+
+ Maybe<Map> result = coercer.tryCoerce("x", Map.class);
+ if (result.isPresent()) Asserts.fail("Should have failed, instead produced map "+result.get());
}
@Test
@@ -374,36 +378,7 @@ public class TypeCoercionsTest {
Assert.assertEquals(s, ImmutableMap.of("a", 1, "b", 2));
}
- @Test
- public void testKeyEqualsValueStringToMapCoercion() {
- Map<?,?> s = coerce("a=1,b=2", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", "2"));
- }
-
- @Test
- public void testJsonStringWithoutBracesOrSpaceDisallowedAsMapCoercion() {
- Map<?,?> s = coerce("a:1,b:2", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", "2"));
- // NB: snakeyaml 1.17 required spaces after the colon, but 1.21 accepts the above
- }
-
- @Test
- public void testEqualsInBracesMapCoercion() {
- Map<?,?> s = coerce("{ a = 1, b = '2' }", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", 1, "b", "2"));
- }
-
- @Test
- public void testKeyEqualsOrColonValueWithBracesStringToMapCoercion() {
- Map<?,?> s = coerce("{ a=1, b: 2 }", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", 2));
- }
-
- @Test
- public void testKeyEqualsOrColonValueWithoutBracesStringToMapCoercion() {
- Map<?,?> s = coerce("a=1, b: 2", Map.class);
- Assert.assertEquals(s, ImmutableMap.of("a", "1", "b", 2));
- }
+ // map tests removed, tested in downstream core/internalTypeCoercionsTest
@Test
public void testURItoStringCoercion() {
[brooklyn-server] 05/08: don't use longhand object writer when given a primitive or string input
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 f35185c46609495853f727b5e5ad207a011c8367
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri May 26 11:20:46 2023 +0100
don't use longhand object writer when given a primitive or string input
---
.../brooklyn/core/resolve/jackson/BeanWithTypeUtils.java | 7 ++++++-
.../jackson/BrooklynMiscJacksonSerializationTest.java | 16 ++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
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 c829dfed68..ff5902eee1 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
@@ -199,7 +199,12 @@ public class BeanWithTypeUtils {
public static <T> T convertDeeply(ManagementContext mgmt, Object mapOrListToSerializeThenDeserialize, TypeToken<T> type, boolean allowRegisteredTypes, BrooklynClassLoadingContext loader, boolean allowJavaTypes) throws JsonProcessingException {
// try full serialization - but won't work if things being written cannot be deserialized, eg due to unknown type
ObjectMapper mapper = newMapper(mgmt, allowRegisteredTypes, loader, allowJavaTypes);
- String serialization = type.getRawType().equals(Object.class) ? mapper.writeValueAsString(mapOrListToSerializeThenDeserialize) : mapper.writerFor(Object.class).writeValueAsString(mapOrListToSerializeThenDeserialize);
+ boolean useLonghandObjectWriter = true;
+
+ if (type.getRawType().equals(Object.class)) useLonghandObjectWriter = false;
+ else if (mapOrListToSerializeThenDeserialize==null || mapOrListToSerializeThenDeserialize instanceof String || Boxing.isPrimitiveOrBoxedObject(mapOrListToSerializeThenDeserialize)) useLonghandObjectWriter = false;
+
+ String serialization = !useLonghandObjectWriter ? mapper.writeValueAsString(mapOrListToSerializeThenDeserialize) : mapper.writerFor(Object.class).writeValueAsString(mapOrListToSerializeThenDeserialize);
return mapper.readValue(serialization, BrooklynJacksonType.asJavaType(mapper, type));
}
diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java
index 5caa8ef19f..0db76c0ea6 100644
--- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java
@@ -37,6 +37,9 @@ import java.util.Map;
import java.util.TimeZone;
import java.util.function.BiConsumer;
+import com.google.common.reflect.TypeToken;
+import org.apache.brooklyn.api.mgmt.ManagementContext;
+import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
@@ -201,6 +204,19 @@ public class BrooklynMiscJacksonSerializationTest implements MapperTestFixture {
Assert.assertEquals( impl2.instant, impl.instant );
}
+ @Test
+ public void testInstantFromLong() throws Exception {
+ mapper = BeanWithTypeUtils.newYamlMapper(null, false, null, true);
+ long utc = new Date().getTime();
+ mapper.readerFor(Instant.class).readValue( mapper.writeValueAsString(utc) );
+ // below known not to work, as long is converted to ["j...Long", utc] which we don't process
+ //mapper.readerFor(Instant.class).readValue( mapper.writerFor(Object.class).writeValueAsString(utc) );
+
+ ManagementContext mgmt = LocalManagementContextForTests.newInstance();
+ BeanWithTypeUtils.convertShallow(mgmt, utc, TypeToken.of(Instant.class), false, null, false);
+ BeanWithTypeUtils.convertDeeply(mgmt, utc, TypeToken.of(Instant.class), false, null, false);
+ }
+
@Test
public void testFailsOnTrailing() throws Exception {
try {
[brooklyn-server] 04/08: permit jackson to be used to coerce strings and primitives
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 4660a30ce04a3d168098cee68a5ecef286337938
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Fri May 26 10:53:28 2023 +0100
permit jackson to be used to coerce strings and primitives
(previously only partially permitted)
---
.../core/resolve/jackson/BeanWithTypeUtils.java | 26 ++++--
.../brooklyn/util/core/flags/TypeCoercions.java | 2 +-
.../BrooklynMiscJacksonSerializationTest.java | 96 +++++++++++++++++++---
.../util/core/internal/TypeCoercionsTest.java | 2 +-
.../util/core/predicates/DslPredicateTest.java | 12 ++-
5 files changed, 111 insertions(+), 27 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 f04e7fc6c9..c829dfed68 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
@@ -29,10 +29,7 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLMapper;
import com.google.common.annotations.Beta;
import com.google.common.reflect.TypeToken;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
import java.util.Map.Entry;
import java.util.function.Predicate;
import org.apache.brooklyn.api.entity.Entity;
@@ -157,9 +154,21 @@ public class BeanWithTypeUtils {
* see in JsonDeserializerForCommonBrooklynThings. See DslSerializationTest.
*/
+ static ThreadLocal<Stack<Object>> activeConversions = new ThreadLocal<>();
public static <T> T convert(ManagementContext mgmt, Object mapOrListToSerializeThenDeserialize, TypeToken<T> type, boolean allowRegisteredTypes, BrooklynClassLoadingContext loader, boolean allowJavaTypes) throws JsonProcessingException {
+ Stack<Object> stack = activeConversions.get();
+ if (stack==null) {
+ stack = new Stack<>();
+ activeConversions.set(stack);
+ }
+ // simple things, like string, might be converted by a JsonDeserializer or by other TypeCoercions;
+ // so the former calls the latter, and the latter calls the former. but stop at some point!
+ if (stack.contains(mapOrListToSerializeThenDeserialize)) throw new IllegalStateException("Aborting recursive attempt to convert '"+mapOrListToSerializeThenDeserialize+"'");
+
try {
+ stack.push(mapOrListToSerializeThenDeserialize);
+
return convertDeeply(mgmt, mapOrListToSerializeThenDeserialize, type, allowRegisteredTypes, loader, allowJavaTypes);
} catch (Exception e) {
@@ -168,6 +177,9 @@ public class BeanWithTypeUtils {
} catch (Exception e2) {
throw Exceptions.propagate(Arrays.asList(e, e2));
}
+ } finally {
+ stack.pop();
+ if (stack.isEmpty()) activeConversions.remove();
}
}
@@ -208,7 +220,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)) {
+ if (!(o instanceof Map) && !(o instanceof List) && !Boxing.isPrimitiveOrBoxedObject(o) && !(o instanceof String)) {
if (type.isSupertypeOf(o.getClass())) {
return (Maybe<T>)inputMap;
} else {
@@ -299,7 +311,7 @@ public class BeanWithTypeUtils {
}
}
- // we want some special object. if we have a map or a string then conversion might sort us out.
- return (t instanceof Map || t instanceof String) ? 1 : -1;
+ // 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;
}
}
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 0e3fa09bd3..d630819a4a 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))) {
+ if (!(input instanceof Map || input instanceof Collection || Boxing.isPrimitiveOrBoxedObject(input) || input instanceof String)) {
return null;
}
if (BeanWithTypeUtils.isConversionRecommended(Maybe.of(input), type)) {
diff --git a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java
index deee812b4c..5caa8ef19f 100644
--- a/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/core/resolve/jackson/BrooklynMiscJacksonSerializationTest.java
@@ -18,36 +18,33 @@
*/
package org.apache.brooklyn.core.resolve.jackson;
+import com.fasterxml.jackson.core.JacksonException;
+import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.SerializationFeature;
-import com.fasterxml.jackson.databind.json.JsonMapper;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.util.TokenBuffer;
import com.fasterxml.jackson.dataformat.yaml.YAMLMapper;
-import com.google.common.reflect.TypeToken;
+
import java.io.IOException;
import java.time.Instant;
-import java.time.LocalDateTime;
import java.util.Date;
import java.util.GregorianCalendar;
import java.util.Locale;
import java.util.Map;
import java.util.TimeZone;
import java.util.function.BiConsumer;
-import java.util.function.Consumer;
-import org.apache.brooklyn.api.typereg.RegisteredType;
-import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
-import org.apache.brooklyn.core.typereg.BasicTypeImplementationPlan;
-import org.apache.brooklyn.core.typereg.RegisteredTypes;
import org.apache.brooklyn.test.Asserts;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.collections.MutableSet;
+import org.apache.brooklyn.util.core.flags.TypeCoercions;
import org.apache.brooklyn.util.core.units.ByteSize;
import org.apache.brooklyn.util.exceptions.Exceptions;
import org.apache.brooklyn.util.javalang.JavaClassNames;
import org.apache.brooklyn.util.text.Secret;
-import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes;
import org.apache.brooklyn.util.text.Strings;
import org.apache.brooklyn.util.time.Duration;
import org.slf4j.Logger;
@@ -253,4 +250,81 @@ public class BrooklynMiscJacksonSerializationTest implements MapperTestFixture {
WrappedValue<Secret<String>> s1;
}
+
+ @JsonDeserialize(using = SampleFromStringDeserializer.class)
+ static class SampleFromStringDeserialized {
+ public Object subtypeWanted;
+ public String x;
+ }
+ @JsonDeserialize(using = JsonDeserializer.None.class)
+ static class SampleFromStringSubtype extends SampleFromStringDeserialized {}
+ @JsonDeserialize(using = JsonDeserializer.None.class)
+ static class SampleFromStringSubtype1 extends SampleFromStringSubtype {}
+ static class SampleFromStringSubtype2 extends SampleFromStringSubtype {}
+
+ static class SampleFromStringDeserializer extends JsonDeserializer {
+ @Override
+ public Object deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JacksonException {
+ TokenBuffer buffer = BrooklynJacksonSerializationUtils.createBufferForParserCurrentObject(p, ctxt);
+ Object raw = new JsonPassThroughDeserializer().deserialize(
+ BrooklynJacksonSerializationUtils.createParserFromTokenBufferAndParser(buffer, p),
+ ctxt);
+
+ Integer rawi = null;
+ if (raw instanceof String) rawi = Integer.parseInt((String)raw);
+ if (raw instanceof Integer) rawi = (Integer)raw;
+ if (rawi!=null) {
+ SampleFromStringSubtype result = null;
+ if (rawi==1) result = new SampleFromStringSubtype1();
+ if (rawi==2) result = new SampleFromStringSubtype2();
+ if (result!=null) {
+ result.subtypeWanted = raw;
+ }
+ return result;
+ }
+
+ if (raw instanceof Map) {
+ Integer stw = TypeCoercions.tryCoerce(((Map) raw).get("subtypeWanted"), Integer.class).orNull();
+ if (stw!=null && stw>=0) {
+ try {
+ return ctxt.findNonContextualValueDeserializer(ctxt.constructType(Class.forName(SampleFromStringSubtype.class.getName()+stw))).deserialize(
+ BrooklynJacksonSerializationUtils.createParserFromTokenBufferAndParser(buffer, p), ctxt);
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+ return ctxt.findNonContextualValueDeserializer(ctxt.constructType(SampleFromStringSubtype.class)).deserialize(
+ BrooklynJacksonSerializationUtils.createParserFromTokenBufferAndParser(buffer, p), ctxt);
+ }
+ }
+
+ @Test
+ public void testDeserializeFromStringOrMapOrEvenWithMapKey() throws JsonProcessingException {
+ Object s;
+ mapper = BeanWithTypeUtils.newSimpleYamlMapper(); //YAMLMapper.builder().build();
+ s = mapper.readerFor(SampleFromStringDeserialized.class).readValue("1");
+ Asserts.assertInstanceOf(s, SampleFromStringSubtype1.class);
+ Asserts.assertEquals( ((SampleFromStringSubtype)s).subtypeWanted, 1 );
+
+ s = mapper.readerFor(SampleFromStringDeserialized.class).readValue("\"2\"");
+ Asserts.assertInstanceOf(s, SampleFromStringSubtype2.class);
+ Asserts.assertEquals( ((SampleFromStringSubtype)s).subtypeWanted, "2" );
+
+ s = mapper.readerFor(SampleFromStringDeserialized.class).readValue("subtypeWanted: 1");
+ Asserts.assertInstanceOf(s, SampleFromStringSubtype1.class);
+ Asserts.assertEquals( ((SampleFromStringSubtype)s).subtypeWanted, 1 );
+
+ s = mapper.readerFor(SampleFromStringDeserialized.class).readValue("subtypeWanted: \"-1\"");
+ Asserts.assertEquals(s.getClass(), SampleFromStringSubtype.class);
+ Asserts.assertEquals( ((SampleFromStringSubtype)s).subtypeWanted, "-1" );
+
+ s = TypeCoercions.coerce("1", SampleFromStringDeserialized.class);
+ Asserts.assertInstanceOf(s, SampleFromStringSubtype1.class);
+ Asserts.assertEquals( ((SampleFromStringSubtype)s).subtypeWanted, "1" );
+ s = TypeCoercions.coerce(1, SampleFromStringDeserialized.class);
+ Asserts.assertEquals( ((SampleFromStringSubtype)s).subtypeWanted, 1 );
+ }
+
}
diff --git a/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java b/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java
index 38e40ae234..c9468f1431 100644
--- a/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java
+++ b/core/src/test/java/org/apache/brooklyn/util/core/internal/TypeCoercionsTest.java
@@ -386,7 +386,7 @@ public class TypeCoercionsTest {
@Test
public void testCoerceStringToNumber() {
- assertEquals(TypeCoercions.coerce("1", Number.class), Double.valueOf(1));
+ assertEquals(TypeCoercions.coerce("1", Number.class), 1); // since Jackson is permtitted to coerce, this prefers integers over doubles, 2023-05
assertEquals(TypeCoercions.coerce("1.0", Number.class), Double.valueOf(1.0));
}
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 74bc686501..5378589e08 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
@@ -65,13 +65,11 @@ public class DslPredicateTest extends BrooklynMgmtUnitTestSupport {
}
@Test
- public void testImplicitEqualsAsRawPredicateNotSupported() {
- Asserts.assertFailsWith(() -> {
- Predicate p = TypeCoercions.coerce("x", Predicate.class);
- }, e -> {
- Asserts.assertStringContainsIgnoreCase(e.toString(), "cannot coerce", "string", "predicate");
- return true;
- });
+ public void testImplicitEqualsAsRawPredicate() {
+ // now this is supported, 2023-05, since we can coerce strings to DslPredicates
+ Predicate p = TypeCoercions.coerce("x", Predicate.class);
+ Asserts.assertTrue(p.test("x"));
+ Asserts.assertFalse(p.test("y"));
}
DslPredicates.DslPredicate predicate(String key, Object value) {
[brooklyn-server] 01/08: allow use of overridden condition
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 1d2f6bf3c99dc48e6b79ed3da45bf36571892567
Author: Alex Heneveld <al...@cloudsoft.io>
AuthorDate: Thu May 25 14:23:54 2023 +0100
allow use of overridden condition
e.g. when using a foreach, condition applies to elements in the list
---
.../org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java | 5 +++--
1 file changed, 3 insertions(+), 2 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 0319263f87..19c28a6baa 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
@@ -1279,9 +1279,10 @@ public class WorkflowExecutionContext {
WorkflowStepDefinition step = getStepsResolved().get(currentStepIndex);
if (step!=null) {
currentStepInstance = new WorkflowStepInstanceExecutionContext(currentStepIndex, step, WorkflowExecutionContext.this);
- if (step.condition!=null) {
+ DslPredicates.DslPredicate conditionResolved = step.getConditionResolved(currentStepInstance);
+ if (conditionResolved!=null) {
if (log.isTraceEnabled()) log.trace("Considering condition "+step.condition+" for "+ workflowStepReference(currentStepIndex));
- boolean conditionMet = DslPredicates.evaluateDslPredicateWithBrooklynObjectContext(step.getConditionResolved(currentStepInstance), WorkflowExecutionContext.this, getEntityOrAdjunctWhereRunning());
+ boolean conditionMet = DslPredicates.evaluateDslPredicateWithBrooklynObjectContext(conditionResolved, WorkflowExecutionContext.this, getEntityOrAdjunctWhereRunning());
if (log.isTraceEnabled()) log.trace("Considered condition "+step.condition+" for "+ workflowStepReference(currentStepIndex)+": "+conditionMet);
if (!conditionMet) {
moveToNextStep("Skipping step "+ workflowStepReference(currentStepIndex), false);