Skip to content

Commit 43f4a54

Browse files
mp911demp911de
committed
Polishing.
Use Standard Exception handling for JDBC SQLException. Introduce easier and consistent mechanism to construct JdbcCustomConversions. Add tests and fix mocking setup. Fix typos. See #1828 Original pull request #2062
1 parent 002acbc commit 43f4a54

File tree

11 files changed

+352
-88
lines changed

11 files changed

+352
-88
lines changed

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

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

18+
import java.util.ArrayList;
19+
import java.util.Arrays;
1820
import java.util.Collection;
1921
import java.util.Collections;
2022
import java.util.List;
23+
import java.util.function.Consumer;
2124

25+
import org.springframework.core.convert.converter.Converter;
26+
import org.springframework.core.convert.converter.ConverterFactory;
27+
import org.springframework.core.convert.converter.GenericConverter;
2228
import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair;
29+
import org.springframework.data.convert.ConverterBuilder;
2330
import org.springframework.data.convert.CustomConversions;
2431
import org.springframework.data.jdbc.core.mapping.JdbcSimpleTypes;
32+
import org.springframework.data.relational.core.dialect.Dialect;
33+
import org.springframework.lang.Contract;
34+
import org.springframework.util.Assert;
2535

2636
/**
2737
* Value object to capture custom conversion. {@link JdbcCustomConversions} also act as factory for
@@ -50,10 +60,10 @@ public JdbcCustomConversions() {
5060
* Create a new {@link JdbcCustomConversions} instance registering the given converters and the default store
5161
* converters.
5262
*
53-
* @param converters must not be {@literal null}.
63+
* @param userConverters must not be {@literal null}.
5464
*/
55-
public JdbcCustomConversions(List<?> converters) {
56-
super(constructConverterConfiguration(converters));
65+
public JdbcCustomConversions(List<?> userConverters) {
66+
this(StoreConversions.of(JdbcSimpleTypes.HOLDER, STORE_CONVERTERS), userConverters);
5767
}
5868

5969
/**
@@ -63,12 +73,7 @@ public JdbcCustomConversions(List<?> converters) {
6373
* @since 2.3
6474
*/
6575
public JdbcCustomConversions(StoreConversions storeConversions, List<?> userConverters) {
66-
67-
super(new ConverterConfiguration( //
68-
storeConversions, //
69-
userConverters, //
70-
JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types //
71-
));
76+
super(JdbcConverterConfigurer.from(storeConversions).registerConverters(userConverters).createConfiguration());
7277
}
7378

7479
/**
@@ -82,15 +87,40 @@ public JdbcCustomConversions(ConverterConfiguration converterConfiguration) {
8287
super(converterConfiguration);
8388
}
8489

85-
private static ConverterConfiguration constructConverterConfiguration(List<?> converters) {
90+
/**
91+
* Create a new {@link JdbcCustomConversions} from the given {@link Dialect} and {@code converters}.
92+
*
93+
* @param dialect must not be {@literal null}.
94+
* @param converters must not be {@literal null}.
95+
* @return a new {@link JdbcCustomConversions} instance configured from the given dialect and configured converters.
96+
* @since 4.0
97+
*/
98+
public static JdbcCustomConversions of(Dialect dialect, Collection<?> converters) {
99+
100+
Assert.notNull(dialect, "Dialect must not be null");
101+
Assert.notNull(converters, "Converters must not be null");
86102

87-
return new ConverterConfiguration( //
88-
StoreConversions.of(JdbcSimpleTypes.HOLDER, STORE_CONVERTERS), //
89-
converters, //
90-
JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types //
91-
);
103+
return create(dialect, configurer -> configurer.registerConverters(converters));
92104
}
93105

106+
/**
107+
* Create a new {@link JdbcCustomConversions} instance using the given {@link Dialect} and
108+
* {@link JdbcConverterConfigurer} callback to configure converters.
109+
*
110+
* @param dialect the {@link Dialect} to use, must not be {@literal null}.
111+
* @param configurer the configurer callback to configure converters, must not be {@literal null}.
112+
* @return a new {@link JdbcCustomConversions} instance configured from the given dialect and configured converters.
113+
*/
114+
public static JdbcCustomConversions create(Dialect dialect, Consumer<JdbcConverterConfigurer> configurer) {
115+
116+
Assert.notNull(dialect, "Dialect must not be null");
117+
Assert.notNull(configurer, "JdbcConverterConfigurer Consumer must not be null");
118+
119+
JdbcConverterConfigurer converterConfigurer = JdbcConverterConfigurer.from(dialect);
120+
configurer.accept(converterConfigurer);
121+
122+
return new JdbcCustomConversions(converterConfigurer.createConfiguration());
123+
}
94124

95125
/**
96126
* Obtain a read only copy of default store converters.
@@ -118,4 +148,112 @@ private static boolean isDateTimeApiConversion(ConvertiblePair cp) {
118148
private static boolean excludeConversionsBetweenDateAndJsr310Types(ConvertiblePair cp) {
119149
return !isDateTimeApiConversion(cp);
120150
}
151+
152+
/**
153+
* {@link JdbcConverterConfigurer} encapsulates creation of
154+
* {@link org.springframework.data.convert.CustomConversions.ConverterConfiguration} with JDBC specifics.
155+
*
156+
* @author Mark Paluch
157+
* @since 4.0
158+
*/
159+
public static class JdbcConverterConfigurer {
160+
161+
private final StoreConversions storeConversions;
162+
private final List<Object> customConverters = new ArrayList<>();
163+
164+
private JdbcConverterConfigurer(StoreConversions storeConversions) {
165+
this.storeConversions = storeConversions;
166+
}
167+
168+
/**
169+
* Create a {@link JdbcConverterConfigurer} using the provided {@code dialect} and our own codecs for JSR-310 types.
170+
*
171+
* @param dialect must not be {@literal null}.
172+
* @return
173+
*/
174+
static JdbcConverterConfigurer from(Dialect dialect) {
175+
176+
List<Object> converters = new ArrayList<>();
177+
converters.addAll(dialect.getConverters());
178+
converters.addAll(JdbcCustomConversions.storeConverters());
179+
180+
StoreConversions storeConversions = StoreConversions.of(JdbcSimpleTypes.HOLDER, converters);
181+
182+
return new JdbcConverterConfigurer(storeConversions);
183+
}
184+
185+
/**
186+
* Create a {@link JdbcConverterConfigurer} using the provided {@code storeConversions}.
187+
*
188+
* @param storeConversions must not be {@literal null}.
189+
* @return
190+
*/
191+
static JdbcConverterConfigurer from(StoreConversions storeConversions) {
192+
return new JdbcConverterConfigurer(storeConversions);
193+
}
194+
195+
/**
196+
* Add a custom {@link Converter} implementation.
197+
*
198+
* @param converter must not be {@literal null}.
199+
* @return this.
200+
*/
201+
@Contract("_ -> this")
202+
public JdbcConverterConfigurer registerConverter(Converter<?, ?> converter) {
203+
204+
Assert.notNull(converter, "Converter must not be null");
205+
customConverters.add(converter);
206+
return this;
207+
}
208+
209+
/**
210+
* Add {@link Converter converters}, {@link ConverterFactory factories}, {@link ConverterBuilder.ConverterAware
211+
* converter-aware objects}, and {@link GenericConverter generic converters}.
212+
*
213+
* @param converters must not be {@literal null} nor contain {@literal null} values.
214+
* @return this.
215+
*/
216+
@Contract("_ -> this")
217+
public JdbcConverterConfigurer registerConverters(Object... converters) {
218+
return registerConverters(Arrays.asList(converters));
219+
}
220+
221+
/**
222+
* Add {@link Converter converters}, {@link ConverterFactory factories}, {@link ConverterBuilder.ConverterAware
223+
* converter-aware objects}, and {@link GenericConverter generic converters}.
224+
*
225+
* @param converters must not be {@literal null} nor contain {@literal null} values.
226+
* @return this.
227+
*/
228+
@Contract("_ -> this")
229+
public JdbcConverterConfigurer registerConverters(Collection<?> converters) {
230+
231+
Assert.notNull(converters, "Converters must not be null");
232+
Assert.noNullElements(converters, "Converters must not be null nor contain null values");
233+
234+
customConverters.addAll(converters);
235+
return this;
236+
}
237+
238+
/**
239+
* Add a custom {@link ConverterFactory} implementation.
240+
*
241+
* @param converterFactory must not be {@literal null}.
242+
* @return this.
243+
*/
244+
@Contract("_ -> this")
245+
public JdbcConverterConfigurer registerConverterFactory(ConverterFactory<?, ?> converterFactory) {
246+
247+
Assert.notNull(converterFactory, "ConverterFactory must not be null");
248+
customConverters.add(converterFactory);
249+
return this;
250+
}
251+
252+
ConverterConfiguration createConfiguration() {
253+
return new ConverterConfiguration(storeConversions, this.customConverters,
254+
JdbcCustomConversions::excludeConversionsBetweenDateAndJsr310Types);
255+
}
256+
257+
}
258+
121259
}

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

Lines changed: 50 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,14 @@
2020
import java.sql.SQLException;
2121
import java.sql.SQLType;
2222
import java.util.Iterator;
23+
import java.util.List;
2324
import java.util.Map;
2425
import java.util.Optional;
2526
import java.util.function.Function;
2627

27-
import org.apache.commons.logging.Log;
28-
import org.apache.commons.logging.LogFactory;
2928
import org.springframework.context.ApplicationContextAware;
3029
import org.springframework.core.convert.converter.Converter;
31-
import org.springframework.dao.NonTransientDataAccessException;
30+
import org.springframework.dao.DataAccessException;
3231
import org.springframework.data.convert.CustomConversions;
3332
import org.springframework.data.jdbc.core.mapping.AggregateReference;
3433
import org.springframework.data.jdbc.core.mapping.JdbcValue;
@@ -46,6 +45,10 @@
4645
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
4746
import org.springframework.data.relational.domain.RowDocument;
4847
import org.springframework.data.util.TypeInformation;
48+
import org.springframework.jdbc.UncategorizedSQLException;
49+
import org.springframework.jdbc.support.SQLErrorCodeSQLExceptionTranslator;
50+
import org.springframework.jdbc.support.SQLExceptionSubclassTranslator;
51+
import org.springframework.jdbc.support.SQLExceptionTranslator;
4952
import org.springframework.lang.Nullable;
5053
import org.springframework.util.Assert;
5154

@@ -67,9 +70,9 @@
6770
*/
6871
public class MappingJdbcConverter extends MappingRelationalConverter implements JdbcConverter, ApplicationContextAware {
6972

70-
private static final Log LOG = LogFactory.getLog(MappingJdbcConverter.class);
7173
private static final Converter<Iterable<?>, Map<?, ?>> ITERABLE_OF_ENTRY_TO_MAP_CONVERTER = new IterableOfEntryToMapConverter();
7274

75+
private SQLExceptionTranslator exceptionTranslator = new SQLExceptionSubclassTranslator();
7376
private final JdbcTypeFactory typeFactory;
7477
private final RelationResolver relationResolver;
7578

@@ -111,6 +114,15 @@ public MappingJdbcConverter(RelationalMappingContext context, RelationResolver r
111114
this.relationResolver = relationResolver;
112115
}
113116

117+
/**
118+
* Set the exception translator for this instance. Defaults to a {@link SQLErrorCodeSQLExceptionTranslator}.
119+
*
120+
* @see SQLExceptionSubclassTranslator
121+
*/
122+
public void setExceptionTranslator(SQLExceptionTranslator exceptionTranslator) {
123+
this.exceptionTranslator = exceptionTranslator;
124+
}
125+
114126
@Nullable
115127
private Class<?> getEntityColumnType(TypeInformation<?> type) {
116128

@@ -189,59 +201,44 @@ public Object readValue(@Nullable Object value, TypeInformation<?> targetType) {
189201
return null;
190202
}
191203

192-
TypeInformation<?> originalTargetType = targetType;
193-
value = readJdbcArray(value);
194-
targetType = determineNestedTargetType(targetType);
204+
value = potentiallyUnwrapArray(value);
195205

196-
return possiblyReadToAggregateReference(getPotentiallyConvertedSimpleRead(value, targetType), originalTargetType);
206+
if (AggregateReference.class.isAssignableFrom(targetType.getType())) {
207+
208+
List<TypeInformation<?>> types = targetType.getTypeArguments();
209+
TypeInformation<?> idType = types.get(1);
210+
if (value instanceof AggregateReference<?, ?> ref) {
211+
return AggregateReference.to(readValue(ref.getId(), idType));
212+
} else {
213+
return AggregateReference.to(readValue(value, idType));
214+
}
215+
}
216+
217+
return getPotentiallyConvertedSimpleRead(value, targetType);
197218
}
198219

199220
/**
200221
* Unwrap a Jdbc array, if such a value is provided
201222
*/
202-
private Object readJdbcArray(Object value) {
223+
private Object potentiallyUnwrapArray(Object value) {
203224

204225
if (value instanceof Array array) {
205226
try {
206227
return array.getArray();
207228
} catch (SQLException e) {
208-
throw new FailedToAccessJdbcArrayException(e);
229+
throw translateException("Array.getArray()", null, e);
209230
}
210231
}
211232

212233
return value;
213234
}
214235

215-
/**
216-
* Determine the id type of an {@link AggregateReference} that the rest of the conversion infrastructure needs to use
217-
* as a conversion target.
218-
*/
219-
private TypeInformation<?> determineNestedTargetType(TypeInformation<?> ultimateTargetType) {
220-
221-
if (AggregateReference.class.isAssignableFrom(ultimateTargetType.getType())) {
222-
// the id type of a AggregateReference
223-
return ultimateTargetType.getTypeArguments().get(1);
224-
}
225-
return ultimateTargetType;
226-
}
227-
228-
/**
229-
* Convert value to an {@link AggregateReference} if that is specified by the parameter targetType.
230-
*/
231-
private Object possiblyReadToAggregateReference(Object value, TypeInformation<?> targetType) {
232-
233-
if (AggregateReference.class.isAssignableFrom(targetType.getType())) {
234-
return AggregateReference.to(value);
235-
}
236-
return value;
237-
}
238-
239236
@Nullable
240237
@Override
241238
protected Object getPotentiallyConvertedSimpleWrite(Object value, TypeInformation<?> type) {
242239

243-
if (value instanceof AggregateReference<?, ?> aggregateReference) {
244-
return writeValue(aggregateReference.getId(), type);
240+
if (value instanceof AggregateReference<?, ?> ref) {
241+
return writeValue(ref.getId(), type);
245242
}
246243

247244
return super.getPotentiallyConvertedSimpleWrite(value, type);
@@ -277,11 +274,11 @@ private boolean canWriteAsJdbcValue(@Nullable Object value) {
277274
public JdbcValue writeJdbcValue(@Nullable Object value, TypeInformation<?> columnType, SQLType sqlType) {
278275

279276
TypeInformation<?> targetType = canWriteAsJdbcValue(value) ? TypeInformation.of(JdbcValue.class) : columnType;
280-
281-
if (value instanceof AggregateReference<?, ?> aggregateReference) {
282-
return writeJdbcValue(aggregateReference.getId(), columnType, sqlType);
277+
278+
if (value instanceof AggregateReference<?, ?> ref) {
279+
return writeJdbcValue(ref.getId(), columnType, sqlType);
283280
}
284-
281+
285282
Object convertedValue = writeValue(value, targetType);
286283

287284
if (convertedValue instanceof JdbcValue result) {
@@ -306,7 +303,7 @@ public JdbcValue writeJdbcValue(@Nullable Object value, TypeInformation<?> colum
306303

307304
return JdbcValue.of(convertedValue, JDBCType.BINARY);
308305
}
309-
306+
310307
return JdbcValue.of(convertedValue, sqlType);
311308
}
312309

@@ -340,10 +337,18 @@ protected RelationalPropertyValueProvider newValueProvider(RowDocumentAccessor d
340337
return super.newValueProvider(documentAccessor, evaluator, context);
341338
}
342339

343-
private static class FailedToAccessJdbcArrayException extends NonTransientDataAccessException {
344-
public FailedToAccessJdbcArrayException(SQLException e) {
345-
super("Failed to read array", e);
346-
}
340+
/**
341+
* Translate the given {@link SQLException} into a generic {@link DataAccessException}.
342+
*
343+
* @param task readable text describing the task being attempted
344+
* @param sql the SQL query or update that caused the problem (can be {@code null})
345+
* @param ex the offending {@code SQLException}
346+
* @return a DataAccessException wrapping the {@code SQLException} (never {@code null})
347+
*/
348+
private DataAccessException translateException(String task, @org.jspecify.annotations.Nullable String sql,
349+
SQLException ex) {
350+
DataAccessException dae = exceptionTranslator.translate(task, sql, ex);
351+
return (dae != null ? dae : new UncategorizedSQLException(task, sql, ex));
347352
}
348353

349354
/**

0 commit comments

Comments
 (0)