Skip to content

Commit 8c6364b

Browse files
mp911demp911de
committed
Polishing.
Simplify R2DBC expression handling. Use new ValueExpression API instead of holding parameter binding duplicates. Reformat code. Add author tags. See #1904 Original pull request: #1906
1 parent d526cd3 commit 8c6364b

File tree

10 files changed

+96
-225
lines changed

10 files changed

+96
-225
lines changed

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

+23-28
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,17 @@
2020
import java.lang.reflect.Array;
2121
import java.lang.reflect.Constructor;
2222
import java.sql.SQLType;
23-
import java.util.AbstractMap;
2423
import java.util.ArrayList;
2524
import java.util.Collection;
2625
import java.util.LinkedHashMap;
2726
import java.util.List;
28-
import java.util.Map;
2927
import java.util.function.Function;
3028
import java.util.function.Supplier;
3129

3230
import org.springframework.beans.BeanInstantiationException;
3331
import org.springframework.beans.BeanUtils;
3432
import org.springframework.beans.factory.BeanFactory;
33+
import org.springframework.core.env.StandardEnvironment;
3534
import org.springframework.data.expression.ValueEvaluationContext;
3635
import org.springframework.data.expression.ValueExpressionParser;
3736
import org.springframework.data.jdbc.core.convert.JdbcColumnTypes;
@@ -51,7 +50,6 @@
5150
import org.springframework.data.repository.query.ValueExpressionQueryRewriter;
5251
import org.springframework.data.util.Lazy;
5352
import org.springframework.data.util.TypeInformation;
54-
import org.springframework.expression.spel.standard.SpelExpressionParser;
5553
import org.springframework.jdbc.core.ResultSetExtractor;
5654
import org.springframework.jdbc.core.RowMapper;
5755
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
@@ -74,6 +72,7 @@
7472
* @author Chirag Tailor
7573
* @author Christopher Klein
7674
* @author Mikhail Polivakha
75+
* @author Marcin Grzejszczak
7776
* @since 2.0
7877
*/
7978
public class StringBasedJdbcQuery extends AbstractJdbcQuery {
@@ -82,13 +81,11 @@ public class StringBasedJdbcQuery extends AbstractJdbcQuery {
8281
private final JdbcConverter converter;
8382
private final RowMapperFactory rowMapperFactory;
8483
private final ValueExpressionQueryRewriter.ParsedQuery parsedQuery;
85-
private final boolean containsSpelExpressions;
8684
private final String query;
8785

8886
private final CachedRowMapperFactory cachedRowMapperFactory;
8987
private final CachedResultSetExtractorFactory cachedResultSetExtractorFactory;
9088
private final ValueExpressionDelegate delegate;
91-
private final List<Map.Entry<String, String>> parameterBindings;
9289

9390
/**
9491
* Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext}
@@ -186,17 +183,11 @@ public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedPara
186183
this.cachedResultSetExtractorFactory = new CachedResultSetExtractorFactory(
187184
this.cachedRowMapperFactory::getRowMapper);
188185

189-
this.parameterBindings = new ArrayList<>();
190-
191-
ValueExpressionQueryRewriter rewriter = ValueExpressionQueryRewriter.of(delegate, (counter, expression) -> {
192-
String newName = String.format("__$synthetic$__%d", counter + 1);
193-
parameterBindings.add(new AbstractMap.SimpleEntry<>(newName, expression));
194-
return newName;
195-
}, String::concat);
186+
ValueExpressionQueryRewriter rewriter = ValueExpressionQueryRewriter.of(delegate,
187+
(counter, expression) -> String.format("__$synthetic$__%d", counter + 1), String::concat);
196188

197189
this.query = query;
198190
this.parsedQuery = rewriter.parse(this.query);
199-
this.containsSpelExpressions = !this.parsedQuery.getQueryString().equals(this.query);
200191
this.delegate = delegate;
201192
}
202193

@@ -217,9 +208,10 @@ public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedPara
217208
public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
218209
RowMapperFactory rowMapperFactory, JdbcConverter converter,
219210
QueryMethodEvaluationContextProvider evaluationContextProvider) {
220-
this(query, queryMethod, operations, rowMapperFactory, converter, new CachingValueExpressionDelegate(new QueryMethodValueEvaluationContextAccessor(null,
221-
rootObject -> evaluationContextProvider.getEvaluationContext(queryMethod.getParameters(), new Object[] { rootObject })), ValueExpressionParser.create(
222-
SpelExpressionParser::new)));
211+
this(query, queryMethod, operations, rowMapperFactory, converter, new CachingValueExpressionDelegate(
212+
new QueryMethodValueEvaluationContextAccessor(new StandardEnvironment(), rootObject -> evaluationContextProvider
213+
.getEvaluationContext(queryMethod.getParameters(), new Object[] { rootObject })),
214+
ValueExpressionParser.create()));
223215
}
224216

225217
@Override
@@ -231,18 +223,22 @@ public Object execute(Object[] objects) {
231223
JdbcQueryExecution<?> queryExecution = createJdbcQueryExecution(accessor, processor);
232224
MapSqlParameterSource parameterMap = this.bindParameters(accessor);
233225

234-
return queryExecution.execute(processSpelExpressions(objects, accessor.getBindableParameters(), parameterMap), parameterMap);
226+
return queryExecution.execute(evaluateExpressions(objects, accessor.getBindableParameters(), parameterMap),
227+
parameterMap);
235228
}
236229

237-
private String processSpelExpressions(Object[] objects, Parameters<?, ?> bindableParameters, MapSqlParameterSource parameterMap) {
230+
private String evaluateExpressions(Object[] objects, Parameters<?, ?> bindableParameters,
231+
MapSqlParameterSource parameterMap) {
232+
233+
if (parsedQuery.hasParameterBindings()) {
238234

239-
if (containsSpelExpressions) {
240235
ValueEvaluationContext evaluationContext = delegate.createValueContextProvider(bindableParameters)
241236
.getEvaluationContext(objects);
242-
for (Map.Entry<String, String> entry : parameterBindings) {
243-
parameterMap.addValue(
244-
entry.getKey(), delegate.parse(entry.getValue()).evaluate(evaluationContext));
245-
}
237+
238+
parsedQuery.getParameterMap().forEach((paramName, valueExpression) -> {
239+
parameterMap.addValue(paramName, valueExpression.evaluate(evaluationContext));
240+
});
241+
246242
return parsedQuery.getQueryString();
247243
}
248244

@@ -254,13 +250,12 @@ private JdbcQueryExecution<?> createJdbcQueryExecution(RelationalParameterAccess
254250

255251
if (getQueryMethod().isModifyingQuery()) {
256252
return createModifyingQueryExecutor();
257-
} else {
253+
}
258254

259-
Supplier<RowMapper<?>> rowMapper = () -> determineRowMapper(processor, accessor.findDynamicProjection() != null);
260-
ResultSetExtractor<Object> resultSetExtractor = determineResultSetExtractor(rowMapper);
255+
Supplier<RowMapper<?>> rowMapper = () -> determineRowMapper(processor, accessor.findDynamicProjection() != null);
256+
ResultSetExtractor<Object> resultSetExtractor = determineResultSetExtractor(rowMapper);
261257

262-
return createReadingQueryExecution(resultSetExtractor, rowMapper);
263-
}
258+
return createReadingQueryExecution(resultSetExtractor, rowMapper);
264259
}
265260

266261
private MapSqlParameterSource bindParameters(RelationalParameterAccessor accessor) {

‎spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.springframework.data.repository.core.RepositoryMetadata;
3333
import org.springframework.data.repository.core.support.PersistentEntityInformation;
3434
import org.springframework.data.repository.core.support.RepositoryFactorySupport;
35+
import org.springframework.data.repository.query.CachingValueExpressionDelegate;
3536
import org.springframework.data.repository.query.QueryLookupStrategy;
3637
import org.springframework.data.repository.query.ValueExpressionDelegate;
3738
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
@@ -48,6 +49,7 @@
4849
* @author Hebert Coelho
4950
* @author Diego Krupitza
5051
* @author Christopher Klein
52+
* @author Marcin Grzejszczak
5153
*/
5254
public class JdbcRepositoryFactory extends RepositoryFactorySupport {
5355

@@ -57,7 +59,7 @@ public class JdbcRepositoryFactory extends RepositoryFactorySupport {
5759
private final DataAccessStrategy accessStrategy;
5860
private final NamedParameterJdbcOperations operations;
5961
private final Dialect dialect;
60-
@Nullable private BeanFactory beanFactory;
62+
private @Nullable BeanFactory beanFactory;
6163

6264
private QueryMappingConfiguration queryMappingConfiguration = QueryMappingConfiguration.EMPTY;
6365
private EntityCallbacks entityCallbacks;
@@ -132,10 +134,12 @@ protected Class<?> getRepositoryBaseClass(RepositoryMetadata repositoryMetadata)
132134
return SimpleJdbcRepository.class;
133135
}
134136

135-
@Override protected Optional<QueryLookupStrategy> getQueryLookupStrategy(QueryLookupStrategy.Key key,
137+
@Override
138+
protected Optional<QueryLookupStrategy> getQueryLookupStrategy(@Nullable QueryLookupStrategy.Key key,
136139
ValueExpressionDelegate valueExpressionDelegate) {
137140
return Optional.of(JdbcQueryLookupStrategy.create(key, publisher, entityCallbacks, context, converter, dialect,
138-
queryMappingConfiguration, operations, beanFactory, valueExpressionDelegate));
141+
queryMappingConfiguration, operations, beanFactory,
142+
new CachingValueExpressionDelegate(valueExpressionDelegate)));
139143
}
140144

141145
/**

‎spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import org.junit.jupiter.api.Test;
3535
import org.mockito.ArgumentCaptor;
3636

37-
import org.springframework.context.support.StaticApplicationContext;
3837
import org.springframework.core.convert.converter.Converter;
3938
import org.springframework.core.env.StandardEnvironment;
4039
import org.springframework.dao.DataAccessException;
@@ -79,6 +78,7 @@
7978
* @author Dennis Effing
8079
* @author Chirag Tailor
8180
* @author Christopher Klein
81+
* @author Marcin Grzejszczak
8282
*/
8383
class StringBasedJdbcQueryUnitTests {
8484

@@ -95,8 +95,7 @@ void setup() {
9595
this.operations = mock(NamedParameterJdbcOperations.class);
9696
this.context = mock(RelationalMappingContext.class, RETURNS_DEEP_STUBS);
9797
this.converter = new MappingJdbcConverter(context, mock(RelationResolver.class));
98-
QueryMethodValueEvaluationContextAccessor accessor = new QueryMethodValueEvaluationContextAccessor(new StandardEnvironment(), new StaticApplicationContext());
99-
this.delegate = new ValueExpressionDelegate(accessor, ValueExpressionParser.create());
98+
this.delegate = ValueExpressionDelegate.create();
10099
}
101100

102101
@Test // DATAJDBC-165

‎spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/DefaultR2dbcSpELExpressionEvaluator.java

-78
This file was deleted.

‎spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionEvaluatingParameterBinder.java

+22-8
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@
1515
*/
1616
package org.springframework.data.r2dbc.repository.query;
1717

18-
import static org.springframework.data.r2dbc.repository.query.ExpressionQuery.*;
19-
2018
import java.util.Map;
2119
import java.util.Optional;
2220
import java.util.concurrent.ConcurrentHashMap;
2321
import java.util.regex.Pattern;
2422

23+
import org.springframework.data.expression.ValueEvaluationContext;
24+
import org.springframework.data.expression.ValueExpression;
2525
import org.springframework.data.r2dbc.core.ReactiveDataAccessStrategy;
2626
import org.springframework.data.r2dbc.dialect.BindTargetBinder;
2727
import org.springframework.data.relational.repository.query.RelationalParameterAccessor;
@@ -64,26 +64,40 @@ class ExpressionEvaluatingParameterBinder {
6464
* @param evaluator must not be {@literal null}.
6565
*/
6666
void bind(BindTarget bindTarget,
67-
RelationalParameterAccessor parameterAccessor, R2dbcSpELExpressionEvaluator evaluator) {
67+
RelationalParameterAccessor parameterAccessor, ValueEvaluationContext evaluationContext) {
6868

6969
Object[] values = parameterAccessor.getValues();
7070
Parameters<?, ?> bindableParameters = parameterAccessor.getBindableParameters();
7171

72-
bindExpressions(bindTarget, evaluator);
72+
bindExpressions(bindTarget, evaluationContext);
7373
bindParameters(bindTarget, parameterAccessor.hasBindableNullValue(), values, bindableParameters);
7474
}
7575

7676
private void bindExpressions(BindTarget bindSpec,
77-
R2dbcSpELExpressionEvaluator evaluator) {
77+
ValueEvaluationContext evaluationContext) {
7878

7979
BindTargetBinder binder = new BindTargetBinder(bindSpec);
80-
for (ParameterBinding binding : expressionQuery.getBindings()) {
80+
81+
expressionQuery.getBindings().forEach((paramName, valueExpression) -> {
8182

8283
org.springframework.r2dbc.core.Parameter valueForBinding = getBindValue(
83-
evaluator.evaluate(binding.getExpression()));
84+
evaluate(valueExpression, evaluationContext));
85+
86+
binder.bind(paramName, valueForBinding);
87+
});
88+
}
89+
90+
private org.springframework.r2dbc.core.Parameter evaluate(ValueExpression expression,
91+
ValueEvaluationContext context) {
8492

85-
binder.bind(binding.getParameterName(), valueForBinding);
93+
Object value = expression.evaluate(context);
94+
Class<?> valueType = value != null ? value.getClass() : null;
95+
96+
if (valueType == null) {
97+
valueType = expression.getValueType(context);
8698
}
99+
100+
return org.springframework.r2dbc.core.Parameter.fromOrEmpty(value, valueType == null ? Object.class : valueType);
87101
}
88102

89103
private void bindParameters(BindTarget bindSpec,

0 commit comments

Comments
 (0)