Skip to content

Commit 55f8f1f

Browse files
mp911demp911de
committed
Polishing.
Simplify R2DBC expression handling. Use new ValueExpression API instead of holding parameter binding duplicates. Reformat code. Add author tags.
1 parent b010a4e commit 55f8f1f

File tree

10 files changed

+93
-223
lines changed

10 files changed

+93
-223
lines changed

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

Lines changed: 23 additions & 28 deletions
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}
@@ -185,17 +182,11 @@ public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedPara
185182
this.cachedResultSetExtractorFactory = new CachedResultSetExtractorFactory(
186183
this.cachedRowMapperFactory::getRowMapper);
187184

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

196188
this.query = query;
197189
this.parsedQuery = rewriter.parse(this.query);
198-
this.containsSpelExpressions = !this.parsedQuery.getQueryString().equals(this.query);
199190
this.delegate = delegate;
200191
}
201192

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

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

233-
return queryExecution.execute(processSpelExpressions(objects, accessor.getBindableParameters(), parameterMap), parameterMap);
225+
return queryExecution.execute(evaluateExpressions(objects, accessor.getBindableParameters(), parameterMap),
226+
parameterMap);
234227
}
235228

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

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

@@ -253,13 +249,12 @@ private JdbcQueryExecution<?> createJdbcQueryExecution(RelationalParameterAccess
253249

254250
if (getQueryMethod().isModifyingQuery()) {
255251
return createModifyingQueryExecutor();
256-
} else {
252+
}
257253

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

261-
return createReadingQueryExecution(resultSetExtractor, rowMapper);
262-
}
257+
return createReadingQueryExecution(resultSetExtractor, rowMapper);
263258
}
264259

265260
private MapSqlParameterSource bindParameters(RelationalParameterAccessor accessor) {

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

Lines changed: 7 additions & 3 deletions
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‎

Lines changed: 2 additions & 3 deletions
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‎

Lines changed: 0 additions & 78 deletions
This file was deleted.

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

Lines changed: 22 additions & 8 deletions
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)