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 <[email protected]> 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) {
