Skip to content

Commit 329e256

Browse files
mipo256mipo256
committed
spring-projectsGH-2020 code review fixes
1 parent d92a729 commit 329e256

File tree

5 files changed

+97
-70
lines changed

5 files changed

+97
-70
lines changed

Diff for: ‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java

+79-23
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616
package org.springframework.data.jdbc.repository.query;
1717

18-
import static org.springframework.data.jdbc.repository.query.JdbcQueryExecution.*;
18+
import static org.springframework.data.jdbc.repository.query.JdbcQueryExecution.ResultProcessingConverter;
1919

2020
import java.lang.reflect.Array;
2121
import java.lang.reflect.Constructor;
@@ -32,22 +32,17 @@
3232
import org.springframework.beans.BeanInstantiationException;
3333
import org.springframework.beans.BeanUtils;
3434
import org.springframework.beans.factory.BeanFactory;
35-
import org.springframework.core.env.StandardEnvironment;
3635
import org.springframework.data.expression.ValueEvaluationContext;
37-
import org.springframework.data.expression.ValueExpressionParser;
3836
import org.springframework.data.jdbc.core.convert.JdbcColumnTypes;
3937
import org.springframework.data.jdbc.core.convert.JdbcConverter;
4038
import org.springframework.data.jdbc.core.mapping.JdbcValue;
4139
import org.springframework.data.jdbc.support.JdbcUtil;
42-
import org.springframework.data.relational.core.dialect.SqlTypeResolver;
4340
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
4441
import org.springframework.data.relational.repository.query.RelationalParameterAccessor;
4542
import org.springframework.data.relational.repository.query.RelationalParametersParameterAccessor;
46-
import org.springframework.data.repository.query.CachingValueExpressionDelegate;
4743
import org.springframework.data.repository.query.Parameter;
4844
import org.springframework.data.repository.query.Parameters;
4945
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
50-
import org.springframework.data.repository.query.QueryMethodValueEvaluationContextAccessor;
5146
import org.springframework.data.repository.query.ResultProcessor;
5247
import org.springframework.data.repository.query.ValueExpressionDelegate;
5348
import org.springframework.data.repository.query.ValueExpressionQueryRewriter;
@@ -92,15 +87,52 @@ public class StringBasedJdbcQuery extends AbstractJdbcQuery {
9287
private final CachedResultSetExtractorFactory cachedResultSetExtractorFactory;
9388
private final ValueExpressionDelegate delegate;
9489

90+
/**
91+
* Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext}
92+
* and {@link RowMapper}.
93+
*
94+
* @param queryMethod must not be {@literal null}.
95+
* @param operations must not be {@literal null}.
96+
* @param defaultRowMapper can be {@literal null} (only in case of a modifying query).
97+
* @deprecated since 3.4, use the constructors accepting {@link ValueExpressionDelegate} instead.
98+
*/
99+
@Deprecated(since = "3.4")
100+
public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
101+
@Nullable RowMapper<?> defaultRowMapper, JdbcConverter converter,
102+
QueryMethodEvaluationContextProvider evaluationContextProvider) {
103+
this(queryMethod.getRequiredQuery(), queryMethod, operations, result -> (RowMapper<Object>) defaultRowMapper,
104+
converter, evaluationContextProvider);
105+
}
106+
95107
/**
96108
* Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext}
97109
* and {@link RowMapperFactory}.
98110
*
99-
* @param queryMethod must not be {@literal null}.
100-
* @param operations must not be {@literal null}.
111+
* @param queryMethod must not be {@literal null}.
112+
* @param operations must not be {@literal null}.
113+
* @param rowMapperFactory must not be {@literal null}.
114+
* @param converter must not be {@literal null}.
115+
* @param evaluationContextProvider must not be {@literal null}.
116+
* @since 2.3
117+
* @deprecated use alternative constructor
118+
*/
119+
@Deprecated(since = "3.4")
120+
public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
121+
RowMapperFactory rowMapperFactory, JdbcConverter converter,
122+
QueryMethodEvaluationContextProvider evaluationContextProvider) {
123+
this(queryMethod.getRequiredQuery(), queryMethod, operations, rowMapperFactory, converter,
124+
evaluationContextProvider);
125+
}
126+
127+
/**
128+
* Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext}
129+
* and {@link RowMapperFactory}.
130+
*
131+
* @param queryMethod must not be {@literal null}.
132+
* @param operations must not be {@literal null}.
101133
* @param rowMapperFactory must not be {@literal null}.
102-
* @param converter must not be {@literal null}.
103-
* @param delegate must not be {@literal null}.
134+
* @param converter must not be {@literal null}.
135+
* @param delegate must not be {@literal null}.
104136
* @since 3.4
105137
*/
106138
public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
@@ -112,12 +144,12 @@ public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOpera
112144
* Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext}
113145
* and {@link RowMapperFactory}.
114146
*
115-
* @param query must not be {@literal null} or empty.
116-
* @param queryMethod must not be {@literal null}.
117-
* @param operations must not be {@literal null}.
147+
* @param query must not be {@literal null} or empty.
148+
* @param queryMethod must not be {@literal null}.
149+
* @param operations must not be {@literal null}.
118150
* @param rowMapperFactory must not be {@literal null}.
119-
* @param converter must not be {@literal null}.
120-
* @param delegate must not be {@literal null}.
151+
* @param converter must not be {@literal null}.
152+
* @param delegate must not be {@literal null}.
121153
* @since 3.4
122154
*/
123155
public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
@@ -161,6 +193,28 @@ public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedPara
161193
this.delegate = delegate;
162194
}
163195

196+
/**
197+
* Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext}
198+
* and {@link RowMapperFactory}.
199+
*
200+
* @param query must not be {@literal null} or empty.
201+
* @param queryMethod must not be {@literal null}.
202+
* @param operations must not be {@literal null}.
203+
* @param rowMapperFactory must not be {@literal null}.
204+
* @param converter must not be {@literal null}.
205+
* @param evaluationContextProvider must not be {@literal null}.
206+
* @since 3.4
207+
* @deprecated since 3.4, use the constructors accepting {@link ValueExpressionDelegate} instead.
208+
*/
209+
@Deprecated(since = "3.4")
210+
public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
211+
RowMapperFactory rowMapperFactory, JdbcConverter converter,
212+
QueryMethodEvaluationContextProvider evaluationContextProvider) {
213+
this(query, queryMethod, operations, rowMapperFactory, converter, new CachingValueExpressionDelegate(
214+
new QueryMethodValueEvaluationContextAccessor(new StandardEnvironment(),
215+
rootObject -> evaluationContextProvider.getEvaluationContext(queryMethod.getParameters(),
216+
new Object[] { rootObject })), ValueExpressionParser.create()));
217+
}
164218

165219
@Override
166220
public Object execute(Object[] objects) {
@@ -355,7 +409,8 @@ private static boolean isUnconfigured(@Nullable Class<?> configuredClass, Class<
355409
}
356410

357411
@Deprecated(since = "3.4")
358-
public void setBeanFactory(BeanFactory beanFactory) {}
412+
public void setBeanFactory(BeanFactory beanFactory) {
413+
}
359414

360415
class CachedRowMapperFactory {
361416

@@ -414,19 +469,20 @@ public CachedResultSetExtractorFactory(Supplier<RowMapper<?>> resultSetExtractor
414469
String resultSetExtractorRef = getQueryMethod().getResultSetExtractorRef();
415470
Class<? extends ResultSetExtractor> resultSetExtractorClass = getQueryMethod().getResultSetExtractorClass();
416471

417-
if (!ObjectUtils.isEmpty(resultSetExtractorRef)
418-
&& !isUnconfigured(resultSetExtractorClass, ResultSetExtractor.class)) {
472+
if (!ObjectUtils.isEmpty(resultSetExtractorRef) && !isUnconfigured(resultSetExtractorClass,
473+
ResultSetExtractor.class)) {
419474
throw new IllegalArgumentException(
420475
"Invalid ResultSetExtractor configuration. Configure either one but not both via @Query(resultSetExtractorRef = …, resultSetExtractorClass = …) for query method "
421476
+ getQueryMethod());
422477
}
423478

424-
this.configuredResultSetExtractor = !ObjectUtils.isEmpty(resultSetExtractorRef)
425-
|| !isUnconfigured(resultSetExtractorClass, ResultSetExtractor.class);
479+
this.configuredResultSetExtractor =
480+
!ObjectUtils.isEmpty(resultSetExtractorRef) || !isUnconfigured(resultSetExtractorClass,
481+
ResultSetExtractor.class);
426482

427-
this.rowMapperConstructor = resultSetExtractorClass != null
428-
? ClassUtils.getConstructorIfAvailable(resultSetExtractorClass, RowMapper.class)
429-
: null;
483+
this.rowMapperConstructor = resultSetExtractorClass != null ?
484+
ClassUtils.getConstructorIfAvailable(resultSetExtractorClass, RowMapper.class) :
485+
null;
430486
this.constructor = resultSetExtractorClass != null ? findPrimaryConstructor(resultSetExtractorClass) : null;
431487
this.resultSetExtractorFactory = rowMapper -> {
432488

Diff for: ‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/JdbcQueryMethodUnitTests.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.mockito.ArgumentMatchers.*;
2020
import static org.mockito.Mockito.*;
2121

22+
import java.lang.reflect.Array;
2223
import java.lang.reflect.Method;
2324
import java.sql.JDBCType;
2425
import java.sql.ResultSet;
@@ -95,7 +96,7 @@ public void testSqlTypeResolver() throws NoSuchMethodException {
9596
JdbcQueryMethod queryMethod = createJdbcQueryMethod(
9697
"findUserTestMethod",
9798
new DefaultSqlTypeResolver(),
98-
Integer.class, String.class, List.class
99+
Integer.class, String.class, Integer[].class
99100
);
100101

101102
JdbcParameters parameters = queryMethod.getParameters();
@@ -196,7 +197,7 @@ private void queryMethodWithWriteLock() {}
196197
private void findUserTestMethod(
197198
@SqlType(name = "TINYINT", vendorTypeNumber = Types.TINYINT) Integer age,
198199
String name,
199-
List<@SqlType(name = "SMALLINT", vendorTypeNumber = Types.SMALLINT) Integer> statuses
200+
@SqlType(name = "SMALLINT", vendorTypeNumber = Types.SMALLINT) Integer[] statuses
200201
) {}
201202

202203
@Lock(LockMode.PESSIMISTIC_READ)

Diff for: ‎spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/DefaultSqlTypeResolver.java

+12-42
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2020-2025 the original author or authors.
2+
* Copyright 2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,29 +16,24 @@
1616

1717
package org.springframework.data.relational.core.dialect;
1818

19-
import java.lang.reflect.AnnotatedParameterizedType;
20-
import java.lang.reflect.AnnotatedType;
21-
import java.lang.reflect.Parameter;
2219
import java.sql.SQLType;
2320

24-
import org.springframework.core.MethodParameter;
25-
import org.springframework.data.relational.repository.query.SqlType;
2621
import org.springframework.data.relational.repository.query.RelationalParameters;
27-
import org.springframework.data.util.TypeInformation;
22+
import org.springframework.data.relational.repository.query.SqlType;
2823
import org.springframework.lang.Nullable;
2924
import org.springframework.util.Assert;
3025

3126
/**
3227
* Default implementation of {@link SqlTypeResolver}. Capable to resolve the {@link SqlType} annotation
33-
* on the {@link java.lang.annotation.ElementType#TYPE_USE}, like this:
28+
* on the {@link java.lang.annotation.ElementType#PARAMETER method parameters}, like this:
3429
* <p>
3530
* <pre class="code">
3631
* List<User> findByAge(&#64;SqlType(name = "TINYINT", vendorTypeNumber = Types.TINYINT) byte age);
3732
* </pre>
3833
*
39-
* Or, if the intention is to specify the actual {@link SQLType}, then the following needs to be done:
34+
* Qualification of the actual {@link SQLType} (the sql type of the component), then the following needs to be done:
4035
* <pre class="code">
41-
* List<User> findByAgeIn(List<&#64;SqlType(name = "TINYINT", vendorTypeNumber = Types.TINYINT) Integer> age);
36+
* List<User> findByAgeIn(&#64;SqlType(name = "TINYINT", vendorTypeNumber = Types.TINYINT) Integer[] age);
4237
* </pre>
4338
*
4439
* @author Mikhail Polivakha
@@ -50,46 +45,21 @@ public class DefaultSqlTypeResolver implements SqlTypeResolver {
5045
@Override
5146
@Nullable
5247
public SQLType resolveSqlType(RelationalParameters.RelationalParameter relationalParameter) {
53-
SqlType parameterAnnotation = relationalParameter.getMethodParameter().getParameterAnnotation(SqlType.class);
54-
55-
if (parameterAnnotation != null) {
56-
return new AnnotationBasedSqlType(parameterAnnotation);
57-
} else {
58-
return null;
59-
}
48+
return resolveInternally(relationalParameter);
6049
}
6150

6251
@Override
6352
@Nullable
6453
public SQLType resolveActualSqlType(RelationalParameters.RelationalParameter relationalParameter) {
65-
MethodParameter methodParameter = relationalParameter.getMethodParameter();
66-
67-
TypeInformation<?> typeOfParameter = TypeInformation.of(methodParameter.getParameterType());
68-
69-
if (typeOfParameter.isCollectionLike()) {
70-
Parameter parameter = methodParameter.getParameter();
71-
AnnotatedType annotatedType = parameter.getAnnotatedType();
72-
73-
if (annotatedType instanceof AnnotatedParameterizedType parameterizedType) {
74-
return searchForGenericTypeUseAnnotation(parameterizedType);
75-
}
76-
}
77-
78-
return null;
54+
return resolveInternally(relationalParameter);
7955
}
8056

81-
@Nullable
82-
private static AnnotationBasedSqlType searchForGenericTypeUseAnnotation(AnnotatedParameterizedType parameterizedType) {
83-
AnnotatedType[] annotatedArguments = parameterizedType.getAnnotatedActualTypeArguments();
84-
85-
if (annotatedArguments.length != 1) {
86-
return null;
87-
}
88-
89-
SqlType typeUseSqlTypeAnnotation = annotatedArguments[0].getAnnotation(SqlType.class);
57+
private static AnnotationBasedSqlType resolveInternally(
58+
RelationalParameters.RelationalParameter relationalParameter) {
59+
SqlType parameterAnnotation = relationalParameter.getMethodParameter().getParameterAnnotation(SqlType.class);
9060

91-
if (typeUseSqlTypeAnnotation != null) {
92-
return new AnnotationBasedSqlType(typeUseSqlTypeAnnotation);
61+
if (parameterAnnotation != null) {
62+
return new AnnotationBasedSqlType(parameterAnnotation);
9363
} else {
9464
return null;
9565
}

Diff for: ‎spring-data-relational/src/main/java/org/springframework/data/relational/core/dialect/SqlTypeResolver.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2020-2025 the original author or authors.
2+
* Copyright 2025 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.

Diff for: ‎spring-data-relational/src/main/java/org/springframework/data/relational/repository/query/SqlType.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@
2929
* Serves as a hint to the {@link DefaultSqlTypeResolver}, that signals the {@link java.sql.SQLType} to be used.
3030
* The arguments of this annotation are identical to the methods on {@link java.sql.SQLType} interface, expect for
3131
* the {@link SQLType#getVendor()}, which is absent, because it typically does not matter as such for the underlying
32-
* JDBC drivers. For examples of usage, take a look onto {@link DefaultSqlTypeResolver}.
32+
* JDBC drivers. The examples of usage, can be found in javadoc of {@link DefaultSqlTypeResolver}.
3333
*
3434
* @see DefaultSqlTypeResolver
3535
* @author Mikhail Polivakha
3636
*/
3737
@Documented
38-
@Target({ElementType.PARAMETER, ElementType.TYPE_USE})
38+
@Target({ElementType.PARAMETER})
3939
@Retention(RetentionPolicy.RUNTIME)
4040
public @interface SqlType {
4141

0 commit comments

Comments
 (0)