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 f05fe050fbce8404222896481163807abcbe7f15 Author: Alex Heneveld <a...@cloudsoft.io> AuthorDate: Fri May 10 14:04:01 2024 +0100 better ordering of type coercions, run "wrong bean" coercions after standard coercions negative indexed coercions run after the standard coercions, and the natural order comparator is used so that 11 runs after 9 --- .../brooklyn/util/core/flags/TypeCoercions.java | 2 +- .../coerce/CommonAdaptorTypeCoercions.java | 4 +- .../javalang/coerce/TypeCoercerExtensible.java | 124 ++++++++++++--------- .../javalang/coerce/TypeCoercerExtensibleTest.java | 33 ++++++ 4 files changed, 109 insertions(+), 54 deletions(-) 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 3840c0b838..35b1752690 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 @@ -381,7 +381,7 @@ public class TypeCoercions { } }); - registerAdapter("81-wrong-bean-to-map-or-bean", new TryCoercer() { + registerAdapter("-20-wrong-bean-to-map-or-bean", new TryCoercer() { @Override public <T> Maybe<T> tryCoerce(Object input, TypeToken<T> type) { 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 20c8649b40..a792c70880 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 @@ -88,7 +88,9 @@ public class CommonAdaptorTypeCoercions { public synchronized <A,B> Function<? super A,B> registerAdapter(Class<A> sourceType, Class<B> targetType, Function<? super A,B> fn) { return coercer.registerAdapter(sourceType, targetType, fn); } - /** Registers an adapter for use with type coercion. */ + /** Registers an adapter for use with type coercion. nameAndOrder is of the form NUM-NAME with the natural order prevailing (ordered by NUM numerically), + * eg 1-x before 2-x before 9-x before 11-x; + * negative indexed orders are processed after, with -1-x before -2-x before -11-x */ public synchronized void registerAdapter(String nameAndOrder, TryCoercer coerceFn) { coercer.registerAdapter(nameAndOrder, coerceFn); } 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 c235d537ab..fe5ec0ca49 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 @@ -20,10 +20,26 @@ package org.apache.brooklyn.util.javalang.coerce; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; -import java.util.*; +import java.util.Collection; +import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.stream.Collectors; -import com.google.common.collect.*; +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.Iterables; +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; import org.apache.brooklyn.core.validation.BrooklynValidation; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.exceptions.Exceptions; @@ -32,17 +48,13 @@ import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.guava.TypeTokens; import org.apache.brooklyn.util.javalang.Boxing; import org.apache.brooklyn.util.javalang.Reflections; +import org.apache.brooklyn.util.text.NaturalOrderComparator; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.Duration; import org.apache.brooklyn.util.time.Time; import org.slf4j.Logger; 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.reflect.TypeToken; - /** * Attempts to coerce {@code value} to {@code targetType}. * <p> @@ -88,9 +100,7 @@ public class TypeCoercerExtensible implements TypeCoercer { private Table<Class<?>, Class<?>, Function<?,?>> registry = HashBasedTable.create(); /** Store the generic coercers, ordered by the name; reset each time updated */ - private SortedMap<String,TryCoercer> genericCoercersByName = Maps.newTreeMap(); - /** Put the list in a cache, reset each time the map is updated. */ - private List<TryCoercer> genericCoercers = new ArrayList<>(); + private SortedMap<String,TryCoercer> genericCoercersByName = Maps.newTreeMap(NaturalOrderComparator.INSTANCE); @Override public <T> T coerce(Object value, Class<T> targetType) { @@ -148,43 +158,14 @@ public class TypeCoercerExtensible implements TypeCoercer { if (targetType.isInstance(value)) return Maybe.of( (T) value ); targetTypeToken = TypeTokens.getTypeToken(targetTypeToken, targetType); - for (TryCoercer coercer : genericCoercers) { - result = coercer.tryCoerce(value, targetTypeToken); - - if (result!=null && result.isPresentAndNonNull()) { - // Check if need to unwrap again (e.g. if want List<Integer> and are given a String "1,2,3" - // then we'll have so far converted to List.of("1", "2", "3"). Call recursively. - // First check that value has changed, to avoid stack overflow! - if (!Objects.equal(value, result.get()) && !Objects.equal(value.getClass(), result.get().getClass()) - // previously did this just for generics but it's more useful than that, e.g. if was a WrappedValue - //&& targetTypeToken.getType() instanceof ParameterizedType - ) { - Maybe<T> resultM = tryCoerce(result.get(), targetTypeToken); - if (resultM!=null) { - if (resultM.isPresent()) return resultM; - // if couldn't coerce parameterized types then back out of this coercer - result = resultM; - } - } else { - return result; - } - } - - if (result!=null) { - if (result.isAbsent()) errors.add(result); - else { - if (coercer instanceof TryCoercer.TryCoercerReturningNull) { - return result; - } else { - String c = getCoercerName(coercer); - log.warn("Coercer " + c + " returned wrapped null when coercing " + value); - errors.add(Maybe.absent("coercion returned null ("+c+")")); - // coercers the return null should implement 'TryCoercerReturningNull' - } - } + for (Entry<String, TryCoercer> mapEntry : genericCoercersByName.entrySet()) { + String coercerName = mapEntry.getKey(); + if (coercerName != null && !coercerName.startsWith("-")) { + Maybe<T> resultM = applyCoercer(value, targetTypeToken, errors, mapEntry.getValue(), coercerName); + if (resultM != null) return resultM; } } - + //ENHANCEMENT could look in type hierarchy of both types for a conversion method... //at this point, if either is primitive then run instead over boxed types @@ -242,6 +223,15 @@ public class TypeCoercerExtensible implements TypeCoercer { } } + // now try negative ordered coercers + for (Entry<String, TryCoercer> mapEntry : genericCoercersByName.entrySet()) { + String coercerName = mapEntry.getKey(); + if (coercerName != null && coercerName.startsWith("-")) { + Maybe<T> resultM = applyCoercer(value, targetTypeToken, errors, mapEntry.getValue(), coercerName); + if (resultM != null) return resultM; + } + } + // not found if (!errors.isEmpty()) { if (errors.size()==1) return Iterables.getOnlyElement(errors); @@ -256,11 +246,42 @@ public class TypeCoercerExtensible implements TypeCoercer { return Maybe.absent(new ClassCoercionException("Cannot coerce type "+value.getClass().getCanonicalName()+" to "+targetTypeToken+" ("+value+"): no adapter known")); } - protected String getCoercerName(TryCoercer coercer) { - Optional<Map.Entry<String, TryCoercer>> bn = genericCoercersByName.entrySet().stream().filter(es -> Objects.equal(coercer, es.getValue())).findAny(); - if (bn.isPresent()) return bn.get().getKey(); - int index = genericCoercers.indexOf(coercer); - return coercer.toString() + (index>=0 ? " (index "+index+")" : ""); + private <T> Maybe<T> applyCoercer(Object value, TypeToken<T> targetTypeToken, List<Maybe<T>> errors, TryCoercer coercer, String coercerName) { + Maybe<T> result; + result = coercer.tryCoerce(value, targetTypeToken); + + if (result!=null && result.isPresentAndNonNull()) { + // Check if need to unwrap again (e.g. if want List<Integer> and are given a String "1,2,3" + // then we'll have so far converted to List.of("1", "2", "3"). Call recursively. + // First check that value has changed, to avoid stack overflow! + if (!Objects.equal(value, result.get()) && !Objects.equal(value.getClass(), result.get().getClass()) + // previously did this just for generics but it's more useful than that, e.g. if was a WrappedValue + //&& targetTypeToken.getType() instanceof ParameterizedType + ) { + Maybe<T> resultM = tryCoerce(result.get(), targetTypeToken); + if (resultM!=null) { + if (resultM.isPresent()) return resultM; + // if couldn't coerce parameterized types then back out of this coercer + result = resultM; + } + } else { + return result; + } + } + + if (result!=null) { + if (result.isAbsent()) errors.add(result); + else { + if (coercer instanceof TryCoercer.TryCoercerReturningNull) { + return result; + } else { + log.warn("Coercer " + coercerName + " returned wrapped null when coercing " + value); + errors.add(Maybe.absent("coercion returned null ("+coercerName+")")); + // coercers that return null should implement 'TryCoercerReturningNull' + } + } + } + return null; } @SuppressWarnings("unchecked") @@ -353,9 +374,8 @@ public class TypeCoercerExtensible implements TypeCoercer { TreeMap<String, TryCoercer> gcn = Maps.newTreeMap(genericCoercersByName); gcn.put(nameAndOrder, fn); genericCoercersByName = gcn; - genericCoercers = ImmutableList.copyOf(genericCoercersByName.values()); } - + /** @deprecated since introduction, use {@link #registerAdapter(String, TryCoercer)} */ @Beta @Deprecated public void registerAdapter(TryCoercer fn) { diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensibleTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensibleTest.java index 5c86b2cfcb..2d67597878 100644 --- a/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensibleTest.java +++ b/utils/common/src/test/java/org/apache/brooklyn/util/javalang/coerce/TypeCoercerExtensibleTest.java @@ -81,4 +81,37 @@ public class TypeCoercerExtensibleTest { return MoreObjects.toStringHelper(this).add("val", val).toString(); } } + + void registerOrderedConditionalMyClassAdapter(int order, String requiredToken) { + coercer.registerAdapter(""+order+"-"+requiredToken, + new TryCoercer() { + @Override + @SuppressWarnings("unchecked") + public <T> Maybe<T> tryCoerce(Object input, TypeToken<T> type) { + if (input instanceof String && type.getRawType().isAssignableFrom(Integer.class) && ((String)input).contains(requiredToken)) + return Maybe.of((T)(Object) (order*100+((String)input).length())); + return null; + } + }); + } + + @Test + public void testPriority() { + registerOrderedConditionalMyClassAdapter(-1, "11"); + registerOrderedConditionalMyClassAdapter(1, "a"); + registerOrderedConditionalMyClassAdapter(2, "b"); + registerOrderedConditionalMyClassAdapter(3, "33"); + registerOrderedConditionalMyClassAdapter(-2, "22"); + + assertEquals(coerce("a", Integer.class), (Integer) 101); + assertEquals(coerce("ab", Integer.class), (Integer) 102); + assertEquals(coerce("bb", Integer.class), (Integer) 202); + assertEquals(coerce("33", Integer.class), (Integer) 302); + assertEquals(coerce("ab11", Integer.class), (Integer) 104); // coercer 1-a applies + assertEquals(coerce("d11", Integer.class), (Integer) (-97)); // coercer -1-11 applies + assertEquals(coerce("d1122", Integer.class), (Integer) (-95)); // coercer -1-11 applies + assertEquals(coerce("d2222", Integer.class), (Integer) (-195)); // coercer -2-22 applies + assertEquals(coerce("33", Integer.class), (Integer) (302)); // coercer 3-33 applies before Integer.toString + assertEquals(coerce("1122", Integer.class), (Integer) (1122)); // Integer.fromString coercion applies before negative coercers + } }