Do not convert from LocalDateTime to Timestamp by default.

Most supported databases don't need that conversion.

Db2 does need it and gets it through JdbcDb2Dialect.

As part of the effort it became obvious that the filtering for conversions between Date and JSR310 data types was broken.
It is fixed now, which required a dedicated reading conversion from LocalDateTime to Date for JdbcMySqlDialect.

Closes #974
Original pull request: #985.

Author: schauder

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcColumnTypes.java

@@ -16,6 +16,7 @@
 package org.springframework.data.jdbc.core.convert;
 
 import java.sql.Timestamp;
+import java.time.LocalDateTime;
 import java.time.OffsetDateTime;
 import java.time.ZonedDateTime;
 import java.time.temporal.Temporal;
@@ -54,6 +55,7 @@ public Class<?> resolvePrimitiveType(Class<?> type) {
 		javaToDbType.put(Enum.class, String.class);
 		javaToDbType.put(ZonedDateTime.class, String.class);
 		javaToDbType.put(OffsetDateTime.class, OffsetDateTime.class);
+		javaToDbType.put(LocalDateTime.class, LocalDateTime.class);
 		javaToDbType.put(Temporal.class, Timestamp.class);
 	}
 

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java

@@ -15,11 +15,9 @@
  */
 package org.springframework.data.jdbc.core.convert;
 
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
-import java.util.function.Predicate;
 
 import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair;
 import org.springframework.data.convert.CustomConversions;
@@ -38,7 +36,8 @@
  */
 public class JdbcCustomConversions extends CustomConversions {
 
-	private static final Collection<Object> STORE_CONVERTERS = Collections.unmodifiableCollection(Jsr310TimestampBasedConverters.getConvertersToRegister());
+	private static final Collection<Object> STORE_CONVERTERS = Collections
+			.unmodifiableCollection(Jsr310TimestampBasedConverters.getConvertersToRegister());
 	private static final StoreConversions STORE_CONVERSIONS = StoreConversions.of(JdbcSimpleTypes.HOLDER,
 			STORE_CONVERTERS);
 
@@ -50,21 +49,33 @@ public JdbcCustomConversions() {
 	}
 
 	/**
-	 * Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store converters.
+	 * Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store
+	 * converters.
 	 *
 	 * @param converters must not be {@literal null}.
 	 */
 	public JdbcCustomConversions(List<?> converters) {
-		super(new ConverterConfiguration(STORE_CONVERSIONS, converters, JdbcCustomConversions::isDateTimeApiConversion));
+
+		super(new ConverterConfiguration( //
+				STORE_CONVERSIONS, //
+				converters, //
+				JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types //
+		));
 	}
 
 	/**
-	 * Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store converters.
+	 * Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store
+	 * converters.
 	 *
 	 * @since 2.3
 	 */
 	public JdbcCustomConversions(StoreConversions storeConversions, List<?> userConverters) {
-		super(new ConverterConfiguration(storeConversions, userConverters, JdbcCustomConversions::isDateTimeApiConversion));
+
+		super(new ConverterConfiguration( //
+				storeConversions, //
+				userConverters, //
+				JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types //
+		));
 	}
 
 	/**
@@ -98,6 +109,10 @@ private static boolean isDateTimeApiConversion(ConvertiblePair cp) {
 			return cp.getSourceType().getTypeName().startsWith("java.time.");
 		}
 
-		return true;
+		return false;
+	}
+
+	private static boolean excludeConversionsBetweenDateAndJsr310Types(ConvertiblePair cp) {
+		return !isDateTimeApiConversion(cp);
 	}
 }

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/Jsr310TimestampBasedConverters.java

@@ -45,22 +45,21 @@
  * @author Jens Schauder
  * @since 2.2
  */
-abstract class Jsr310TimestampBasedConverters {
-
-	private static final List<Class<?>> CLASSES = Arrays.asList(LocalDateTime.class, LocalDate.class, LocalTime.class,
-			Instant.class, ZoneId.class, Duration.class, Period.class);
+public abstract class Jsr310TimestampBasedConverters {
 
 	/**
-	 * Returns the converters to be registered. Will only return converters in case we're running on Java 8.
+	 * Returns the converters to be registered.
+	 *
+	 * Note that the {@link LocalDateTimeToTimestampConverter} is not included, since many database don't need that conversion.
+	 * Databases that do need it, should include it in the conversions offered by their respective dialect.
 	 *
-	 * @return
+	 * @return a collection of converters. Guaranteed to be not {@literal null}.
 	 */
 	public static Collection<Converter<?, ?>> getConvertersToRegister() {
 
 		List<Converter<?, ?>> converters = new ArrayList<>(8);
 
 		converters.add(TimestampToLocalDateTimeConverter.INSTANCE);
-		converters.add(LocalDateTimeToTimestampConverter.INSTANCE);
 		converters.add(TimestampToLocalDateConverter.INSTANCE);
 		converters.add(LocalDateToTimestampConverter.INSTANCE);
 		converters.add(TimestampToLocalTimeConverter.INSTANCE);

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/dialect/JdbcDb2Dialect.java

@@ -23,6 +23,7 @@
 
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.convert.WritingConverter;
+import org.springframework.data.jdbc.core.convert.Jsr310TimestampBasedConverters;
 import org.springframework.data.relational.core.dialect.Db2Dialect;
 
 /**
@@ -43,6 +44,7 @@ public Collection<Object> getConverters() {
 
 		List<Object> converters = new ArrayList<>(super.getConverters());
 		converters.add(OffsetDateTimeToTimestampConverter.INSTANCE);
+		converters.add(Jsr310TimestampBasedConverters.LocalDateTimeToTimestampConverter.INSTANCE);
 
 		return converters;
 	}

Diff for: spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/dialect/JdbcMySqlDialect.java

@@ -15,17 +15,23 @@
  */
 package org.springframework.data.jdbc.core.dialect;
 
+import static java.time.ZoneId.*;
+
 import java.sql.JDBCType;
+import java.time.LocalDateTime;
 import java.time.OffsetDateTime;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Date;
 
 import org.springframework.core.convert.converter.Converter;
+import org.springframework.data.convert.ReadingConverter;
 import org.springframework.data.convert.WritingConverter;
 import org.springframework.data.jdbc.core.convert.JdbcValue;
 import org.springframework.data.relational.core.dialect.Db2Dialect;
 import org.springframework.data.relational.core.dialect.MySqlDialect;
 import org.springframework.data.relational.core.sql.IdentifierProcessing;
+import org.springframework.lang.NonNull;
 
 /**
  * {@link Db2Dialect} that registers JDBC specific converters.
@@ -47,6 +53,7 @@ public Collection<Object> getConverters() {
 
 		ArrayList<Object> converters = new ArrayList<>(super.getConverters());
 		converters.add(OffsetDateTimeToTimestampJdbcValueConverter.INSTANCE);
+		converters.add(LocalDateTimeToDateConverter.INSTANCE);
 
 		return converters;
 	}
@@ -61,4 +68,16 @@ public JdbcValue convert(OffsetDateTime source) {
 			return JdbcValue.of(source, JDBCType.TIMESTAMP);
 		}
 	}
+
+	@ReadingConverter
+	enum LocalDateTimeToDateConverter implements Converter<LocalDateTime, Date> {
+
+		INSTANCE;
+
+		@NonNull
+		@Override
+		public Date convert(LocalDateTime source) {
+			return Date.from(source.atZone(systemDefault()).toInstant());
+		}
+	}
 }

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationUnitTests.java

@@ -17,6 +17,7 @@
 
 import static java.util.Collections.*;
 import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.SoftAssertions.*;
 import static org.mockito.Mockito.*;
 
 import java.util.ArrayList;
@@ -26,7 +27,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import org.assertj.core.api.SoftAssertions;
 import org.junit.jupiter.api.Test;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
@@ -87,7 +87,7 @@ public void simpleReference() {
 
 		executor.execute(aggregateChange);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 
 			softly.assertThat(entity.rootId).isEqualTo(1);
 			softly.assertThat(entity.single.id).isEqualTo(2);
@@ -107,7 +107,7 @@ public void listReference() {
 
 		executor.execute(aggregateChange);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 
 			softly.assertThat(entity.rootId).isEqualTo(1);
 			softly.assertThat(entity.contentList).extracting(c -> c.id).containsExactly(2, 3);
@@ -171,7 +171,7 @@ public void setIdForDeepReferenceElementList() {
 
 		executor.execute(aggregateChange);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 
 			softly.assertThat(entity.rootId).isEqualTo(1);
 			softly.assertThat(entity.single.id).isEqualTo(2);
@@ -198,7 +198,7 @@ public void setIdForDeepElementSetElementSet() {
 
 		executor.execute(aggregateChange);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 
 			softly.assertThat(entity.rootId).isEqualTo(1);
 			softly.assertThat(entity.contentSet) //
@@ -233,7 +233,7 @@ public void setIdForDeepElementListSingleReference() {
 
 		executor.execute(aggregateChange);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 
 			softly.assertThat(entity.rootId).isEqualTo(1);
 			softly.assertThat(entity.contentList) //
@@ -267,7 +267,7 @@ public void setIdForDeepElementListElementList() {
 
 		executor.execute(aggregateChange);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 
 			softly.assertThat(entity.rootId).isEqualTo(1);
 			softly.assertThat(entity.contentList) //
@@ -305,7 +305,7 @@ public void setIdForDeepElementMapElementMap() {
 
 		executor.execute(aggregateChange);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 
 			softly.assertThat(entity.rootId).isEqualTo(1);
 			softly.assertThat(entity.contentMap.entrySet()) //

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java

@@ -17,6 +17,7 @@
 
 import static java.util.Collections.*;
 import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.SoftAssertions.*;
 import static org.springframework.data.jdbc.testing.TestDatabaseFeatures.Feature.*;
 import static org.springframework.test.context.TestExecutionListeners.MergeMode.*;
 
@@ -664,7 +665,7 @@ public void shouldDeleteChainOfListsWithoutIds() {
 		NoIdListChain4 saved = template.save(createNoIdTree());
 		template.deleteById(saved.four, NoIdListChain4.class);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 
 			softly.assertThat(count("NO_ID_LIST_CHAIN4")).describedAs("Chain4 elements got deleted").isEqualTo(0);
 			softly.assertThat(count("NO_ID_LIST_CHAIN3")).describedAs("Chain3 elements got deleted").isEqualTo(0);
@@ -691,7 +692,7 @@ public void shouldDeleteChainOfMapsWithoutIds() {
 		NoIdMapChain4 saved = template.save(createNoIdMapTree());
 		template.deleteById(saved.four, NoIdMapChain4.class);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 
 			softly.assertThat(count("NO_ID_MAP_CHAIN4")).describedAs("Chain4 elements got deleted").isEqualTo(0);
 			softly.assertThat(count("NO_ID_MAP_CHAIN3")).describedAs("Chain3 elements got deleted").isEqualTo(0);

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverterUnitTests.java

@@ -16,6 +16,7 @@
 package org.springframework.data.jdbc.core.convert;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.SoftAssertions.*;
 import static org.mockito.Mockito.*;
 
 import lombok.Data;
@@ -26,6 +27,7 @@
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.LocalTime;
+import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
 import java.util.Date;
@@ -69,12 +71,14 @@ public void testTargetTypesForPropertyType() {
 		SoftAssertions softly = new SoftAssertions();
 
 		checkTargetType(softly, entity, "someEnum", String.class);
-		checkTargetType(softly, entity, "localDateTime", Timestamp.class);
+		checkTargetType(softly, entity, "localDateTime", LocalDateTime.class);
 		checkTargetType(softly, entity, "localDate", Timestamp.class);
 		checkTargetType(softly, entity, "localTime", Timestamp.class);
+		checkTargetType(softly, entity, "zonedDateTime", String.class);
+		checkTargetType(softly, entity, "offsetDateTime", OffsetDateTime.class);
 		checkTargetType(softly, entity, "instant", Timestamp.class);
 		checkTargetType(softly, entity, "date", Date.class);
-		checkTargetType(softly, entity, "zonedDateTime", String.class);
+		checkTargetType(softly, entity, "timestamp", Timestamp.class);
 		checkTargetType(softly, entity, "uuid", UUID.class);
 
 		softly.assertAll();
@@ -116,7 +120,7 @@ void conversionOfDateLikeValueAndBackYieldsOriginalValue() {
 
 		RelationalPersistentEntity<?> persistentEntity = context.getRequiredPersistentEntity(DummyEntity.class);
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 			LocalDateTime testLocalDateTime = LocalDateTime.of(2001, 2, 3, 4, 5, 6, 123456789);
 			checkConversionToTimestampAndBack(softly, persistentEntity, "localDateTime", testLocalDateTime);
 			checkConversionToTimestampAndBack(softly, persistentEntity, "localDate", LocalDate.of(2001, 2, 3));
@@ -165,9 +169,11 @@ private static class DummyEntity {
 		private final LocalDateTime localDateTime;
 		private final LocalDate localDate;
 		private final LocalTime localTime;
+		private final ZonedDateTime zonedDateTime;
+		private final OffsetDateTime offsetDateTime;
 		private final Instant instant;
 		private final Date date;
-		private final ZonedDateTime zonedDateTime;
+		private final Timestamp timestamp;
 		private final AggregateReference<DummyEntity, Long> reference;
 		private final UUID uuid;
 

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java

@@ -18,6 +18,7 @@
 import static java.util.Arrays.*;
 import static java.util.Collections.*;
 import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.SoftAssertions.*;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
 
@@ -43,12 +44,10 @@
 
 import javax.naming.OperationNotSupportedException;
 
-import org.assertj.core.api.SoftAssertions;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentMatchers;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
-
 import org.springframework.data.annotation.Id;
 import org.springframework.data.annotation.PersistenceConstructor;
 import org.springframework.data.annotation.Transient;
@@ -1217,7 +1216,7 @@ private static class Fixture<T> {
 
 		public void assertOn(T result) {
 
-			SoftAssertions.assertSoftly(softly -> {
+			assertSoftly(softly -> {
 				expectations.forEach(expectation -> {
 
 					softly.assertThat(expectation.extractor.apply(result)).describedAs("From column: " + expectation.sourceColumn)

Diff for: spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierUnitTests.java

@@ -16,6 +16,7 @@
 package org.springframework.data.jdbc.core.convert;
 
 import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.SoftAssertions.*;
 import static org.springframework.data.relational.core.sql.SqlIdentifier.*;
 
 import java.util.ArrayList;
@@ -25,7 +26,6 @@
 import java.util.Map;
 
 import org.assertj.core.api.Assertions;
-import org.assertj.core.api.SoftAssertions;
 import org.junit.jupiter.api.Test;
 import org.springframework.data.relational.core.sql.IdentifierProcessing;
 import org.springframework.data.relational.core.sql.SqlIdentifier;
@@ -125,7 +125,7 @@ public void identifierPartsCanBeAccessedByString() {
 
 		Map<SqlIdentifier, Object> map = id.toMap();
 
-		SoftAssertions.assertSoftly(softly -> {
+		assertSoftly(softly -> {
 			softly.assertThat(map.get("aName")).describedAs("aName").isEqualTo("one");
 			softly.assertThat(map.get("Other")).describedAs("Other").isEqualTo("two");
 			softly.assertThat(map.get("other")).describedAs("other").isNull();