Skip to content

Commit 373d2d3

Browse files
schauderschauder
committed
DATAJDBC-327 - Address review comments.
1 parent ee68f1d commit 373d2d3

File tree

14 files changed

+128
-111
lines changed

14 files changed

+128
-111
lines changed

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,24 +22,24 @@
2222
import java.util.Arrays;
2323
import java.util.HashMap;
2424
import java.util.HashSet;
25-
import java.util.LinkedHashMap;
2625
import java.util.List;
2726
import java.util.Map;
27+
import java.util.function.Predicate;
2828

2929
import org.springframework.dao.DataRetrievalFailureException;
3030
import org.springframework.dao.EmptyResultDataAccessException;
3131
import org.springframework.dao.InvalidDataAccessApiUsageException;
3232
import org.springframework.data.jdbc.core.convert.JdbcConverter;
33-
import org.springframework.data.jdbc.core.convert.JdbcTypeAware;
33+
import org.springframework.data.jdbc.core.convert.JdbcValue;
3434
import org.springframework.data.jdbc.support.JdbcUtil;
35+
import org.springframework.data.mapping.PersistentProperty;
3536
import org.springframework.data.mapping.PersistentPropertyAccessor;
3637
import org.springframework.data.mapping.PersistentPropertyPath;
3738
import org.springframework.data.mapping.PropertyHandler;
3839
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
3940
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
4041
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
4142
import org.springframework.data.relational.domain.Identifier;
42-
import org.springframework.data.util.ClassTypeInformation;
4343
import org.springframework.jdbc.core.RowMapper;
4444
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
4545
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
@@ -104,7 +104,7 @@ public <T> Object insert(T instance, Class<T> domainType, Identifier identifier)
104104
KeyHolder holder = new GeneratedKeyHolder();
105105
RelationalPersistentEntity<T> persistentEntity = getRequiredPersistentEntity(domainType);
106106

107-
MapSqlParameterSource parameterSource = getParameterSource(instance, persistentEntity, "", true);
107+
MapSqlParameterSource parameterSource = getParameterSource(instance, persistentEntity, "", PersistentProperty::isIdProperty);
108108

109109
identifier.forEach((name, value, type) -> addConvertedPropertyValue(parameterSource, name, value, type));
110110

@@ -134,7 +134,7 @@ public <S> boolean update(S instance, Class<S> domainType) {
134134
RelationalPersistentEntity<S> persistentEntity = getRequiredPersistentEntity(domainType);
135135

136136
return operations.update(sql(domainType).getUpdate(),
137-
getParameterSource(instance, persistentEntity, "", false)) != 0;
137+
getParameterSource(instance, persistentEntity, "", property -> false)) != 0;
138138
}
139139

140140
/*
@@ -289,15 +289,15 @@ public <T> boolean existsById(Object id, Class<T> domainType) {
289289
}
290290

291291
private <S, T> MapSqlParameterSource getParameterSource(S instance, RelationalPersistentEntity<S> persistentEntity,
292-
String prefix, boolean skipId) {
292+
String prefix, Predicate<RelationalPersistentProperty> skipProperty) {
293293

294294
MapSqlParameterSource parameters = new MapSqlParameterSource();
295295

296296
PersistentPropertyAccessor<S> propertyAccessor = persistentEntity.getPropertyAccessor(instance);
297297

298298
persistentEntity.doWithProperties((PropertyHandler<RelationalPersistentProperty>) property -> {
299299

300-
if (skipId && property.isIdProperty()) {
300+
if (skipProperty.test(property)) {
301301
return;
302302
}
303303
if (property.isEntity() && !property.isEmbedded()) {
@@ -309,7 +309,7 @@ private <S, T> MapSqlParameterSource getParameterSource(S instance, RelationalPe
309309
Object value = propertyAccessor.getProperty(property);
310310
RelationalPersistentEntity<?> embeddedEntity = context.getPersistentEntity(property.getType());
311311
MapSqlParameterSource additionalParameters = getParameterSource((T) value,
312-
(RelationalPersistentEntity<T>) embeddedEntity, prefix + property.getEmbeddedPrefix(), skipId);
312+
(RelationalPersistentEntity<T>) embeddedEntity, prefix + property.getEmbeddedPrefix(), skipProperty);
313313
parameters.addValues(additionalParameters.getValues());
314314
} else {
315315

@@ -390,48 +390,48 @@ private <T> MapSqlParameterSource createIdParameterSource(Object id, Class<T> do
390390
private void addConvertedPropertyValue(MapSqlParameterSource parameterSource, RelationalPersistentProperty property,
391391
Object value, String paramName) {
392392

393-
JdbcTypeAware jdbcTypeAware = converter.writeTypeAware( //
393+
JdbcValue jdbcValue = converter.writeJdbcValue( //
394394
value, //
395395
property.getColumnType(), //
396396
property.getSqlType() //
397397
);
398398

399-
parameterSource.addValue(paramName, jdbcTypeAware.getValue(), JdbcUtil.sqlTypeFor(jdbcTypeAware.getJdbcType()));
399+
parameterSource.addValue(paramName, jdbcValue.getValue(), JdbcUtil.sqlTypeFor(jdbcValue.getJdbcType()));
400400
}
401401

402402
private void addConvertedPropertyValue(MapSqlParameterSource parameterSource, String name, Object value,
403403
Class<?> type) {
404404

405-
JdbcTypeAware jdbcTypeAware = converter.writeTypeAware( //
405+
JdbcValue jdbcValue = converter.writeJdbcValue( //
406406
value, //
407407
type, //
408408
JdbcUtil.sqlTypeFor(type) //
409409
);
410410

411411
parameterSource.addValue( //
412412
name, //
413-
jdbcTypeAware.getValue(), //
414-
JdbcUtil.sqlTypeFor(jdbcTypeAware.getJdbcType()) //
413+
jdbcValue.getValue(), //
414+
JdbcUtil.sqlTypeFor(jdbcValue.getJdbcType()) //
415415
);
416416
}
417417

418418
private void addConvertedPropertyValuesAsList(MapSqlParameterSource parameterSource,
419419
RelationalPersistentProperty property, Iterable<?> values, String paramName) {
420420

421421
List<Object> convertedIds = new ArrayList<>();
422-
JdbcTypeAware jdbcTypeAware = null;
422+
JdbcValue jdbcValue = null;
423423
for (Object id : values) {
424424

425425
Class<?> columnType = property.getColumnType();
426426
int sqlType = property.getSqlType();
427427

428-
jdbcTypeAware = converter.writeTypeAware(id, columnType, sqlType);
429-
convertedIds.add(jdbcTypeAware.getValue());
428+
jdbcValue = converter.writeJdbcValue(id, columnType, sqlType);
429+
convertedIds.add(jdbcValue.getValue());
430430
}
431431

432-
Assert.notNull(jdbcTypeAware, "JdbcTypeAware must be not null at this point. Please report this as a bug.");
432+
Assert.notNull(jdbcValue, "JdbcValue must be not null at this point. Please report this as a bug.");
433433

434-
JDBCType jdbcType = jdbcTypeAware.getJdbcType();
434+
JDBCType jdbcType = jdbcValue.getJdbcType();
435435
int typeNumber = jdbcType == null ? JdbcUtils.TYPE_UNKNOWN : jdbcType.getVendorTypeNumber();
436436

437437
parameterSource.addValue(paramName, convertedIds, typeNumber);

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jdbc.core.convert;
17+
18+
import lombok.experimental.UtilityClass;
19+
20+
/**
21+
* A collection of utility methods for dealing with arrays.
22+
*
23+
* @author Jens Schauder
24+
*/
25+
@UtilityClass
26+
class ArrayUtil {
27+
28+
/**
29+
* Convertes an {@code Byte[]} into a {@code byte[]}
30+
* @param byteArray the array to be converted. Must not be {@literal null}.
31+
*
32+
* @return a {@code byte[]} of same size with the unboxed values of the input array. Guaranteed to be not {@literal null}.
33+
*/
34+
static Object toPrimitiveByteArray(Byte[] byteArray) {
35+
36+
byte[] bytes = new byte[byteArray.length];
37+
for (int i = 0; i < byteArray.length; i++) {
38+
bytes[i] = byteArray[i];
39+
}
40+
return bytes;
41+
}
42+
}

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

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,10 @@
1818
import java.sql.Array;
1919
import java.sql.JDBCType;
2020
import java.sql.SQLException;
21-
import java.util.Arrays;
21+
import java.util.Optional;
2222

2323
import org.slf4j.Logger;
2424
import org.slf4j.LoggerFactory;
25-
import org.springframework.core.convert.ConversionException;
2625
import org.springframework.core.convert.ConverterNotFoundException;
2726
import org.springframework.data.convert.CustomConversions;
2827
import org.springframework.data.jdbc.core.mapping.AggregateReference;
@@ -91,7 +90,7 @@ public BasicJdbcConverter(
9190
@Deprecated
9291
public BasicJdbcConverter(
9392
MappingContext<? extends RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> context) {
94-
this(context, JdbcTypeFactory.DUMMY_JDBC_TYPE_FACTORY);
93+
this(context, JdbcTypeFactory.unsupported());
9594
}
9695

9796
/**
@@ -105,7 +104,7 @@ public BasicJdbcConverter(
105104
public BasicJdbcConverter(
106105
MappingContext<? extends RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> context,
107106
CustomConversions conversions) {
108-
this(context, conversions, JdbcTypeFactory.DUMMY_JDBC_TYPE_FACTORY);
107+
this(context, conversions, JdbcTypeFactory.unsupported());
109108
}
110109

111110
/*
@@ -161,65 +160,65 @@ public Object writeValue(@Nullable Object value, TypeInformation<?> type) {
161160
return super.writeValue(value, type);
162161
}
163162

164-
@Override
165-
public boolean canWriteValue(@Nullable Object value, TypeInformation<?> type) {
163+
private boolean canWriteAsJdbcValue(@Nullable Object value) {
166164

167165
if (value == null) {
168166
return true;
169167
}
170168

171169
if (AggregateReference.class.isAssignableFrom(value.getClass())) {
172-
return canWriteValue(((AggregateReference) value).getId(), type);
170+
return canWriteAsJdbcValue(((AggregateReference) value).getId());
171+
}
172+
173+
RelationalPersistentEntity<?> persistentEntity = getMappingContext().getPersistentEntity(value.getClass());
174+
175+
if (persistentEntity != null) {
176+
177+
Object id = persistentEntity.getIdentifierAccessor(value).getIdentifier();
178+
return canWriteAsJdbcValue(id);
179+
}
180+
181+
if (value instanceof JdbcValue) {
182+
return true;
173183
}
174-
return super.canWriteValue(value, type);
184+
185+
Optional<Class<?>> customWriteTarget = getConversions().getCustomWriteTarget(value.getClass());
186+
return customWriteTarget.isPresent() && customWriteTarget.get().isAssignableFrom(JdbcValue.class);
175187
}
176188

177-
public JdbcTypeAware writeTypeAware(@Nullable Object value, Class<?> columnType, int sqlType) {
189+
public JdbcValue writeJdbcValue(@Nullable Object value, Class<?> columnType, int sqlType) {
178190

179-
JdbcTypeAware jdbcTypeAware = tryToConvertToJdbcTypeAware(value);
180-
if (jdbcTypeAware != null) {
181-
return jdbcTypeAware;
191+
JdbcValue jdbcValue = tryToConvertToJdbcValue(value);
192+
if (jdbcValue != null) {
193+
return jdbcValue;
182194
}
183195

184196
Object convertedValue = writeValue(value, ClassTypeInformation.from(columnType));
185197

186198
if (convertedValue == null || !convertedValue.getClass().isArray()) {
187-
return JdbcTypeAware.of(convertedValue, JdbcUtil.jdbcTypeFor(sqlType));
199+
return JdbcValue.of(convertedValue, JdbcUtil.jdbcTypeFor(sqlType));
188200
}
189201

190202
Class<?> componentType = convertedValue.getClass().getComponentType();
191203
if (componentType != byte.class && componentType != Byte.class) {
192-
return JdbcTypeAware.of(typeFactory.createArray((Object[]) convertedValue), JDBCType.ARRAY);
204+
return JdbcValue.of(typeFactory.createArray((Object[]) convertedValue), JDBCType.ARRAY);
193205
}
194206

195207
if (componentType == Byte.class) {
196-
Byte[] boxedBytes = (Byte[]) convertedValue;
197-
byte[] bytes = new byte[boxedBytes.length];
198-
for (int i = 0; i < boxedBytes.length; i++) {
199-
bytes[i] = boxedBytes[i];
200-
}
201-
convertedValue = bytes;
208+
convertedValue = ArrayUtil.toPrimitiveByteArray((Byte[]) convertedValue);
202209
}
203210

204-
return JdbcTypeAware.of(convertedValue, JDBCType.BINARY);
211+
return JdbcValue.of(convertedValue, JDBCType.BINARY);
205212

206213
}
207214

208-
private JdbcTypeAware tryToConvertToJdbcTypeAware(@Nullable Object value) {
209-
210-
JdbcTypeAware jdbcTypeAware = null;
211-
ClassTypeInformation<JdbcTypeAware> jdbcTypeAwareClassTypeInformation = ClassTypeInformation
212-
.from(JdbcTypeAware.class);
213-
if (canWriteValue(value, jdbcTypeAwareClassTypeInformation)) {
214-
215-
try {
216-
217-
jdbcTypeAware = (JdbcTypeAware) writeValue(value, jdbcTypeAwareClassTypeInformation);
215+
private JdbcValue tryToConvertToJdbcValue(@Nullable Object value) {
218216

219-
} catch (ConversionException __) {
220-
// a conversion may still fail
221-
}
217+
JdbcValue jdbcValue = null;
218+
if (canWriteAsJdbcValue(value)) {
219+
jdbcValue = (JdbcValue) writeValue(value, ClassTypeInformation.from(JdbcValue.class));
222220
}
223-
return jdbcTypeAware;
221+
222+
return jdbcValue;
224223
}
225224
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ public DefaultJdbcTypeFactory(JdbcOperations operations) {
4040
@Override
4141
public Array createArray(Object[] value) {
4242

43+
Assert.notNull(value, "Value must not be null.");
44+
4345
Class<?> componentType = innermostComponentType(value);
4446

4547
JDBCType jdbcType = JdbcUtil.jdbcTypeFor(componentType);

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@
2929
public interface JdbcConverter extends RelationalConverter {
3030

3131
/**
32-
* Convert a property value into a {@link JdbcTypeAware} that contains the converted value and information how to bind
32+
* Convert a property value into a {@link JdbcValue} that contains the converted value and information how to bind
3333
* it to JDBC parameters.
3434
*
3535
* @param value a value as it is used in the object model. May be {@code null}.
3636
* @param type {@link TypeInformation} into which the value is to be converted. Must not be {@code null}.
3737
* @param sqlType the type constant from {@link java.sql.Types} to be used if non is specified by a converter.
38-
* @return The converted value wrapped in a {@link JdbcTypeAware}. Guaranteed to be not {@literal null}.
38+
* @return The converted value wrapped in a {@link JdbcValue}. Guaranteed to be not {@literal null}.
3939
*/
40-
JdbcTypeAware writeTypeAware(@Nullable Object value, Class<?> type, int sqlType);
40+
JdbcValue writeJdbcValue(@Nullable Object value, Class<?> type, int sqlType);
4141
}

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@
2626
public interface JdbcTypeFactory {
2727

2828
/**
29-
* A dummy implementation used in places where a proper {@code JdbcTypeFactory} can not be provided but an instance
30-
* needs to be provided anyway, mostly for providing backward compatibility. Calling it will result in an exception.
31-
* The features normally supported by a {@link JdbcTypeFactory} will not work.
29+
* An implementation used in places where a proper {@code JdbcTypeFactory} can not be provided but an instance needs
30+
* to be provided anyway, mostly for providing backward compatibility. Calling it will result in an exception. The
31+
* features normally supported by a {@link JdbcTypeFactory} will not work.
3232
*/
33-
JdbcTypeFactory DUMMY_JDBC_TYPE_FACTORY = value -> {
34-
throw new UnsupportedOperationException("This JdbcTypeFactory does not support Array creation");
35-
};
33+
static JdbcTypeFactory unsupported() {
34+
35+
return value -> {
36+
throw new UnsupportedOperationException("This JdbcTypeFactory does not support Array creation");
37+
};
38+
}
3639

3740
/**
3841
* Converts the provided value in a {@link Array} instance.

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcTypeAware.java renamed to ‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/JdbcValue.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@
2121

2222
/**
2323
* Wraps a value with the JDBCType that should be used to pass it as a bind parameter to a
24-
* {@link java.sql.PreparedStatement}. Register a converter from any type to {@link JdbcTypeAware} in order to control
24+
* {@link java.sql.PreparedStatement}. Register a converter from any type to {@link JdbcValue} in order to control
2525
* the value and the {@link JDBCType} as which a value should get passed to the JDBC driver.
2626
*
2727
* @author Jens Schauder
2828
* @since 1.1
2929
*/
3030
@Value(staticConstructor = "of")
31-
public class JdbcTypeAware {
31+
public class JdbcValue {
3232

3333
Object value;
3434
JDBCType jdbcType;

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/config/JdbcConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public RelationalMappingContext jdbcMappingContext(Optional<NamingStrategy> nami
7676
@Bean
7777
public RelationalConverter relationalConverter(RelationalMappingContext mappingContext) {
7878

79-
return new BasicJdbcConverter(mappingContext, jdbcCustomConversions(), JdbcTypeFactory.DUMMY_JDBC_TYPE_FACTORY);
79+
return new BasicJdbcConverter(mappingContext, jdbcCustomConversions(), JdbcTypeFactory.unsupported());
8080
}
8181

8282
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public class BasicRelationalConverterAggregateReferenceUnitTests {
4343
ConversionService conversionService = new DefaultConversionService();
4444

4545
JdbcMappingContext context = new JdbcMappingContext();
46-
RelationalConverter converter = new BasicJdbcConverter(context, JdbcTypeFactory.DUMMY_JDBC_TYPE_FACTORY);
46+
RelationalConverter converter = new BasicJdbcConverter(context, JdbcTypeFactory.unsupported());
4747

4848
RelationalPersistentEntity<?> entity = context.getRequiredPersistentEntity(DummyEntity.class);
4949

0 commit comments

Comments
 (0)