Skip to content

Commit 8652418

Browse files
schaudermp911de
schauder
authored and
mp911de
committed
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 spring-projects#974 Original pull request: spring-projects#985.
1 parent 2c413a2 commit 8652418

22 files changed

+217
-67
lines changed

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcColumnTypes.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.data.jdbc.core.convert;
1717

1818
import java.sql.Timestamp;
19+
import java.time.LocalDateTime;
1920
import java.time.OffsetDateTime;
2021
import java.time.ZonedDateTime;
2122
import java.time.temporal.Temporal;
@@ -54,6 +55,7 @@ public Class<?> resolvePrimitiveType(Class<?> type) {
5455
javaToDbType.put(Enum.class, String.class);
5556
javaToDbType.put(ZonedDateTime.class, String.class);
5657
javaToDbType.put(OffsetDateTime.class, OffsetDateTime.class);
58+
javaToDbType.put(LocalDateTime.class, LocalDateTime.class);
5759
javaToDbType.put(Temporal.class, Timestamp.class);
5860
}
5961

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcCustomConversions.java

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,9 @@
1515
*/
1616
package org.springframework.data.jdbc.core.convert;
1717

18-
import java.util.Arrays;
1918
import java.util.Collection;
2019
import java.util.Collections;
2120
import java.util.List;
22-
import java.util.function.Predicate;
2321

2422
import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair;
2523
import org.springframework.data.convert.CustomConversions;
@@ -38,7 +36,8 @@
3836
*/
3937
public class JdbcCustomConversions extends CustomConversions {
4038

41-
private static final Collection<Object> STORE_CONVERTERS = Collections.unmodifiableCollection(Jsr310TimestampBasedConverters.getConvertersToRegister());
39+
private static final Collection<Object> STORE_CONVERTERS = Collections
40+
.unmodifiableCollection(Jsr310TimestampBasedConverters.getConvertersToRegister());
4241
private static final StoreConversions STORE_CONVERSIONS = StoreConversions.of(JdbcSimpleTypes.HOLDER,
4342
STORE_CONVERTERS);
4443

@@ -50,21 +49,33 @@ public JdbcCustomConversions() {
5049
}
5150

5251
/**
53-
* Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store converters.
52+
* Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store
53+
* converters.
5454
*
5555
* @param converters must not be {@literal null}.
5656
*/
5757
public JdbcCustomConversions(List<?> converters) {
58-
super(new ConverterConfiguration(STORE_CONVERSIONS, converters, JdbcCustomConversions::isDateTimeApiConversion));
58+
59+
super(new ConverterConfiguration( //
60+
STORE_CONVERSIONS, //
61+
converters, //
62+
JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types //
63+
));
5964
}
6065

6166
/**
62-
* Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store converters.
67+
* Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store
68+
* converters.
6369
*
6470
* @since 2.3
6571
*/
6672
public JdbcCustomConversions(StoreConversions storeConversions, List<?> userConverters) {
67-
super(new ConverterConfiguration(storeConversions, userConverters, JdbcCustomConversions::isDateTimeApiConversion));
73+
74+
super(new ConverterConfiguration( //
75+
storeConversions, //
76+
userConverters, //
77+
JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types //
78+
));
6879
}
6980

7081
/**
@@ -98,6 +109,10 @@ private static boolean isDateTimeApiConversion(ConvertiblePair cp) {
98109
return cp.getSourceType().getTypeName().startsWith("java.time.");
99110
}
100111

101-
return true;
112+
return false;
113+
}
114+
115+
private static boolean excludeConversionsBetweenDateAndJsr310Types(ConvertiblePair cp) {
116+
return !isDateTimeApiConversion(cp);
102117
}
103118
}

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/Jsr310TimestampBasedConverters.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,22 +45,21 @@
4545
* @author Jens Schauder
4646
* @since 2.2
4747
*/
48-
abstract class Jsr310TimestampBasedConverters {
49-
50-
private static final List<Class<?>> CLASSES = Arrays.asList(LocalDateTime.class, LocalDate.class, LocalTime.class,
51-
Instant.class, ZoneId.class, Duration.class, Period.class);
48+
public abstract class Jsr310TimestampBasedConverters {
5249

5350
/**
54-
* Returns the converters to be registered. Will only return converters in case we're running on Java 8.
51+
* Returns the converters to be registered.
52+
*
53+
* Note that the {@link LocalDateTimeToTimestampConverter} is not included, since many database don't need that conversion.
54+
* Databases that do need it, should include it in the conversions offered by their respective dialect.
5555
*
56-
* @return
56+
* @return a collection of converters. Guaranteed to be not {@literal null}.
5757
*/
5858
public static Collection<Converter<?, ?>> getConvertersToRegister() {
5959

6060
List<Converter<?, ?>> converters = new ArrayList<>(8);
6161

6262
converters.add(TimestampToLocalDateTimeConverter.INSTANCE);
63-
converters.add(LocalDateTimeToTimestampConverter.INSTANCE);
6463
converters.add(TimestampToLocalDateConverter.INSTANCE);
6564
converters.add(LocalDateToTimestampConverter.INSTANCE);
6665
converters.add(TimestampToLocalTimeConverter.INSTANCE);

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/dialect/JdbcDb2Dialect.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import org.springframework.core.convert.converter.Converter;
2525
import org.springframework.data.convert.WritingConverter;
26+
import org.springframework.data.jdbc.core.convert.Jsr310TimestampBasedConverters;
2627
import org.springframework.data.relational.core.dialect.Db2Dialect;
2728

2829
/**
@@ -43,6 +44,7 @@ public Collection<Object> getConverters() {
4344

4445
List<Object> converters = new ArrayList<>(super.getConverters());
4546
converters.add(OffsetDateTimeToTimestampConverter.INSTANCE);
47+
converters.add(Jsr310TimestampBasedConverters.LocalDateTimeToTimestampConverter.INSTANCE);
4648

4749
return converters;
4850
}

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/dialect/JdbcMySqlDialect.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,23 @@
1515
*/
1616
package org.springframework.data.jdbc.core.dialect;
1717

18+
import static java.time.ZoneId.*;
19+
1820
import java.sql.JDBCType;
21+
import java.time.LocalDateTime;
1922
import java.time.OffsetDateTime;
2023
import java.util.ArrayList;
2124
import java.util.Collection;
25+
import java.util.Date;
2226

2327
import org.springframework.core.convert.converter.Converter;
28+
import org.springframework.data.convert.ReadingConverter;
2429
import org.springframework.data.convert.WritingConverter;
2530
import org.springframework.data.jdbc.core.convert.JdbcValue;
2631
import org.springframework.data.relational.core.dialect.Db2Dialect;
2732
import org.springframework.data.relational.core.dialect.MySqlDialect;
2833
import org.springframework.data.relational.core.sql.IdentifierProcessing;
34+
import org.springframework.lang.NonNull;
2935

3036
/**
3137
* {@link Db2Dialect} that registers JDBC specific converters.
@@ -47,6 +53,7 @@ public Collection<Object> getConverters() {
4753

4854
ArrayList<Object> converters = new ArrayList<>(super.getConverters());
4955
converters.add(OffsetDateTimeToTimestampJdbcValueConverter.INSTANCE);
56+
converters.add(LocalDateTimeToDateConverter.INSTANCE);
5057

5158
return converters;
5259
}
@@ -61,4 +68,16 @@ public JdbcValue convert(OffsetDateTime source) {
6168
return JdbcValue.of(source, JDBCType.TIMESTAMP);
6269
}
6370
}
71+
72+
@ReadingConverter
73+
enum LocalDateTimeToDateConverter implements Converter<LocalDateTime, Date> {
74+
75+
INSTANCE;
76+
77+
@NonNull
78+
@Override
79+
public Date convert(LocalDateTime source) {
80+
return Date.from(source.atZone(systemDefault()).toInstant());
81+
}
82+
}
6483
}

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AggregateChangeIdGenerationUnitTests.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import static java.util.Collections.*;
1919
import static org.assertj.core.api.Assertions.*;
20+
import static org.assertj.core.api.SoftAssertions.*;
2021
import static org.mockito.Mockito.*;
2122

2223
import java.util.ArrayList;
@@ -26,7 +27,6 @@
2627
import java.util.Map;
2728
import java.util.Set;
2829

29-
import org.assertj.core.api.SoftAssertions;
3030
import org.junit.jupiter.api.Test;
3131
import org.mockito.invocation.InvocationOnMock;
3232
import org.mockito.stubbing.Answer;
@@ -87,7 +87,7 @@ public void simpleReference() {
8787

8888
executor.execute(aggregateChange);
8989

90-
SoftAssertions.assertSoftly(softly -> {
90+
assertSoftly(softly -> {
9191

9292
softly.assertThat(entity.rootId).isEqualTo(1);
9393
softly.assertThat(entity.single.id).isEqualTo(2);
@@ -107,7 +107,7 @@ public void listReference() {
107107

108108
executor.execute(aggregateChange);
109109

110-
SoftAssertions.assertSoftly(softly -> {
110+
assertSoftly(softly -> {
111111

112112
softly.assertThat(entity.rootId).isEqualTo(1);
113113
softly.assertThat(entity.contentList).extracting(c -> c.id).containsExactly(2, 3);
@@ -171,7 +171,7 @@ public void setIdForDeepReferenceElementList() {
171171

172172
executor.execute(aggregateChange);
173173

174-
SoftAssertions.assertSoftly(softly -> {
174+
assertSoftly(softly -> {
175175

176176
softly.assertThat(entity.rootId).isEqualTo(1);
177177
softly.assertThat(entity.single.id).isEqualTo(2);
@@ -198,7 +198,7 @@ public void setIdForDeepElementSetElementSet() {
198198

199199
executor.execute(aggregateChange);
200200

201-
SoftAssertions.assertSoftly(softly -> {
201+
assertSoftly(softly -> {
202202

203203
softly.assertThat(entity.rootId).isEqualTo(1);
204204
softly.assertThat(entity.contentSet) //
@@ -233,7 +233,7 @@ public void setIdForDeepElementListSingleReference() {
233233

234234
executor.execute(aggregateChange);
235235

236-
SoftAssertions.assertSoftly(softly -> {
236+
assertSoftly(softly -> {
237237

238238
softly.assertThat(entity.rootId).isEqualTo(1);
239239
softly.assertThat(entity.contentList) //
@@ -267,7 +267,7 @@ public void setIdForDeepElementListElementList() {
267267

268268
executor.execute(aggregateChange);
269269

270-
SoftAssertions.assertSoftly(softly -> {
270+
assertSoftly(softly -> {
271271

272272
softly.assertThat(entity.rootId).isEqualTo(1);
273273
softly.assertThat(entity.contentList) //
@@ -305,7 +305,7 @@ public void setIdForDeepElementMapElementMap() {
305305

306306
executor.execute(aggregateChange);
307307

308-
SoftAssertions.assertSoftly(softly -> {
308+
assertSoftly(softly -> {
309309

310310
softly.assertThat(entity.rootId).isEqualTo(1);
311311
softly.assertThat(entity.contentMap.entrySet()) //

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/JdbcAggregateTemplateIntegrationTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import static java.util.Collections.*;
1919
import static org.assertj.core.api.Assertions.*;
20+
import static org.assertj.core.api.SoftAssertions.*;
2021
import static org.springframework.data.jdbc.testing.TestDatabaseFeatures.Feature.*;
2122
import static org.springframework.test.context.TestExecutionListeners.MergeMode.*;
2223

@@ -664,7 +665,7 @@ public void shouldDeleteChainOfListsWithoutIds() {
664665
NoIdListChain4 saved = template.save(createNoIdTree());
665666
template.deleteById(saved.four, NoIdListChain4.class);
666667

667-
SoftAssertions.assertSoftly(softly -> {
668+
assertSoftly(softly -> {
668669

669670
softly.assertThat(count("NO_ID_LIST_CHAIN4")).describedAs("Chain4 elements got deleted").isEqualTo(0);
670671
softly.assertThat(count("NO_ID_LIST_CHAIN3")).describedAs("Chain3 elements got deleted").isEqualTo(0);
@@ -691,7 +692,7 @@ public void shouldDeleteChainOfMapsWithoutIds() {
691692
NoIdMapChain4 saved = template.save(createNoIdMapTree());
692693
template.deleteById(saved.four, NoIdMapChain4.class);
693694

694-
SoftAssertions.assertSoftly(softly -> {
695+
assertSoftly(softly -> {
695696

696697
softly.assertThat(count("NO_ID_MAP_CHAIN4")).describedAs("Chain4 elements got deleted").isEqualTo(0);
697698
softly.assertThat(count("NO_ID_MAP_CHAIN3")).describedAs("Chain3 elements got deleted").isEqualTo(0);

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/BasicJdbcConverterUnitTests.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.data.jdbc.core.convert;
1717

1818
import static org.assertj.core.api.Assertions.*;
19+
import static org.assertj.core.api.SoftAssertions.*;
1920
import static org.mockito.Mockito.*;
2021

2122
import lombok.Data;
@@ -26,6 +27,7 @@
2627
import java.time.LocalDate;
2728
import java.time.LocalDateTime;
2829
import java.time.LocalTime;
30+
import java.time.OffsetDateTime;
2931
import java.time.ZoneOffset;
3032
import java.time.ZonedDateTime;
3133
import java.util.Date;
@@ -69,12 +71,14 @@ public void testTargetTypesForPropertyType() {
6971
SoftAssertions softly = new SoftAssertions();
7072

7173
checkTargetType(softly, entity, "someEnum", String.class);
72-
checkTargetType(softly, entity, "localDateTime", Timestamp.class);
74+
checkTargetType(softly, entity, "localDateTime", LocalDateTime.class);
7375
checkTargetType(softly, entity, "localDate", Timestamp.class);
7476
checkTargetType(softly, entity, "localTime", Timestamp.class);
77+
checkTargetType(softly, entity, "zonedDateTime", String.class);
78+
checkTargetType(softly, entity, "offsetDateTime", OffsetDateTime.class);
7579
checkTargetType(softly, entity, "instant", Timestamp.class);
7680
checkTargetType(softly, entity, "date", Date.class);
77-
checkTargetType(softly, entity, "zonedDateTime", String.class);
81+
checkTargetType(softly, entity, "timestamp", Timestamp.class);
7882
checkTargetType(softly, entity, "uuid", UUID.class);
7983

8084
softly.assertAll();
@@ -116,7 +120,7 @@ void conversionOfDateLikeValueAndBackYieldsOriginalValue() {
116120

117121
RelationalPersistentEntity<?> persistentEntity = context.getRequiredPersistentEntity(DummyEntity.class);
118122

119-
SoftAssertions.assertSoftly(softly -> {
123+
assertSoftly(softly -> {
120124
LocalDateTime testLocalDateTime = LocalDateTime.of(2001, 2, 3, 4, 5, 6, 123456789);
121125
checkConversionToTimestampAndBack(softly, persistentEntity, "localDateTime", testLocalDateTime);
122126
checkConversionToTimestampAndBack(softly, persistentEntity, "localDate", LocalDate.of(2001, 2, 3));
@@ -165,9 +169,11 @@ private static class DummyEntity {
165169
private final LocalDateTime localDateTime;
166170
private final LocalDate localDate;
167171
private final LocalTime localTime;
172+
private final ZonedDateTime zonedDateTime;
173+
private final OffsetDateTime offsetDateTime;
168174
private final Instant instant;
169175
private final Date date;
170-
private final ZonedDateTime zonedDateTime;
176+
private final Timestamp timestamp;
171177
private final AggregateReference<DummyEntity, Long> reference;
172178
private final UUID uuid;
173179

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/EntityRowMapperUnitTests.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static java.util.Arrays.*;
1919
import static java.util.Collections.*;
2020
import static org.assertj.core.api.Assertions.*;
21+
import static org.assertj.core.api.SoftAssertions.*;
2122
import static org.mockito.ArgumentMatchers.*;
2223
import static org.mockito.Mockito.*;
2324

@@ -43,12 +44,10 @@
4344

4445
import javax.naming.OperationNotSupportedException;
4546

46-
import org.assertj.core.api.SoftAssertions;
4747
import org.junit.jupiter.api.Test;
4848
import org.mockito.ArgumentMatchers;
4949
import org.mockito.invocation.InvocationOnMock;
5050
import org.mockito.stubbing.Answer;
51-
5251
import org.springframework.data.annotation.Id;
5352
import org.springframework.data.annotation.PersistenceConstructor;
5453
import org.springframework.data.annotation.Transient;
@@ -1217,7 +1216,7 @@ private static class Fixture<T> {
12171216

12181217
public void assertOn(T result) {
12191218

1220-
SoftAssertions.assertSoftly(softly -> {
1219+
assertSoftly(softly -> {
12211220
expectations.forEach(expectation -> {
12221221

12231222
softly.assertThat(expectation.extractor.apply(result)).describedAs("From column: " + expectation.sourceColumn)

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/IdentifierUnitTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.data.jdbc.core.convert;
1717

1818
import static org.assertj.core.api.Assertions.*;
19+
import static org.assertj.core.api.SoftAssertions.*;
1920
import static org.springframework.data.relational.core.sql.SqlIdentifier.*;
2021

2122
import java.util.ArrayList;
@@ -25,7 +26,6 @@
2526
import java.util.Map;
2627

2728
import org.assertj.core.api.Assertions;
28-
import org.assertj.core.api.SoftAssertions;
2929
import org.junit.jupiter.api.Test;
3030
import org.springframework.data.relational.core.sql.IdentifierProcessing;
3131
import org.springframework.data.relational.core.sql.SqlIdentifier;
@@ -125,7 +125,7 @@ public void identifierPartsCanBeAccessedByString() {
125125

126126
Map<SqlIdentifier, Object> map = id.toMap();
127127

128-
SoftAssertions.assertSoftly(softly -> {
128+
assertSoftly(softly -> {
129129
softly.assertThat(map.get("aName")).describedAs("aName").isEqualTo("one");
130130
softly.assertThat(map.get("Other")).describedAs("Other").isEqualTo("two");
131131
softly.assertThat(map.get("other")).describedAs("other").isNull();

0 commit comments

Comments
 (0)