Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address #1089 issue. Change ZonedDateTime conversion mechanism #1119

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.springframework.core.convert.converter.Converter;
import org.springframework.data.convert.CustomConversions;
import org.springframework.data.jdbc.core.mapping.AggregateReference;
import org.springframework.data.jdbc.repository.config.DialectResolver;
import org.springframework.data.jdbc.support.JdbcUtil;
import org.springframework.data.mapping.PersistentPropertyAccessor;
import org.springframework.data.mapping.PersistentPropertyPath;
Expand All @@ -51,6 +52,7 @@
import org.springframework.data.relational.core.sql.IdentifierProcessing;
import org.springframework.data.util.ClassTypeInformation;
import org.springframework.data.util.TypeInformation;
import org.springframework.lang.NonNull;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

Expand All @@ -64,6 +66,8 @@
* @author Jens Schauder
* @author Christoph Strobl
* @author Myeonghyeon Lee
* @author Mikhail Polivakha
*
* @see MappingContext
* @see SimpleTypeHolder
* @see CustomConversions
Expand Down Expand Up @@ -170,7 +174,14 @@ private Class<?> getReferenceColumnType(RelationalPersistentProperty property) {
*/
@Override
public int getSqlType(RelationalPersistentProperty property) {
return JdbcUtil.sqlTypeFor(getColumnType(property));
if (typeFactory instanceof DefaultJdbcTypeFactory) {
return JdbcUtil.sqlTypeFor(
getColumnType(property),
DialectResolver.getDialect(((DefaultJdbcTypeFactory) typeFactory).getOperations())
);
} else {
return JdbcUtil.sqlTypeFor(getColumnType(property));
}
}

/*
Expand All @@ -196,7 +207,7 @@ private Class<?> doGetColumnType(RelationalPersistentProperty property) {
}
}

Class<?> componentColumnType = JdbcColumnTypes.INSTANCE.resolvePrimitiveType(property.getActualType());
Class<?> componentColumnType = resolvePropertyPrimitiveType(property);

while (componentColumnType.isArray()) {
componentColumnType = componentColumnType.getComponentType();
Expand All @@ -209,6 +220,14 @@ private Class<?> doGetColumnType(RelationalPersistentProperty property) {
return componentColumnType;
}

private Class<?> resolvePropertyPrimitiveType(RelationalPersistentProperty property) {
if (typeFactory instanceof DefaultJdbcTypeFactory) {
return JdbcColumnTypes.INSTANCE.resolvePrimitiveType(property.getActualType(), DialectResolver.getDialect(((DefaultJdbcTypeFactory) typeFactory).getOperations()));
} else {
return JdbcColumnTypes.INSTANCE.resolvePrimitiveType(property.getActualType());
}
}

/*
* (non-Javadoc)
* @see org.springframework.data.relational.core.conversion.RelationalConverter#readValue(java.lang.Object, org.springframework.data.util.TypeInformation)
Expand Down Expand Up @@ -240,21 +259,6 @@ public Object readValue(@Nullable Object value, TypeInformation<?> type) {
return super.readValue(value, type);
}

/*
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can even remove this API, because parent method also perform the same null check. Also if we assume that any external client also use this API, then they will not notice anything, because in this case they will just call parent writeValue directly.

* (non-Javadoc)
* @see org.springframework.data.relational.core.conversion.RelationalConverter#writeValue(java.lang.Object, org.springframework.data.util.TypeInformation)
*/
@Override
@Nullable
public Object writeValue(@Nullable Object value, TypeInformation<?> type) {

if (value == null) {
return null;
}

return super.writeValue(value, type);
}

private boolean canWriteAsJdbcValue(@Nullable Object value) {

if (value == null) {
Expand Down Expand Up @@ -285,6 +289,7 @@ private boolean canWriteAsJdbcValue(@Nullable Object value) {
* (non-Javadoc)
* @see org.springframework.data.jdbc.core.convert.JdbcConverter#writeValue(java.lang.Object, java.lang.Class, int)
*/
@NonNull
@Override
public JdbcValue writeJdbcValue(@Nullable Object value, Class<?> columnType, int sqlType) {

Expand Down Expand Up @@ -329,8 +334,12 @@ private JdbcValue tryToConvertToJdbcValue(@Nullable Object value) {

@Override
public <T> T mapRow(RelationalPersistentEntity<T> entity, ResultSet resultSet, Object key) {
return new ReadingContext<T>(new PersistentPropertyPathExtension(getMappingContext(), entity),
new ResultSetAccessor(resultSet), Identifier.empty(), key).mapRow();
return new ReadingContext<T>(
new PersistentPropertyPathExtension(getMappingContext(), entity),
new ResultSetAccessor(resultSet),
Identifier.empty(),
key
).mapRow();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.springframework.dao.OptimisticLockingFailureException;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.data.jdbc.repository.config.DialectResolver;
import org.springframework.data.jdbc.support.JdbcUtil;
import org.springframework.data.mapping.PersistentProperty;
import org.springframework.data.mapping.PersistentPropertyAccessor;
Expand Down Expand Up @@ -67,6 +68,8 @@
* @author Myeonghyeon Lee
* @author Yunyoung LEE
* @author Radim Tlusty
* @author Mikhail Polivakha
*
* @since 1.1
*/
public class DefaultDataAccessStrategy implements DataAccessStrategy {
Expand Down Expand Up @@ -545,13 +548,26 @@ private IdentifierProcessing getIdentifierProcessing() {
private void addConvertedPropertyValue(SqlIdentifierParameterSource parameterSource,
RelationalPersistentProperty property, @Nullable Object value, SqlIdentifier name) {

addConvertedValue(parameterSource, value, name, converter.getColumnType(property), converter.getSqlType(property));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JdbcConverter#getSqlType will invoke getColumnType internaly. This is inefficient, since we already called converter#getColumnType() for this RelationalPersistentProperty

final Class<?> javaColumnType = converter.getColumnType(property);
addConvertedValue(
parameterSource,
value,
name,
javaColumnType,
JdbcUtil.sqlTypeFor(javaColumnType, DialectResolver.getDialect(operations.getJdbcOperations()))
);
}

private void addConvertedPropertyValue(SqlIdentifierParameterSource parameterSource, SqlIdentifier name, Object value,
Class<?> javaType) {

addConvertedValue(parameterSource, value, name, javaType, JdbcUtil.sqlTypeFor(javaType));
addConvertedValue(
parameterSource,
value,
name,
javaType,
JdbcUtil.sqlTypeFor(javaType, DialectResolver.getDialect(operations.getJdbcOperations()))
);
}

private void addConvertedValue(SqlIdentifierParameterSource parameterSource, @Nullable Object value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import java.sql.JDBCType;

import org.springframework.data.jdbc.core.dialect.JdbcArrayColumns;
import org.springframework.data.jdbc.repository.config.DialectResolver;
import org.springframework.data.jdbc.support.JdbcUtil;
import org.springframework.jdbc.core.ConnectionCallback;
import org.springframework.jdbc.core.JdbcOperations;
import org.springframework.lang.NonNull;
import org.springframework.util.Assert;

/**
Expand All @@ -30,6 +32,8 @@
*
* @author Jens Schauder
* @author Mark Paluch
* @author Mikhail Polivakha
*
* @since 1.1
*/
public class DefaultJdbcTypeFactory implements JdbcTypeFactory {
Expand Down Expand Up @@ -68,11 +72,15 @@ public Array createArray(Object[] value) {

Class<?> componentType = arrayColumns.getArrayType(value.getClass());

JDBCType jdbcType = JdbcUtil.jdbcTypeFor(componentType);
JDBCType jdbcType = JdbcUtil.jdbcTypeFor(componentType, DialectResolver.getDialect(operations));
Assert.notNull(jdbcType, () -> String.format("Couldn't determine JDBCType for %s", componentType));
String typeName = arrayColumns.getArrayTypeName(jdbcType);

return operations.execute((ConnectionCallback<Array>) c -> c.createArrayOf(typeName, value));
}

@NonNull
public JdbcOperations getOperations() {
return operations;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension;
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.util.Assert;

/**
* Maps a {@link ResultSet} to an entity of type {@code T}, including entities referenced. This {@link RowMapper} might
Expand Down Expand Up @@ -50,6 +51,9 @@ public EntityRowMapper(PersistentPropertyPathExtension path, JdbcConverter conve

public EntityRowMapper(RelationalPersistentEntity<T> entity, JdbcConverter converter) {

Assert.notNull(entity, "relationalPersistentEntity must not be null!");
Assert.notNull(converter, "jdbcConverter must not be null!");

this.entity = entity;
this.path = null;
this.converter = converter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,29 @@
import java.util.Date;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Optional;

import org.springframework.data.relational.core.dialect.Dialect;
import org.springframework.util.ClassUtils;

/**
* Utility that determines the necessary type conversions between Java types used in the domain model and types
* compatible with JDBC drivers.
*
* @author Jens Schauder
* @author Mikhail Polivakha
* @since 2.0
*/
public enum JdbcColumnTypes {

INSTANCE {

/**
* @deprecated because this method will resolve the target column type regardless of the
* current client {@link Dialect} - it will search only in common types mappings.
* When possible, please, use {@link #resolvePrimitiveType(Class, Dialect)}
*/
@Deprecated
@SuppressWarnings({ "unchecked", "rawtypes" })
public Class<?> resolvePrimitiveType(Class<?> type) {

Expand All @@ -46,18 +55,33 @@ public Class<?> resolvePrimitiveType(Class<?> type) {
.findFirst() //
.orElseGet(() -> (Class) ClassUtils.resolvePrimitiveIfNecessary(type));
}

@SuppressWarnings({ "unchecked", "rawtypes", "deprecation" })
public Class<?> resolvePrimitiveType(Class<?> type, Dialect dialect) {
return Optional
.ofNullable(dialect.getCustomJdbcColumnsMappings().get(type))
.orElse((Class) this.resolvePrimitiveType(type));
}
};

private static final Map<Class<?>, Class<?>> javaToDbType = new LinkedHashMap<>();

static {

javaToDbType.put(Enum.class, String.class);
javaToDbType.put(ZonedDateTime.class, String.class);
javaToDbType.put(ZonedDateTime.class, ZonedDateTime.class);
javaToDbType.put(OffsetDateTime.class, OffsetDateTime.class);
javaToDbType.put(LocalDateTime.class, LocalDateTime.class);
javaToDbType.put(Temporal.class, Timestamp.class);
}

/**
* @deprecated because this method will resolve the target column type regardless of the
* current client {@link Dialect} - it will search only in common types mappings.
* When possible, please, use {@link #resolvePrimitiveType(Class, Dialect)}
*/
@Deprecated
public abstract Class<?> resolvePrimitiveType(Class<?> type);

public abstract Class<?> resolvePrimitiveType(Class<?> type, Dialect dialect);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
import java.sql.ResultSet;

import org.springframework.data.relational.core.conversion.RelationalConverter;
import org.springframework.data.relational.core.dialect.Dialect;
import org.springframework.data.relational.core.mapping.PersistentPropertyPathExtension;
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
import org.springframework.data.util.TypeInformation;
import org.springframework.lang.NonNull;
import org.springframework.lang.Nullable;

/**
Expand All @@ -42,6 +44,7 @@ public interface JdbcConverter extends RelationalConverter {
* @param sqlType the type constant from {@link java.sql.Types} to be used if non is specified by a converter.
* @return The converted value wrapped in a {@link JdbcValue}. Guaranteed to be not {@literal null}.
*/
@NonNull
JdbcValue writeJdbcValue(@Nullable Object value, Class<?> type, int sqlType);

/**
Expand Down Expand Up @@ -72,7 +75,7 @@ public interface JdbcConverter extends RelationalConverter {
* top-level array type (e.g. {@code String[][]} returns {@code String[]}).
*
* @return a {@link Class} that is suitable for usage with JDBC drivers.
* @see org.springframework.data.jdbc.support.JdbcUtil#sqlTypeFor(Class)
* @see org.springframework.data.jdbc.support.JdbcUtil#sqlTypeFor(Class, Dialect)
* @since 2.0
*/
Class<?> getColumnType(RelationalPersistentProperty property);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.OffsetDateTime;
import java.time.Period;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -43,6 +46,7 @@
*
* @see org.springframework.data.convert.Jsr310Converters
* @author Jens Schauder
* @author Mikhail Polivakha
* @since 2.2
*/
public abstract class Jsr310TimestampBasedConverters {
Expand All @@ -66,6 +70,8 @@ public abstract class Jsr310TimestampBasedConverters {
converters.add(LocalTimeToTimestampConverter.INSTANCE);
converters.add(TimestampToInstantConverter.INSTANCE);
converters.add(InstantToTimestampConverter.INSTANCE);
converters.add(TimestampToZonedDateTimeConverter.INSTANCE);
converters.add(TimestampToOffsetDateTimeConverter.INSTANCE);

return converters;
}
Expand Down Expand Up @@ -165,4 +171,26 @@ public Timestamp convert(Instant source) {
return Timestamp.from(source);
}
}

@ReadingConverter
public enum TimestampToZonedDateTimeConverter implements Converter<Timestamp, ZonedDateTime> {

INSTANCE;

@Override
public ZonedDateTime convert(Timestamp source) {
return ZonedDateTime.ofInstant(source.toInstant(), ZoneId.of("UTC"));
}
}

@ReadingConverter
public enum TimestampToOffsetDateTimeConverter implements Converter<Timestamp, OffsetDateTime> {

INSTANCE;

@Override
public OffsetDateTime convert(Timestamp source) {
return OffsetDateTime.ofInstant(source.toInstant(), ZoneId.of("UTC"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@
* @since 2.7
*/
@ReadingConverter
public enum H2TimestampWithTimeZoneToOffsetDateTimeConverter
implements Converter<TimestampWithTimeZone, OffsetDateTime> {
public enum H2TimestampWithTimeZoneToOffsetDateTimeConverter implements Converter<TimestampWithTimeZone, OffsetDateTime> {

INSTANCE;

Expand Down
Loading