From 0f3a641588eb5dd66ac260d28a4824733708b1ca Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Wed, 15 Apr 2020 15:39:57 +0200 Subject: [PATCH] [jOOQ/jOOQ#10071] DefaultRecordMapper should apply ConverterProvider for Record1.into(...) calls --- .../main/java/org/jooq/ConverterProvider.java | 8 +- .../java/org/jooq/impl/AbstractRecord.java | 5 +- .../jooq/impl/DefaultConverterProvider.java | 132 +++++++++++++++--- .../org/jooq/impl/DefaultRecordMapper.java | 7 +- .../main/java/org/jooq/impl/ResultImpl.java | 3 +- jOOQ/src/main/java/org/jooq/impl/Tools.java | 50 ++++++- 6 files changed, 167 insertions(+), 38 deletions(-) diff --git a/jOOQ/src/main/java/org/jooq/ConverterProvider.java b/jOOQ/src/main/java/org/jooq/ConverterProvider.java index 9a1e01d2b1..801269b9c0 100644 --- a/jOOQ/src/main/java/org/jooq/ConverterProvider.java +++ b/jOOQ/src/main/java/org/jooq/ConverterProvider.java @@ -48,10 +48,6 @@ import org.jooq.impl.DefaultConverterProvider; * {@link RecordMapper}, e.g. when mapping {@link JSON} or {@link XML} data * types onto POJO types using third party libraries like Jackson, Gson, JAXB, * or others. - *

- * It is recommended to delegate all calls to - * {@link DefaultConverterProvider#provide(Class, Class)} for pairs of classes - * that are not handled by this converter provider. * * @author Lukas Eder */ @@ -61,6 +57,10 @@ public interface ConverterProvider { /** * Provide a converter that can convert between <T> and * <U> types. + * + * @return The converter for <T, U>, or null + * if no such converter could be provided, in case of which jOOQ's + * {@link DefaultConverterProvider} applies. */ Converter provide(Class tType, Class uType); } diff --git a/jOOQ/src/main/java/org/jooq/impl/AbstractRecord.java b/jOOQ/src/main/java/org/jooq/impl/AbstractRecord.java index 806398cd9e..578331ad32 100644 --- a/jOOQ/src/main/java/org/jooq/impl/AbstractRecord.java +++ b/jOOQ/src/main/java/org/jooq/impl/AbstractRecord.java @@ -41,6 +41,7 @@ package org.jooq.impl; import static java.util.Arrays.asList; import static org.jooq.conf.SettingsTools.updatablePrimaryKeys; import static org.jooq.impl.Tools.EMPTY_FIELD; +import static org.jooq.impl.Tools.converterOrFail; import static org.jooq.impl.Tools.embeddedFields; import static org.jooq.impl.Tools.indexOrFail; import static org.jooq.impl.Tools.isEmbeddable; @@ -254,7 +255,7 @@ abstract class AbstractRecord extends AbstractStore implements Record { @Override public final T get(Field field, Class type) { - return (T) Tools.converter(this, field.getType(), (Class) type).from(get(field)); + return (T) converterOrFail(this, field.getType(), (Class) type).from(get(field)); } @Override @@ -269,7 +270,7 @@ abstract class AbstractRecord extends AbstractStore implements Record { @Override public final T get(int index, Class type) { - return (T) Tools.converter(this, field(safeIndex(index)).getType(), (Class) type).from(get(index)); + return (T) converterOrFail(this, field(safeIndex(index)).getType(), (Class) type).from(get(index)); } @Override diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultConverterProvider.java b/jOOQ/src/main/java/org/jooq/impl/DefaultConverterProvider.java index dc5dfd740a..340a551ab0 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultConverterProvider.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultConverterProvider.java @@ -37,8 +37,27 @@ */ package org.jooq.impl; +import static org.jooq.tools.reflect.Reflect.wrapper; + +import java.io.File; +import java.net.URI; +import java.net.URL; +import java.sql.Struct; +import java.time.temporal.Temporal; +import java.util.Calendar; +import java.util.Collection; +import java.util.Optional; +import java.util.UUID; + +import javax.persistence.EnumType; + +// ... import org.jooq.Converter; import org.jooq.ConverterProvider; +import org.jooq.JSON; +import org.jooq.JSONB; +import org.jooq.Record; +import org.jooq.UDTRecord; import org.jooq.tools.Convert; /** @@ -50,32 +69,101 @@ public final class DefaultConverterProvider implements ConverterProvider { @Override public final Converter provide(final Class tType, final Class uType) { - return new Converter() { + Class tWrapper = wrapper(tType); + Class uWrapper = wrapper(uType); - /** - * Generated UID. - */ - private static final long serialVersionUID = 8011099590775678430L; + // TODO: [#10071] These checks are required to be able to return null in + // case this implementation cannot produce a Converter. + // It corresponds to a super set of what org.jooq.tools.Convert + // can do. There is certainly room for refactoring the two + // classes. + if (tWrapper == uWrapper + || uWrapper.isAssignableFrom(tWrapper) + || isCollection(tWrapper) && isCollection(uWrapper) + || tWrapper == Optional.class + || uWrapper == Optional.class + || uWrapper == String.class + || uWrapper == byte[].class + || Number.class.isAssignableFrom(uWrapper) // No fail-fast implemented yet! + || Boolean.class.isAssignableFrom(uWrapper) // No fail-fast implemented yet! + || Character.class.isAssignableFrom(uWrapper) + || uWrapper == URI.class && tWrapper == String.class + || uWrapper == URL.class && tWrapper == String.class + || uWrapper == File.class && tWrapper == String.class + || isDate(tWrapper) && isDate(uWrapper) + || isEnum(tWrapper) && isEnum(uWrapper) + || isUUID(tWrapper) && isUUID(uWrapper) + || isJSON(tWrapper) && isJSON(uWrapper) + || Record.class.isAssignableFrom(tWrapper) + || Struct.class.isAssignableFrom(tWrapper) && UDTRecord.class.isAssignableFrom(uWrapper) + ) { + return new Converter() { - @Override - public U from(T t) { - return Convert.convert(t, uType); - } + /** + * Generated UID. + */ + private static final long serialVersionUID = 8011099590775678430L; - @Override - public T to(U u) { - return Convert.convert(u, tType); - } + @Override + public U from(T t) { + return Convert.convert(t, uType); + } - @Override - public Class fromType() { - return tType; - } + @Override + public T to(U u) { + return Convert.convert(u, tType); + } - @Override - public Class toType() { - return uType; - } - }; + @Override + public Class fromType() { + return tType; + } + + @Override + public Class toType() { + return uType; + } + }; + } + else + return null; + } + + private final boolean isJSON(Class type) { + return type == JSON.class + || type == JSONB.class + || type == String.class; + } + + private final boolean isUUID(Class type) { + return type == String.class + || type == byte[].class + || type == UUID.class; + } + + private final boolean isEnum(Class type) { + return Enum.class.isAssignableFrom(type) + || type == String.class + || EnumType.class.isAssignableFrom(type); + } + + private final boolean isDate(Class type) { + return java.util.Date.class.isAssignableFrom(type) + || Calendar.class.isAssignableFrom(type) + + || Temporal.class.isAssignableFrom(type) + + || type == Long.class + || type == String.class; + } + + private final boolean isCollection(Class type) { + return type.isArray() + || Collection.class.isAssignableFrom(type) + + + + // [#3443] Conversion from Object[] to JDBC Array + || type == java.sql.Array.class; } } diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapper.java b/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapper.java index e5cb66cab8..916ec60fbf 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapper.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultRecordMapper.java @@ -337,11 +337,8 @@ public class DefaultRecordMapper implements RecordMapper extends AbstractCursor implements Re @Override public final List getValues(int fieldIndex, Class type) { List result = new ArrayList<>(size()); - Converter converter = Tools.converter(this, field(safeIndex(fieldIndex)).getType(), (Class) type); + Converter converter = converterOrFail(this, field(safeIndex(fieldIndex)).getType(), (Class) type); for (R record : this) result.add((U) converter.from(record.get(fieldIndex))); diff --git a/jOOQ/src/main/java/org/jooq/impl/Tools.java b/jOOQ/src/main/java/org/jooq/impl/Tools.java index 20528ec491..16481a1325 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Tools.java +++ b/jOOQ/src/main/java/org/jooq/impl/Tools.java @@ -1082,17 +1082,59 @@ final class Tools { } /** - * Get a converter from a {@link ConverterProvider}. + * Get a converter from a {@link ConverterProvider} or null if + * no converter could be provided. + */ + static final Converter converter(Configuration configuration, Class tType, Class uType) { + Converter result = configuration(configuration).converterProvider().provide(tType, uType); + + if (result == null) + result = CTX.configuration().converterProvider().provide(tType, uType); + + return result; + } + + /** + * Get a converter from a {@link ConverterProvider} or null if + * no converter could be provided. + */ + static final Converter converter(Scope scope, Class tType, Class uType) { + return converter(configuration(scope), tType, uType); + } + + /** + * Get a converter from a {@link ConverterProvider} or null if + * no converter could be provided. */ static final Converter converter(Attachable attachable, Class tType, Class uType) { - return configuration(attachable).converterProvider().provide(tType, uType); + return converter(configuration(attachable), tType, uType); + } + + /** + * Get a converter from a {@link ConverterProvider} or null if + * no converter could be provided. + */ + static final Converter converterOrFail(Configuration configuration, Class tType, Class uType) { + Converter result = converter(configuration, tType, uType); + + if (result == null) + throw new DataTypeException("No Converter found for types " + tType.getName() + " and " + uType.getName()); + + return result; } /** * Get a converter from a {@link ConverterProvider}. */ - static final Converter converter(Scope scope, Class tType, Class uType) { - return configuration(scope).converterProvider().provide(tType, uType); + static final Converter converterOrFail(Scope scope, Class tType, Class uType) { + return converterOrFail(configuration(scope), tType, uType); + } + + /** + * Get a converter from a {@link ConverterProvider}. + */ + static final Converter converterOrFail(Attachable attachable, Class tType, Class uType) { + return converterOrFail(configuration(attachable), tType, uType); } /**