From 356c269caafc529f0a17ed19aa277fc7190850e4 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 1 Oct 2024 10:55:15 +0200 Subject: [PATCH 1/3] Preparing to work on the issue --- pom.xml | 4 ++-- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-r2dbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index 335f5969b8..5455f1f684 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-GH-1904-SNAPSHOT pom Spring Data Relational Parent @@ -20,7 +20,7 @@ spring-data-jdbc - 3.4.0-SNAPSHOT + 3.4.0-GH-3049-SNAPSHOT 4.21.1 reuseReports diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 84eeb178e0..e764e4e143 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-GH-1904-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index 266af71b96..72e2bc74ac 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 3.4.0-SNAPSHOT + 3.4.0-GH-1904-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-GH-1904-SNAPSHOT diff --git a/spring-data-r2dbc/pom.xml b/spring-data-r2dbc/pom.xml index e335b56501..fa17ec2c43 100644 --- a/spring-data-r2dbc/pom.xml +++ b/spring-data-r2dbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-r2dbc - 3.4.0-SNAPSHOT + 3.4.0-GH-1904-SNAPSHOT Spring Data R2DBC Spring Data module for R2DBC @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-GH-1904-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 6be1155d38..948b10535d 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 3.4.0-SNAPSHOT + 3.4.0-GH-1904-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.4.0-SNAPSHOT + 3.4.0-GH-1904-SNAPSHOT From b010a4e7061cc20816258d2691226b978d12145e Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 3 Oct 2024 17:45:09 +0200 Subject: [PATCH 2/3] Add support for Value Expressions for Repository Query methods. Closes #3619 --- .../query/StringBasedJdbcQuery.java | 91 +++++++++++++++---- .../support/JdbcQueryLookupStrategy.java | 34 +++---- .../support/JdbcRepositoryFactory.java | 10 +- .../query/StringBasedJdbcQueryUnitTests.java | 31 ++++--- .../JdbcQueryLookupStrategyUnitTests.java | 5 +- .../DefaultR2dbcSpELExpressionEvaluator.java | 20 ++-- .../repository/query/ExpressionQuery.java | 10 +- .../query/StringBasedR2dbcQuery.java | 62 +++++++++++-- .../support/CachingExpressionParser.java | 51 ----------- .../support/R2dbcRepositoryFactory.java | 26 ++---- .../query/ExpressionQueryUnitTests.java | 18 ++-- .../query/StringBasedR2dbcQueryUnitTests.java | 17 +++- .../R2dbcRepositoryFactoryBeanUnitTests.java | 7 +- 13 files changed, 218 insertions(+), 164 deletions(-) delete mode 100644 spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/CachingExpressionParser.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java index 3e8aebc44d..2130ba0bb5 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java @@ -20,16 +20,20 @@ import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.sql.SQLType; +import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.function.Function; import java.util.function.Supplier; import org.springframework.beans.BeanInstantiationException; import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanFactory; +import org.springframework.data.expression.ValueEvaluationContext; +import org.springframework.data.expression.ValueExpressionParser; import org.springframework.data.jdbc.core.convert.JdbcColumnTypes; import org.springframework.data.jdbc.core.convert.JdbcConverter; import org.springframework.data.jdbc.core.mapping.JdbcValue; @@ -37,14 +41,17 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.repository.query.RelationalParameterAccessor; import org.springframework.data.relational.repository.query.RelationalParametersParameterAccessor; +import org.springframework.data.repository.query.CachingValueExpressionDelegate; import org.springframework.data.repository.query.Parameter; import org.springframework.data.repository.query.Parameters; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; +import org.springframework.data.repository.query.QueryMethodValueEvaluationContextAccessor; import org.springframework.data.repository.query.ResultProcessor; -import org.springframework.data.repository.query.SpelEvaluator; -import org.springframework.data.repository.query.SpelQueryContext; +import org.springframework.data.repository.query.ValueExpressionDelegate; +import org.springframework.data.repository.query.ValueExpressionQueryRewriter; import org.springframework.data.util.Lazy; import org.springframework.data.util.TypeInformation; +import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.jdbc.core.ResultSetExtractor; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; @@ -74,12 +81,14 @@ public class StringBasedJdbcQuery extends AbstractJdbcQuery { private static final String PARAMETER_NEEDS_TO_BE_NAMED = "For queries with named parameters you need to provide names for method parameters; Use @Param for query method parameters, or use the javac flag -parameters"; private final JdbcConverter converter; private final RowMapperFactory rowMapperFactory; - private final SpelEvaluator spelEvaluator; + private final ValueExpressionQueryRewriter.ParsedQuery parsedQuery; private final boolean containsSpelExpressions; private final String query; private final CachedRowMapperFactory cachedRowMapperFactory; private final CachedResultSetExtractorFactory cachedResultSetExtractorFactory; + private final ValueExpressionDelegate delegate; + private final List> parameterBindings; /** * Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext} @@ -88,7 +97,9 @@ public class StringBasedJdbcQuery extends AbstractJdbcQuery { * @param queryMethod must not be {@literal null}. * @param operations must not be {@literal null}. * @param defaultRowMapper can be {@literal null} (only in case of a modifying query). + * @deprecated since 3.4, use the constructors accepting {@link ValueExpressionDelegate} instead. */ + @Deprecated(since = "3.4") public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations, @Nullable RowMapper defaultRowMapper, JdbcConverter converter, QueryMethodEvaluationContextProvider evaluationContextProvider) { @@ -115,6 +126,23 @@ public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOpera evaluationContextProvider); } + /** + * Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext} + * and {@link RowMapperFactory}. + * + * @param queryMethod must not be {@literal null}. + * @param operations must not be {@literal null}. + * @param rowMapperFactory must not be {@literal null}. + * @param converter must not be {@literal null}. + * @param delegate must not be {@literal null}. + * @since 3.4 + */ + public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations, + RowMapperFactory rowMapperFactory, JdbcConverter converter, + ValueExpressionDelegate delegate) { + this(queryMethod.getRequiredQuery(), queryMethod, operations, rowMapperFactory, converter, delegate); + } + /** * Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext} * and {@link RowMapperFactory}. @@ -124,15 +152,13 @@ public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOpera * @param operations must not be {@literal null}. * @param rowMapperFactory must not be {@literal null}. * @param converter must not be {@literal null}. - * @param evaluationContextProvider must not be {@literal null}. + * @param delegate must not be {@literal null}. * @since 3.4 */ public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations, RowMapperFactory rowMapperFactory, JdbcConverter converter, - QueryMethodEvaluationContextProvider evaluationContextProvider) { - + ValueExpressionDelegate delegate) { super(queryMethod, operations); - Assert.hasText(query, "Query must not be null or empty"); Assert.notNull(rowMapperFactory, "RowMapperFactory must not be null"); @@ -159,13 +185,40 @@ public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedPara this.cachedResultSetExtractorFactory = new CachedResultSetExtractorFactory( this.cachedRowMapperFactory::getRowMapper); - SpelQueryContext.EvaluatingSpelQueryContext queryContext = SpelQueryContext - .of((counter, expression) -> String.format("__$synthetic$__%d", counter + 1), String::concat) - .withEvaluationContextProvider(evaluationContextProvider); + this.parameterBindings = new ArrayList<>(); + + ValueExpressionQueryRewriter rewriter = ValueExpressionQueryRewriter.of(delegate, (counter, expression) -> { + String newName = String.format("__$synthetic$__%d", counter + 1); + parameterBindings.add(new AbstractMap.SimpleEntry<>(newName, expression)); + return newName; + }, String::concat); this.query = query; - this.spelEvaluator = queryContext.parse(this.query, getQueryMethod().getParameters()); - this.containsSpelExpressions = !this.spelEvaluator.getQueryString().equals(this.query); + this.parsedQuery = rewriter.parse(this.query); + this.containsSpelExpressions = !this.parsedQuery.getQueryString().equals(this.query); + this.delegate = delegate; + } + + /** + * Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext} + * and {@link RowMapperFactory}. + * + * @param query must not be {@literal null} or empty. + * @param queryMethod must not be {@literal null}. + * @param operations must not be {@literal null}. + * @param rowMapperFactory must not be {@literal null}. + * @param converter must not be {@literal null}. + * @param evaluationContextProvider must not be {@literal null}. + * @since 3.4 + * @deprecated since 3.4, use the constructors accepting {@link ValueExpressionDelegate} instead. + */ + @Deprecated(since = "3.4") + public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations, + RowMapperFactory rowMapperFactory, JdbcConverter converter, + QueryMethodEvaluationContextProvider evaluationContextProvider) { + this(query, queryMethod, operations, rowMapperFactory, converter, new CachingValueExpressionDelegate(new QueryMethodValueEvaluationContextAccessor(null, + rootObject -> evaluationContextProvider.getEvaluationContext(queryMethod.getParameters(), new Object[] { rootObject })), ValueExpressionParser.create( + SpelExpressionParser::new))); } @Override @@ -177,15 +230,19 @@ public Object execute(Object[] objects) { JdbcQueryExecution queryExecution = createJdbcQueryExecution(accessor, processor); MapSqlParameterSource parameterMap = this.bindParameters(accessor); - return queryExecution.execute(processSpelExpressions(objects, parameterMap), parameterMap); + return queryExecution.execute(processSpelExpressions(objects, accessor.getBindableParameters(), parameterMap), parameterMap); } - private String processSpelExpressions(Object[] objects, MapSqlParameterSource parameterMap) { + private String processSpelExpressions(Object[] objects, Parameters bindableParameters, MapSqlParameterSource parameterMap) { if (containsSpelExpressions) { - - spelEvaluator.evaluate(objects).forEach(parameterMap::addValue); - return spelEvaluator.getQueryString(); + ValueEvaluationContext evaluationContext = delegate.createValueContextProvider(bindableParameters) + .getEvaluationContext(objects); + for (Map.Entry entry : parameterBindings) { + parameterMap.addValue( + entry.getKey(), delegate.parse(entry.getValue()).evaluate(evaluationContext)); + } + return parsedQuery.getQueryString(); } return this.query; diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java index d265b1becd..a145365de9 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategy.java @@ -41,8 +41,8 @@ import org.springframework.data.repository.core.NamedQueries; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.query.QueryLookupStrategy; -import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.RepositoryQuery; +import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.jdbc.core.ResultSetExtractor; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.SingleColumnRowMapper; @@ -73,12 +73,12 @@ abstract class JdbcQueryLookupStrategy extends RelationalQueryLookupStrategy { private final JdbcConverter converter; private final QueryMappingConfiguration queryMappingConfiguration; private final NamedParameterJdbcOperations operations; - protected final QueryMethodEvaluationContextProvider evaluationContextProvider; + protected final ValueExpressionDelegate delegate; JdbcQueryLookupStrategy(ApplicationEventPublisher publisher, @Nullable EntityCallbacks callbacks, RelationalMappingContext context, JdbcConverter converter, Dialect dialect, QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations, - QueryMethodEvaluationContextProvider evaluationContextProvider) { + ValueExpressionDelegate delegate) { super(context, dialect); @@ -86,7 +86,7 @@ abstract class JdbcQueryLookupStrategy extends RelationalQueryLookupStrategy { Assert.notNull(converter, "JdbcConverter must not be null"); Assert.notNull(queryMappingConfiguration, "QueryMappingConfiguration must not be null"); Assert.notNull(operations, "NamedParameterJdbcOperations must not be null"); - Assert.notNull(evaluationContextProvider, "QueryMethodEvaluationContextProvider must not be null"); + Assert.notNull(delegate, "ValueExpressionDelegate must not be null"); this.context = context; this.publisher = publisher; @@ -94,7 +94,7 @@ abstract class JdbcQueryLookupStrategy extends RelationalQueryLookupStrategy { this.converter = converter; this.queryMappingConfiguration = queryMappingConfiguration; this.operations = operations; - this.evaluationContextProvider = evaluationContextProvider; + this.delegate = delegate; } public RelationalMappingContext getMappingContext() { @@ -112,10 +112,10 @@ static class CreateQueryLookupStrategy extends JdbcQueryLookupStrategy { CreateQueryLookupStrategy(ApplicationEventPublisher publisher, @Nullable EntityCallbacks callbacks, RelationalMappingContext context, JdbcConverter converter, Dialect dialect, QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations, - QueryMethodEvaluationContextProvider evaluationContextProvider) { + ValueExpressionDelegate delegate) { super(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, - evaluationContextProvider); + delegate); } @Override @@ -143,9 +143,9 @@ static class DeclaredQueryLookupStrategy extends JdbcQueryLookupStrategy { DeclaredQueryLookupStrategy(ApplicationEventPublisher publisher, @Nullable EntityCallbacks callbacks, RelationalMappingContext context, JdbcConverter converter, Dialect dialect, QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations, - @Nullable BeanFactory beanfactory, QueryMethodEvaluationContextProvider evaluationContextProvider) { + @Nullable BeanFactory beanfactory, ValueExpressionDelegate delegate) { super(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, - evaluationContextProvider); + delegate); this.rowMapperFactory = new BeanFactoryRowMapperFactory(beanfactory); } @@ -166,7 +166,7 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata repository String queryString = evaluateTableExpressions(repositoryMetadata, queryMethod.getRequiredQuery()); return new StringBasedJdbcQuery(queryString, queryMethod, getOperations(), rowMapperFactory, getConverter(), - evaluationContextProvider); + delegate); } throw new IllegalStateException( @@ -235,10 +235,10 @@ static class CreateIfNotFoundQueryLookupStrategy extends JdbcQueryLookupStrategy RelationalMappingContext context, JdbcConverter converter, Dialect dialect, QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations, CreateQueryLookupStrategy createStrategy, - DeclaredQueryLookupStrategy lookupStrategy, QueryMethodEvaluationContextProvider evaluationContextProvider) { + DeclaredQueryLookupStrategy lookupStrategy, ValueExpressionDelegate delegate) { super(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, - evaluationContextProvider); + delegate); Assert.notNull(createStrategy, "CreateQueryLookupStrategy must not be null"); Assert.notNull(lookupStrategy, "DeclaredQueryLookupStrategy must not be null"); @@ -284,20 +284,20 @@ JdbcQueryMethod getJdbcQueryMethod(Method method, RepositoryMetadata repositoryM public static QueryLookupStrategy create(@Nullable Key key, ApplicationEventPublisher publisher, @Nullable EntityCallbacks callbacks, RelationalMappingContext context, JdbcConverter converter, Dialect dialect, QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations, - @Nullable BeanFactory beanFactory, QueryMethodEvaluationContextProvider evaluationContextProvider) { - + @Nullable BeanFactory beanFactory, ValueExpressionDelegate delegate) { Assert.notNull(publisher, "ApplicationEventPublisher must not be null"); Assert.notNull(context, "RelationalMappingContextPublisher must not be null"); Assert.notNull(converter, "JdbcConverter must not be null"); Assert.notNull(dialect, "Dialect must not be null"); Assert.notNull(queryMappingConfiguration, "QueryMappingConfiguration must not be null"); Assert.notNull(operations, "NamedParameterJdbcOperations must not be null"); + Assert.notNull(delegate, "ValueExpressionDelegate must not be null"); CreateQueryLookupStrategy createQueryLookupStrategy = new CreateQueryLookupStrategy(publisher, callbacks, context, - converter, dialect, queryMappingConfiguration, operations, evaluationContextProvider); + converter, dialect, queryMappingConfiguration, operations, delegate); DeclaredQueryLookupStrategy declaredQueryLookupStrategy = new DeclaredQueryLookupStrategy(publisher, callbacks, - context, converter, dialect, queryMappingConfiguration, operations, beanFactory, evaluationContextProvider); + context, converter, dialect, queryMappingConfiguration, operations, beanFactory, delegate); Key keyToUse = key != null ? key : Key.CREATE_IF_NOT_FOUND; @@ -311,7 +311,7 @@ public static QueryLookupStrategy create(@Nullable Key key, ApplicationEventPubl case CREATE_IF_NOT_FOUND: return new CreateIfNotFoundQueryLookupStrategy(publisher, callbacks, context, converter, dialect, queryMappingConfiguration, operations, createQueryLookupStrategy, declaredQueryLookupStrategy, - evaluationContextProvider); + delegate); default: throw new IllegalArgumentException(String.format("Unsupported query lookup strategy %s", key)); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java index f6becf1ff6..31683a0da7 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java @@ -33,7 +33,7 @@ import org.springframework.data.repository.core.support.PersistentEntityInformation; import org.springframework.data.repository.core.support.RepositoryFactorySupport; import org.springframework.data.repository.query.QueryLookupStrategy; -import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; +import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -132,12 +132,10 @@ protected Class getRepositoryBaseClass(RepositoryMetadata repositoryMetadata) return SimpleJdbcRepository.class; } - @Override - protected Optional getQueryLookupStrategy(@Nullable QueryLookupStrategy.Key key, - QueryMethodEvaluationContextProvider evaluationContextProvider) { - + @Override protected Optional getQueryLookupStrategy(QueryLookupStrategy.Key key, + ValueExpressionDelegate valueExpressionDelegate) { return Optional.of(JdbcQueryLookupStrategy.create(key, publisher, entityCallbacks, context, converter, dialect, - queryMappingConfiguration, operations, beanFactory, evaluationContextProvider)); + queryMappingConfiguration, operations, beanFactory, valueExpressionDelegate)); } /** diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java index 8e187453ad..5a937210d2 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java @@ -33,7 +33,10 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; + +import org.springframework.context.support.StaticApplicationContext; import org.springframework.core.convert.converter.Converter; +import org.springframework.core.env.StandardEnvironment; import org.springframework.dao.DataAccessException; import org.springframework.data.convert.ReadingConverter; import org.springframework.data.convert.WritingConverter; @@ -41,6 +44,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; +import org.springframework.data.expression.ValueExpressionParser; import org.springframework.data.jdbc.core.convert.JdbcConverter; import org.springframework.data.jdbc.core.convert.JdbcCustomConversions; import org.springframework.data.jdbc.core.convert.JdbcTypeFactory; @@ -53,9 +57,9 @@ import org.springframework.data.repository.Repository; import org.springframework.data.repository.core.support.DefaultRepositoryMetadata; import org.springframework.data.repository.core.support.PropertiesBasedNamedQueries; -import org.springframework.data.repository.query.ExtensionAwareQueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.Param; -import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; +import org.springframework.data.repository.query.QueryMethodValueEvaluationContextAccessor; +import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.data.spel.spi.EvaluationContextExtension; import org.springframework.jdbc.core.ResultSetExtractor; import org.springframework.jdbc.core.RowMapper; @@ -82,7 +86,7 @@ class StringBasedJdbcQueryUnitTests { NamedParameterJdbcOperations operations; RelationalMappingContext context; JdbcConverter converter; - QueryMethodEvaluationContextProvider evaluationContextProvider; + ValueExpressionDelegate delegate; @BeforeEach void setup() { @@ -91,7 +95,8 @@ void setup() { this.operations = mock(NamedParameterJdbcOperations.class); this.context = mock(RelationalMappingContext.class, RETURNS_DEEP_STUBS); this.converter = new MappingJdbcConverter(context, mock(RelationResolver.class)); - this.evaluationContextProvider = mock(QueryMethodEvaluationContextProvider.class); + QueryMethodValueEvaluationContextAccessor accessor = new QueryMethodValueEvaluationContextAccessor(new StandardEnvironment(), new StaticApplicationContext()); + this.delegate = new ValueExpressionDelegate(accessor, ValueExpressionParser.create()); } @Test // DATAJDBC-165 @@ -248,7 +253,7 @@ void sliceQueryNotSupported() { JdbcQueryMethod queryMethod = createMethod("sliceAll", Pageable.class); assertThatThrownBy( - () -> new StringBasedJdbcQuery(queryMethod, operations, defaultRowMapper, converter, evaluationContextProvider)) + () -> new StringBasedJdbcQuery(queryMethod, operations, result -> defaultRowMapper, converter, delegate)) .isInstanceOf(UnsupportedOperationException.class) .hasMessageContaining("Slice queries are not supported using string-based queries"); } @@ -259,7 +264,7 @@ void pageQueryNotSupported() { JdbcQueryMethod queryMethod = createMethod("pageAll", Pageable.class); assertThatThrownBy( - () -> new StringBasedJdbcQuery(queryMethod, operations, defaultRowMapper, converter, evaluationContextProvider)) + () -> new StringBasedJdbcQuery(queryMethod, operations, result -> defaultRowMapper, converter, delegate)) .isInstanceOf(UnsupportedOperationException.class) .hasMessageContaining("Page queries are not supported using string-based queries"); } @@ -270,7 +275,7 @@ void limitNotSupported() { JdbcQueryMethod queryMethod = createMethod("unsupportedLimitQuery", String.class, Limit.class); assertThatThrownBy( - () -> new StringBasedJdbcQuery(queryMethod, operations, defaultRowMapper, converter, evaluationContextProvider)) + () -> new StringBasedJdbcQuery(queryMethod, operations, result -> defaultRowMapper, converter, delegate)) .isInstanceOf(UnsupportedOperationException.class); } @@ -350,11 +355,11 @@ void spelCanBeUsedInsideQueries() { List list = new ArrayList<>(); list.add(new MyEvaluationContextProvider()); - QueryMethodEvaluationContextProvider evaluationContextProviderImpl = new ExtensionAwareQueryMethodEvaluationContextProvider( - list); - StringBasedJdbcQuery sut = new StringBasedJdbcQuery(queryMethod, operations, defaultRowMapper, converter, - evaluationContextProviderImpl); + QueryMethodValueEvaluationContextAccessor accessor = new QueryMethodValueEvaluationContextAccessor(new StandardEnvironment(), list); + this.delegate = new ValueExpressionDelegate(accessor, ValueExpressionParser.create()); + + StringBasedJdbcQuery sut = new StringBasedJdbcQuery(queryMethod, operations, result -> defaultRowMapper, converter, delegate); ArgumentCaptor paramSource = ArgumentCaptor.forClass(SqlParameterSource.class); ArgumentCaptor query = ArgumentCaptor.forClass(String.class); @@ -398,7 +403,7 @@ public SqlParameterSource extractParameterSource() { : this.converter; StringBasedJdbcQuery query = new StringBasedJdbcQuery(method.getDeclaredQuery(), method, operations, result -> mock(RowMapper.class), - converter, evaluationContextProvider); + converter, delegate); query.execute(arguments); @@ -434,7 +439,7 @@ private StringBasedJdbcQuery createQuery(JdbcQueryMethod queryMethod) { } private StringBasedJdbcQuery createQuery(JdbcQueryMethod queryMethod, String preparedReference, Object value) { - return new StringBasedJdbcQuery(queryMethod, operations, new StubRowMapperFactory(preparedReference, value), converter, evaluationContextProvider); + return new StringBasedJdbcQuery(queryMethod, operations, new StubRowMapperFactory(preparedReference, value), converter, delegate); } interface MyRepository extends Repository { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategyUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategyUnitTests.java index 2d2e12c73f..d3f1d3ff40 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategyUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/support/JdbcQueryLookupStrategyUnitTests.java @@ -42,6 +42,7 @@ import org.springframework.data.repository.query.QueryLookupStrategy; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.RepositoryQuery; +import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.data.util.TypeInformation; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; @@ -137,7 +138,7 @@ void correctLookUpStrategyForKey(QueryLookupStrategy.Key key, Class expectedClas .registerRowMapper(NumberFormat.class, numberFormatMapper); QueryLookupStrategy queryLookupStrategy = JdbcQueryLookupStrategy.create(key, publisher, callbacks, mappingContext, - converter, H2Dialect.INSTANCE, mappingConfiguration, operations, null, evaluationContextProvider); + converter, H2Dialect.INSTANCE, mappingConfiguration, operations, null, ValueExpressionDelegate.create()); assertThat(queryLookupStrategy).isInstanceOf(expectedClass); } @@ -157,7 +158,7 @@ private RepositoryQuery getRepositoryQuery(QueryLookupStrategy.Key key, String n QueryMappingConfiguration mappingConfiguration) { QueryLookupStrategy queryLookupStrategy = JdbcQueryLookupStrategy.create(key, publisher, callbacks, mappingContext, - converter, H2Dialect.INSTANCE, mappingConfiguration, operations, null, evaluationContextProvider); + converter, H2Dialect.INSTANCE, mappingConfiguration, operations, null, ValueExpressionDelegate.create()); Method method = ReflectionUtils.findMethod(MyRepository.class, name); return queryLookupStrategy.resolveQuery(method, metadata, projectionFactory, namedQueries); diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/DefaultR2dbcSpELExpressionEvaluator.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/DefaultR2dbcSpELExpressionEvaluator.java index 94159ac3eb..3c0bdd130c 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/DefaultR2dbcSpELExpressionEvaluator.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/DefaultR2dbcSpELExpressionEvaluator.java @@ -15,9 +15,11 @@ */ package org.springframework.data.r2dbc.repository.query; +import org.springframework.data.expression.ValueEvaluationContext; +import org.springframework.data.expression.ValueExpression; import org.springframework.data.mapping.model.SpELExpressionEvaluator; +import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.expression.EvaluationContext; -import org.springframework.expression.Expression; import org.springframework.expression.ExpressionParser; import org.springframework.r2dbc.core.Parameter; @@ -30,12 +32,12 @@ */ class DefaultR2dbcSpELExpressionEvaluator implements R2dbcSpELExpressionEvaluator { - private final ExpressionParser parser; + private final ValueExpressionDelegate delegate; - private final EvaluationContext context; + private final ValueEvaluationContext context; - DefaultR2dbcSpELExpressionEvaluator(ExpressionParser parser, EvaluationContext context) { - this.parser = parser; + DefaultR2dbcSpELExpressionEvaluator(ValueExpressionDelegate delegate, ValueEvaluationContext context) { + this.delegate = delegate; this.context = context; } @@ -51,12 +53,12 @@ public static R2dbcSpELExpressionEvaluator unsupported() { @Override public Parameter evaluate(String expression) { - Expression expr = parser.parseExpression(expression); + ValueExpression expr = delegate.parse(expression); - Object value = expr.getValue(context, Object.class); - Class valueType = expr.getValueType(context); + Object value = expr.evaluate(context); + Class valueType = value != null ? value.getClass() : Object.class; - return org.springframework.r2dbc.core.Parameter.fromOrEmpty(value, valueType != null ? valueType : Object.class); + return org.springframework.r2dbc.core.Parameter.fromOrEmpty(value, valueType); } /** diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionQuery.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionQuery.java index 4992af9243..3ccd70ea38 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionQuery.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionQuery.java @@ -18,7 +18,8 @@ import java.util.ArrayList; import java.util.List; -import org.springframework.data.repository.query.SpelQueryContext; +import org.springframework.data.expression.ValueExpressionParser; +import org.springframework.data.repository.query.ValueExpressionQueryRewriter; /** * Query using Spring Expression Language to indicate parameter bindings. Queries using SpEL use {@code :#{…}} to @@ -48,18 +49,17 @@ private ExpressionQuery(String query, List parameterBindings) * @param query the query string to parse. * @return the parsed {@link ExpressionQuery}. */ - public static ExpressionQuery create(String query) { + public static ExpressionQuery create(ValueExpressionParser parser, String query) { List parameterBindings = new ArrayList<>(); - SpelQueryContext queryContext = SpelQueryContext.of((counter, expression) -> { - + ValueExpressionQueryRewriter rewriter = ValueExpressionQueryRewriter.of(parser, (counter, expression) -> { String parameterName = String.format(SYNTHETIC_PARAMETER_TEMPLATE, counter); parameterBindings.add(new ParameterBinding(parameterName, expression)); return parameterName; }, String::concat); - SpelQueryContext.SpelExtractor parsed = queryContext.parse(query); + ValueExpressionQueryRewriter.ParsedQuery parsed = rewriter.parse(query); return new ExpressionQuery(parsed.getQueryString(), parameterBindings); } diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java index 80d70404a4..55c5f34819 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java @@ -22,6 +22,10 @@ import java.util.List; import java.util.Map; +import org.springframework.core.env.StandardEnvironment; +import org.springframework.data.expression.ReactiveValueEvaluationContextProvider; +import org.springframework.data.expression.ValueEvaluationContextProvider; +import org.springframework.data.expression.ValueExpressionParser; import org.springframework.data.r2dbc.convert.R2dbcConverter; import org.springframework.data.r2dbc.core.R2dbcEntityOperations; import org.springframework.data.r2dbc.core.ReactiveDataAccessStrategy; @@ -29,8 +33,10 @@ import org.springframework.data.r2dbc.repository.Query; import org.springframework.data.relational.repository.query.RelationalParameterAccessor; import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; +import org.springframework.data.repository.query.QueryMethodValueEvaluationContextAccessor; import org.springframework.data.repository.query.ReactiveQueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.ResultProcessor; +import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.data.spel.ExpressionDependencies; import org.springframework.expression.ExpressionParser; import org.springframework.expression.spel.standard.SpelExpressionParser; @@ -52,10 +58,10 @@ public class StringBasedR2dbcQuery extends AbstractR2dbcQuery { private final ExpressionQuery expressionQuery; private final ExpressionEvaluatingParameterBinder binder; - private final ExpressionParser expressionParser; - private final ReactiveQueryMethodEvaluationContextProvider evaluationContextProvider; private final ExpressionDependencies expressionDependencies; private final ReactiveDataAccessStrategy dataAccessStrategy; + private final ValueExpressionDelegate valueExpressionDelegate; + private final ValueEvaluationContextProvider valueContextProvider; /** * Creates a new {@link StringBasedR2dbcQuery} for the given {@link StringBasedR2dbcQuery}, {@link DatabaseClient}, @@ -67,7 +73,9 @@ public class StringBasedR2dbcQuery extends AbstractR2dbcQuery { * @param dataAccessStrategy must not be {@literal null}. * @param expressionParser must not be {@literal null}. * @param evaluationContextProvider must not be {@literal null}. + * @deprecated use the constructor version with {@link ValueExpressionDelegate} */ + @Deprecated(since = "3.4") public StringBasedR2dbcQuery(R2dbcQueryMethod queryMethod, R2dbcEntityOperations entityOperations, R2dbcConverter converter, ReactiveDataAccessStrategy dataAccessStrategy, ExpressionParser expressionParser, ReactiveQueryMethodEvaluationContextProvider evaluationContextProvider) { @@ -79,26 +87,60 @@ public StringBasedR2dbcQuery(R2dbcQueryMethod queryMethod, R2dbcEntityOperations * Create a new {@link StringBasedR2dbcQuery} for the given {@code query}, {@link R2dbcQueryMethod}, * {@link DatabaseClient}, {@link SpelExpressionParser}, and {@link QueryMethodEvaluationContextProvider}. * + * @param query must not be {@literal null}. * @param method must not be {@literal null}. * @param entityOperations must not be {@literal null}. * @param converter must not be {@literal null}. * @param dataAccessStrategy must not be {@literal null}. * @param expressionParser must not be {@literal null}. * @param evaluationContextProvider must not be {@literal null}. + * @deprecated use the constructor version with {@link ValueExpressionDelegate} */ + @Deprecated(since = "3.4") public StringBasedR2dbcQuery(String query, R2dbcQueryMethod method, R2dbcEntityOperations entityOperations, R2dbcConverter converter, ReactiveDataAccessStrategy dataAccessStrategy, ExpressionParser expressionParser, ReactiveQueryMethodEvaluationContextProvider evaluationContextProvider) { + this(query, method, entityOperations, converter, dataAccessStrategy, new ValueExpressionDelegate(new QueryMethodValueEvaluationContextAccessor(new StandardEnvironment(), evaluationContextProvider.getEvaluationContextProvider()), ValueExpressionParser.create(() -> expressionParser))); + } + + /** + * Create a new {@link StringBasedR2dbcQuery} for the given {@code query}, {@link R2dbcQueryMethod}, + * {@link DatabaseClient}, {@link SpelExpressionParser}, and {@link QueryMethodEvaluationContextProvider}. + * + * @param method must not be {@literal null}. + * @param entityOperations must not be {@literal null}. + * @param converter must not be {@literal null}. + * @param dataAccessStrategy must not be {@literal null}. + * @param valueExpressionDelegate must not be {@literal null}. + */ + public StringBasedR2dbcQuery(R2dbcQueryMethod method, R2dbcEntityOperations entityOperations, + R2dbcConverter converter, ReactiveDataAccessStrategy dataAccessStrategy, ValueExpressionDelegate valueExpressionDelegate) { + this(method.getRequiredAnnotatedQuery(), method, entityOperations, converter, dataAccessStrategy, valueExpressionDelegate); + } + + /** + * Create a new {@link StringBasedR2dbcQuery} for the given {@code query}, {@link R2dbcQueryMethod}, + * {@link DatabaseClient}, {@link SpelExpressionParser}, and {@link QueryMethodEvaluationContextProvider}. + * + * @param method must not be {@literal null}. + * @param entityOperations must not be {@literal null}. + * @param converter must not be {@literal null}. + * @param dataAccessStrategy must not be {@literal null}. + * @param valueExpressionDelegate must not be {@literal null}. + */ + public StringBasedR2dbcQuery(String query, R2dbcQueryMethod method, R2dbcEntityOperations entityOperations, + R2dbcConverter converter, ReactiveDataAccessStrategy dataAccessStrategy, ValueExpressionDelegate valueExpressionDelegate) { super(method, entityOperations, converter); - this.expressionParser = expressionParser; - this.evaluationContextProvider = evaluationContextProvider; + this.valueExpressionDelegate = valueExpressionDelegate; Assert.hasText(query, "Query must not be empty"); this.dataAccessStrategy = dataAccessStrategy; - this.expressionQuery = ExpressionQuery.create(query); + this.expressionQuery = ExpressionQuery.create(valueExpressionDelegate, query); this.binder = new ExpressionEvaluatingParameterBinder(expressionQuery, dataAccessStrategy); + this.valueContextProvider = valueExpressionDelegate.createValueContextProvider( + method.getParameters()); this.expressionDependencies = createExpressionDependencies(); if (method.isSliceQuery()) { @@ -126,7 +168,7 @@ private ExpressionDependencies createExpressionDependencies() { List dependencies = new ArrayList<>(); for (ExpressionQuery.ParameterBinding binding : expressionQuery.getBindings()) { - dependencies.add(ExpressionDependencies.discover(expressionParser.parseExpression(binding.getExpression()))); + dependencies.add(valueExpressionDelegate.parse(binding.getExpression()).getExpressionDependencies()); } return ExpressionDependencies.merged(dependencies); @@ -160,11 +202,11 @@ Class resolveResultType(ResultProcessor resultProcessor) { } private Mono getSpelEvaluator(RelationalParameterAccessor accessor) { - - return evaluationContextProvider - .getEvaluationContextLater(getQueryMethod().getParameters(), accessor.getValues(), expressionDependencies) + Assert.isInstanceOf(ReactiveValueEvaluationContextProvider.class, valueContextProvider, "ValueEvaluationContextProvider must be reactive"); + return ((ReactiveValueEvaluationContextProvider) valueContextProvider) + .getEvaluationContextLater(accessor.getValues(), expressionDependencies) . map( - context -> new DefaultR2dbcSpELExpressionEvaluator(expressionParser, context)) + context -> new DefaultR2dbcSpELExpressionEvaluator(valueExpressionDelegate, context)) .defaultIfEmpty(DefaultR2dbcSpELExpressionEvaluator.unsupported()); } diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/CachingExpressionParser.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/CachingExpressionParser.java deleted file mode 100644 index adc4ac91b1..0000000000 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/CachingExpressionParser.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright 2020-2024 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.r2dbc.repository.support; - -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -import org.springframework.expression.Expression; -import org.springframework.expression.ExpressionParser; -import org.springframework.expression.ParseException; -import org.springframework.expression.ParserContext; - -/** - * Caching variant of {@link ExpressionParser}. This implementation does not support - * {@link #parseExpression(String, ParserContext) parsing with ParseContext}. - * - * @author Mark Paluch - * @since 1.2 - */ -class CachingExpressionParser implements ExpressionParser { - - private final ExpressionParser delegate; - private final Map cache = new ConcurrentHashMap<>(); - - CachingExpressionParser(ExpressionParser delegate) { - this.delegate = delegate; - } - - @Override - public Expression parseExpression(String expressionString) throws ParseException { - return cache.computeIfAbsent(expressionString, delegate::parseExpression); - } - - @Override - public Expression parseExpression(String expressionString, ParserContext context) throws ParseException { - throw new UnsupportedOperationException("Parsing using ParserContext is not supported"); - } -} diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java index 3d49bf3804..03580f746d 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java @@ -37,13 +37,12 @@ import org.springframework.data.repository.core.RepositoryInformation; import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.core.support.ReactiveRepositoryFactorySupport; +import org.springframework.data.repository.query.CachingValueExpressionDelegate; import org.springframework.data.repository.query.QueryLookupStrategy; import org.springframework.data.repository.query.QueryLookupStrategy.Key; -import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.ReactiveQueryMethodEvaluationContextProvider; import org.springframework.data.repository.query.RepositoryQuery; -import org.springframework.expression.ExpressionParser; -import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.lang.Nullable; import org.springframework.r2dbc.core.DatabaseClient; import org.springframework.util.Assert; @@ -56,8 +55,6 @@ */ public class R2dbcRepositoryFactory extends ReactiveRepositoryFactorySupport { - private static final SpelExpressionParser EXPRESSION_PARSER = new SpelExpressionParser(); - private final DatabaseClient databaseClient; private final ReactiveDataAccessStrategy dataAccessStrategy; private final MappingContext, ? extends RelationalPersistentProperty> mappingContext; @@ -116,11 +113,9 @@ protected Object getTargetRepository(RepositoryInformation information) { } @Override - protected Optional getQueryLookupStrategy(@Nullable Key key, - QueryMethodEvaluationContextProvider evaluationContextProvider) { - return Optional.of(new R2dbcQueryLookupStrategy(this.operations, - (ReactiveQueryMethodEvaluationContextProvider) evaluationContextProvider, this.converter, - this.dataAccessStrategy)); + protected Optional getQueryLookupStrategy(Key key, + ValueExpressionDelegate valueExpressionDelegate) { + return Optional.of(new R2dbcQueryLookupStrategy(operations, new CachingValueExpressionDelegate(valueExpressionDelegate), converter, dataAccessStrategy)); } public RelationalEntityInformation getEntityInformation(Class domainClass) { @@ -145,19 +140,17 @@ private RelationalEntityInformation getEntityInformation(Class private static class R2dbcQueryLookupStrategy extends RelationalQueryLookupStrategy { private final R2dbcEntityOperations entityOperations; - private final ReactiveQueryMethodEvaluationContextProvider evaluationContextProvider; private final R2dbcConverter converter; + private final ValueExpressionDelegate delegate; private final ReactiveDataAccessStrategy dataAccessStrategy; - private final ExpressionParser parser = new CachingExpressionParser(EXPRESSION_PARSER); R2dbcQueryLookupStrategy(R2dbcEntityOperations entityOperations, - ReactiveQueryMethodEvaluationContextProvider evaluationContextProvider, R2dbcConverter converter, + ValueExpressionDelegate delegate, R2dbcConverter converter, ReactiveDataAccessStrategy dataAccessStrategy) { super(converter.getMappingContext(), dataAccessStrategy.getDialect()); - + this.delegate = delegate; this.entityOperations = entityOperations; - this.evaluationContextProvider = evaluationContextProvider; this.converter = converter; this.dataAccessStrategy = dataAccessStrategy; } @@ -175,8 +168,7 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata metadata, : queryMethod.getRequiredAnnotatedQuery(); query = evaluateTableExpressions(metadata, query); - return new StringBasedR2dbcQuery(query, queryMethod, this.entityOperations, this.converter, - this.dataAccessStrategy, parser, this.evaluationContextProvider); + return new StringBasedR2dbcQuery(query, queryMethod, this.entityOperations, this.converter, this.dataAccessStrategy, this.delegate); } else { return new PartTreeR2dbcQuery(queryMethod, this.entityOperations, this.converter, this.dataAccessStrategy); diff --git a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/ExpressionQueryUnitTests.java b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/ExpressionQueryUnitTests.java index d57c2163b3..4c080cf13f 100644 --- a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/ExpressionQueryUnitTests.java +++ b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/ExpressionQueryUnitTests.java @@ -17,9 +17,10 @@ import static org.assertj.core.api.Assertions.*; -import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; +import org.springframework.data.expression.ValueExpressionParser; + /** * Unit tests for {@link ExpressionQuery}. * @@ -32,18 +33,15 @@ class ExpressionQueryUnitTests { void bindsMultipleSpelParametersCorrectly() { ExpressionQuery query = ExpressionQuery - .create("INSERT IGNORE INTO table (x, y) VALUES (:#{#point.x}, :#{#point.y})"); + .create(ValueExpressionParser.create(), "INSERT IGNORE INTO table (x, y) VALUES (:#{#point.x}, :${point.y})"); assertThat(query.getQuery()) .isEqualTo("INSERT IGNORE INTO table (x, y) VALUES (:__synthetic_0__, :__synthetic_1__)"); - SoftAssertions.assertSoftly(softly -> { - - softly.assertThat(query.getBindings()).hasSize(2); - softly.assertThat(query.getBindings().get(0).getExpression()).isEqualTo("#point.x"); - softly.assertThat(query.getBindings().get(0).getParameterName()).isEqualTo("__synthetic_0__"); - softly.assertThat(query.getBindings().get(1).getExpression()).isEqualTo("#point.y"); - softly.assertThat(query.getBindings().get(1).getParameterName()).isEqualTo("__synthetic_1__"); - }); + assertThat(query.getBindings()).hasSize(2); + assertThat(query.getBindings().get(0).getExpression()).isEqualTo("#{#point.x}"); + assertThat(query.getBindings().get(0).getParameterName()).isEqualTo("__synthetic_0__"); + assertThat(query.getBindings().get(1).getExpression()).isEqualTo("${point.y}"); + assertThat(query.getBindings().get(1).getParameterName()).isEqualTo("__synthetic_1__"); } } diff --git a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQueryUnitTests.java b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQueryUnitTests.java index 4b9e40f0d0..3ea456ff2b 100644 --- a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQueryUnitTests.java +++ b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQueryUnitTests.java @@ -25,6 +25,7 @@ import java.lang.reflect.Method; import java.time.LocalDate; +import java.util.Collections; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -33,8 +34,10 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; + import org.springframework.data.domain.Limit; import org.springframework.data.domain.Sort; +import org.springframework.data.expression.ValueExpressionParser; import org.springframework.data.projection.ProjectionFactory; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.r2dbc.convert.MappingR2dbcConverter; @@ -51,8 +54,10 @@ import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.core.support.AbstractRepositoryMetadata; import org.springframework.data.repository.query.Param; -import org.springframework.data.repository.query.ReactiveQueryMethodEvaluationContextProvider; +import org.springframework.data.repository.query.QueryMethodValueEvaluationContextAccessor; +import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.mock.env.MockEnvironment; import org.springframework.r2dbc.core.DatabaseClient; import org.springframework.r2dbc.core.PreparedOperation; import org.springframework.r2dbc.core.binding.BindTarget; @@ -67,7 +72,7 @@ @MockitoSettings(strictness = Strictness.LENIENT) public class StringBasedR2dbcQueryUnitTests { - private static final SpelExpressionParser PARSER = new SpelExpressionParser(); + private static final ValueExpressionParser PARSER = ValueExpressionParser.create(SpelExpressionParser::new); @Mock private R2dbcEntityOperations entityOperations; @Mock private BindTarget bindTarget; @@ -77,6 +82,7 @@ public class StringBasedR2dbcQueryUnitTests { private ReactiveDataAccessStrategy accessStrategy; private ProjectionFactory factory; private RepositoryMetadata metadata; + private MockEnvironment environment; @BeforeEach void setUp() { @@ -86,6 +92,7 @@ void setUp() { this.accessStrategy = new DefaultReactiveDataAccessStrategy(PostgresDialect.INSTANCE, converter); this.metadata = AbstractRepositoryMetadata.getMetadata(SampleRepository.class); this.factory = new SpelAwareProxyProjectionFactory(); + this.environment = new MockEnvironment(); } @Test @@ -322,8 +329,10 @@ private StringBasedR2dbcQuery getQueryMethod(String name, Class... args) { R2dbcQueryMethod queryMethod = new R2dbcQueryMethod(method, metadata, factory, converter.getMappingContext()); - return new StringBasedR2dbcQuery(queryMethod, entityOperations, converter, accessStrategy, PARSER, - ReactiveQueryMethodEvaluationContextProvider.DEFAULT); + QueryMethodValueEvaluationContextAccessor accessor = new QueryMethodValueEvaluationContextAccessor( + environment, Collections.emptySet()); + + return new StringBasedR2dbcQuery(queryMethod, entityOperations, converter, accessStrategy, new ValueExpressionDelegate(accessor, PARSER)); } @SuppressWarnings("unused") diff --git a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactoryBeanUnitTests.java b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactoryBeanUnitTests.java index fa2954c378..329ed876fa 100644 --- a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactoryBeanUnitTests.java +++ b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactoryBeanUnitTests.java @@ -24,7 +24,8 @@ import org.springframework.data.r2dbc.core.R2dbcEntityTemplate; import org.springframework.data.r2dbc.dialect.H2Dialect; import org.springframework.data.r2dbc.repository.R2dbcRepository; -import org.springframework.data.repository.query.ReactiveExtensionAwareQueryMethodEvaluationContextProvider; +import org.springframework.data.spel.EvaluationContextProvider; +import org.springframework.data.spel.ReactiveExtensionAwareEvaluationContextProvider; import org.springframework.r2dbc.core.DatabaseClient; import org.springframework.test.util.ReflectionTestUtils; @@ -49,8 +50,8 @@ void shouldConfigureReactiveExtensionAwareQueryMethodEvaluationContextProvider() Object factory = ReflectionTestUtils.getField(factoryBean, "factory"); Object evaluationContextProvider = ReflectionTestUtils.getField(factory, "evaluationContextProvider"); - assertThat(evaluationContextProvider).isInstanceOf(ReactiveExtensionAwareQueryMethodEvaluationContextProvider.class) - .isNotEqualTo(ReactiveExtensionAwareQueryMethodEvaluationContextProvider.DEFAULT); + assertThat(evaluationContextProvider).isInstanceOf(ReactiveExtensionAwareEvaluationContextProvider.class) + .isNotEqualTo(EvaluationContextProvider.DEFAULT); } static class Person {} From 102682bb147a733ed66c107f0522fb031fdcc7b7 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 8 Oct 2024 09:43:27 +0200 Subject: [PATCH 3/3] Polishing. Simplify R2DBC expression handling. Use new ValueExpression API instead of holding parameter binding duplicates. Reformat code. Add author tags. --- .../query/StringBasedJdbcQuery.java | 51 ++++++------ .../support/JdbcRepositoryFactory.java | 10 ++- .../query/StringBasedJdbcQueryUnitTests.java | 5 +- .../DefaultR2dbcSpELExpressionEvaluator.java | 78 ------------------- .../ExpressionEvaluatingParameterBinder.java | 30 +++++-- .../repository/query/ExpressionQuery.java | 52 +++---------- .../query/R2dbcSpELExpressionEvaluator.java | 35 --------- .../query/StringBasedR2dbcQuery.java | 39 +++++----- .../support/R2dbcRepositoryFactory.java | 3 +- .../query/ExpressionQueryUnitTests.java | 18 +++-- 10 files changed, 96 insertions(+), 225 deletions(-) delete mode 100644 spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/DefaultR2dbcSpELExpressionEvaluator.java delete mode 100644 spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcSpELExpressionEvaluator.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java index 2130ba0bb5..dd8e4a35f7 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java @@ -20,18 +20,17 @@ import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.sql.SQLType; -import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.function.Function; import java.util.function.Supplier; import org.springframework.beans.BeanInstantiationException; import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanFactory; +import org.springframework.core.env.StandardEnvironment; import org.springframework.data.expression.ValueEvaluationContext; import org.springframework.data.expression.ValueExpressionParser; import org.springframework.data.jdbc.core.convert.JdbcColumnTypes; @@ -51,7 +50,6 @@ import org.springframework.data.repository.query.ValueExpressionQueryRewriter; import org.springframework.data.util.Lazy; import org.springframework.data.util.TypeInformation; -import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.jdbc.core.ResultSetExtractor; import org.springframework.jdbc.core.RowMapper; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; @@ -74,6 +72,7 @@ * @author Chirag Tailor * @author Christopher Klein * @author Mikhail Polivakha + * @author Marcin Grzejszczak * @since 2.0 */ public class StringBasedJdbcQuery extends AbstractJdbcQuery { @@ -82,13 +81,11 @@ public class StringBasedJdbcQuery extends AbstractJdbcQuery { private final JdbcConverter converter; private final RowMapperFactory rowMapperFactory; private final ValueExpressionQueryRewriter.ParsedQuery parsedQuery; - private final boolean containsSpelExpressions; private final String query; private final CachedRowMapperFactory cachedRowMapperFactory; private final CachedResultSetExtractorFactory cachedResultSetExtractorFactory; private final ValueExpressionDelegate delegate; - private final List> parameterBindings; /** * Creates a new {@link StringBasedJdbcQuery} for the given {@link JdbcQueryMethod}, {@link RelationalMappingContext} @@ -185,17 +182,11 @@ public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedPara this.cachedResultSetExtractorFactory = new CachedResultSetExtractorFactory( this.cachedRowMapperFactory::getRowMapper); - this.parameterBindings = new ArrayList<>(); - - ValueExpressionQueryRewriter rewriter = ValueExpressionQueryRewriter.of(delegate, (counter, expression) -> { - String newName = String.format("__$synthetic$__%d", counter + 1); - parameterBindings.add(new AbstractMap.SimpleEntry<>(newName, expression)); - return newName; - }, String::concat); + ValueExpressionQueryRewriter rewriter = ValueExpressionQueryRewriter.of(delegate, + (counter, expression) -> String.format("__$synthetic$__%d", counter + 1), String::concat); this.query = query; this.parsedQuery = rewriter.parse(this.query); - this.containsSpelExpressions = !this.parsedQuery.getQueryString().equals(this.query); this.delegate = delegate; } @@ -216,9 +207,10 @@ public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedPara public StringBasedJdbcQuery(String query, JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations, RowMapperFactory rowMapperFactory, JdbcConverter converter, QueryMethodEvaluationContextProvider evaluationContextProvider) { - this(query, queryMethod, operations, rowMapperFactory, converter, new CachingValueExpressionDelegate(new QueryMethodValueEvaluationContextAccessor(null, - rootObject -> evaluationContextProvider.getEvaluationContext(queryMethod.getParameters(), new Object[] { rootObject })), ValueExpressionParser.create( - SpelExpressionParser::new))); + this(query, queryMethod, operations, rowMapperFactory, converter, new CachingValueExpressionDelegate( + new QueryMethodValueEvaluationContextAccessor(new StandardEnvironment(), rootObject -> evaluationContextProvider + .getEvaluationContext(queryMethod.getParameters(), new Object[] { rootObject })), + ValueExpressionParser.create())); } @Override @@ -230,18 +222,22 @@ public Object execute(Object[] objects) { JdbcQueryExecution queryExecution = createJdbcQueryExecution(accessor, processor); MapSqlParameterSource parameterMap = this.bindParameters(accessor); - return queryExecution.execute(processSpelExpressions(objects, accessor.getBindableParameters(), parameterMap), parameterMap); + return queryExecution.execute(evaluateExpressions(objects, accessor.getBindableParameters(), parameterMap), + parameterMap); } - private String processSpelExpressions(Object[] objects, Parameters bindableParameters, MapSqlParameterSource parameterMap) { + private String evaluateExpressions(Object[] objects, Parameters bindableParameters, + MapSqlParameterSource parameterMap) { + + if (parsedQuery.hasParameterBindings()) { - if (containsSpelExpressions) { ValueEvaluationContext evaluationContext = delegate.createValueContextProvider(bindableParameters) .getEvaluationContext(objects); - for (Map.Entry entry : parameterBindings) { - parameterMap.addValue( - entry.getKey(), delegate.parse(entry.getValue()).evaluate(evaluationContext)); - } + + parsedQuery.getParameterMap().forEach((paramName, valueExpression) -> { + parameterMap.addValue(paramName, valueExpression.evaluate(evaluationContext)); + }); + return parsedQuery.getQueryString(); } @@ -253,13 +249,12 @@ private JdbcQueryExecution createJdbcQueryExecution(RelationalParameterAccess if (getQueryMethod().isModifyingQuery()) { return createModifyingQueryExecutor(); - } else { + } - Supplier> rowMapper = () -> determineRowMapper(processor, accessor.findDynamicProjection() != null); - ResultSetExtractor resultSetExtractor = determineResultSetExtractor(rowMapper); + Supplier> rowMapper = () -> determineRowMapper(processor, accessor.findDynamicProjection() != null); + ResultSetExtractor resultSetExtractor = determineResultSetExtractor(rowMapper); - return createReadingQueryExecution(resultSetExtractor, rowMapper); - } + return createReadingQueryExecution(resultSetExtractor, rowMapper); } private MapSqlParameterSource bindParameters(RelationalParameterAccessor accessor) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java index 31683a0da7..78c8dee1f5 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/repository/support/JdbcRepositoryFactory.java @@ -32,6 +32,7 @@ import org.springframework.data.repository.core.RepositoryMetadata; import org.springframework.data.repository.core.support.PersistentEntityInformation; import org.springframework.data.repository.core.support.RepositoryFactorySupport; +import org.springframework.data.repository.query.CachingValueExpressionDelegate; import org.springframework.data.repository.query.QueryLookupStrategy; import org.springframework.data.repository.query.ValueExpressionDelegate; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; @@ -48,6 +49,7 @@ * @author Hebert Coelho * @author Diego Krupitza * @author Christopher Klein + * @author Marcin Grzejszczak */ public class JdbcRepositoryFactory extends RepositoryFactorySupport { @@ -57,7 +59,7 @@ public class JdbcRepositoryFactory extends RepositoryFactorySupport { private final DataAccessStrategy accessStrategy; private final NamedParameterJdbcOperations operations; private final Dialect dialect; - @Nullable private BeanFactory beanFactory; + private @Nullable BeanFactory beanFactory; private QueryMappingConfiguration queryMappingConfiguration = QueryMappingConfiguration.EMPTY; private EntityCallbacks entityCallbacks; @@ -132,10 +134,12 @@ protected Class getRepositoryBaseClass(RepositoryMetadata repositoryMetadata) return SimpleJdbcRepository.class; } - @Override protected Optional getQueryLookupStrategy(QueryLookupStrategy.Key key, + @Override + protected Optional getQueryLookupStrategy(@Nullable QueryLookupStrategy.Key key, ValueExpressionDelegate valueExpressionDelegate) { return Optional.of(JdbcQueryLookupStrategy.create(key, publisher, entityCallbacks, context, converter, dialect, - queryMappingConfiguration, operations, beanFactory, valueExpressionDelegate)); + queryMappingConfiguration, operations, beanFactory, + new CachingValueExpressionDelegate(valueExpressionDelegate))); } /** diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java index 5a937210d2..6e8743daa2 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQueryUnitTests.java @@ -34,7 +34,6 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; -import org.springframework.context.support.StaticApplicationContext; import org.springframework.core.convert.converter.Converter; import org.springframework.core.env.StandardEnvironment; import org.springframework.dao.DataAccessException; @@ -79,6 +78,7 @@ * @author Dennis Effing * @author Chirag Tailor * @author Christopher Klein + * @author Marcin Grzejszczak */ class StringBasedJdbcQueryUnitTests { @@ -95,8 +95,7 @@ void setup() { this.operations = mock(NamedParameterJdbcOperations.class); this.context = mock(RelationalMappingContext.class, RETURNS_DEEP_STUBS); this.converter = new MappingJdbcConverter(context, mock(RelationResolver.class)); - QueryMethodValueEvaluationContextAccessor accessor = new QueryMethodValueEvaluationContextAccessor(new StandardEnvironment(), new StaticApplicationContext()); - this.delegate = new ValueExpressionDelegate(accessor, ValueExpressionParser.create()); + this.delegate = ValueExpressionDelegate.create(); } @Test // DATAJDBC-165 diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/DefaultR2dbcSpELExpressionEvaluator.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/DefaultR2dbcSpELExpressionEvaluator.java deleted file mode 100644 index 3c0bdd130c..0000000000 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/DefaultR2dbcSpELExpressionEvaluator.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright 2020-2024 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.r2dbc.repository.query; - -import org.springframework.data.expression.ValueEvaluationContext; -import org.springframework.data.expression.ValueExpression; -import org.springframework.data.mapping.model.SpELExpressionEvaluator; -import org.springframework.data.repository.query.ValueExpressionDelegate; -import org.springframework.expression.EvaluationContext; -import org.springframework.expression.ExpressionParser; -import org.springframework.r2dbc.core.Parameter; - -/** - * Simple {@link R2dbcSpELExpressionEvaluator} implementation using {@link ExpressionParser} and - * {@link EvaluationContext}. - * - * @author Mark Paluch - * @since 1.2 - */ -class DefaultR2dbcSpELExpressionEvaluator implements R2dbcSpELExpressionEvaluator { - - private final ValueExpressionDelegate delegate; - - private final ValueEvaluationContext context; - - DefaultR2dbcSpELExpressionEvaluator(ValueExpressionDelegate delegate, ValueEvaluationContext context) { - this.delegate = delegate; - this.context = context; - } - - /** - * Return a {@link SpELExpressionEvaluator} that does not support expression evaluation. - * - * @return a {@link SpELExpressionEvaluator} that does not support expression evaluation. - */ - public static R2dbcSpELExpressionEvaluator unsupported() { - return NoOpExpressionEvaluator.INSTANCE; - } - - @Override - public Parameter evaluate(String expression) { - - ValueExpression expr = delegate.parse(expression); - - Object value = expr.evaluate(context); - Class valueType = value != null ? value.getClass() : Object.class; - - return org.springframework.r2dbc.core.Parameter.fromOrEmpty(value, valueType); - } - - /** - * {@link SpELExpressionEvaluator} that does not support SpEL evaluation. - * - * @author Mark Paluch - */ - enum NoOpExpressionEvaluator implements R2dbcSpELExpressionEvaluator { - - INSTANCE; - - @Override - public Parameter evaluate(String expression) { - throw new UnsupportedOperationException("Expression evaluation not supported"); - } - } -} diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionEvaluatingParameterBinder.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionEvaluatingParameterBinder.java index 82d6a90677..1d1b0466f7 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionEvaluatingParameterBinder.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionEvaluatingParameterBinder.java @@ -15,13 +15,13 @@ */ package org.springframework.data.r2dbc.repository.query; -import static org.springframework.data.r2dbc.repository.query.ExpressionQuery.*; - import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; +import org.springframework.data.expression.ValueEvaluationContext; +import org.springframework.data.expression.ValueExpression; import org.springframework.data.r2dbc.core.ReactiveDataAccessStrategy; import org.springframework.data.r2dbc.dialect.BindTargetBinder; import org.springframework.data.relational.repository.query.RelationalParameterAccessor; @@ -64,26 +64,40 @@ class ExpressionEvaluatingParameterBinder { * @param evaluator must not be {@literal null}. */ void bind(BindTarget bindTarget, - RelationalParameterAccessor parameterAccessor, R2dbcSpELExpressionEvaluator evaluator) { + RelationalParameterAccessor parameterAccessor, ValueEvaluationContext evaluationContext) { Object[] values = parameterAccessor.getValues(); Parameters bindableParameters = parameterAccessor.getBindableParameters(); - bindExpressions(bindTarget, evaluator); + bindExpressions(bindTarget, evaluationContext); bindParameters(bindTarget, parameterAccessor.hasBindableNullValue(), values, bindableParameters); } private void bindExpressions(BindTarget bindSpec, - R2dbcSpELExpressionEvaluator evaluator) { + ValueEvaluationContext evaluationContext) { BindTargetBinder binder = new BindTargetBinder(bindSpec); - for (ParameterBinding binding : expressionQuery.getBindings()) { + + expressionQuery.getBindings().forEach((paramName, valueExpression) -> { org.springframework.r2dbc.core.Parameter valueForBinding = getBindValue( - evaluator.evaluate(binding.getExpression())); + evaluate(valueExpression, evaluationContext)); + + binder.bind(paramName, valueForBinding); + }); + } + + private org.springframework.r2dbc.core.Parameter evaluate(ValueExpression expression, + ValueEvaluationContext context) { - binder.bind(binding.getParameterName(), valueForBinding); + Object value = expression.evaluate(context); + Class valueType = value != null ? value.getClass() : null; + + if (valueType == null) { + valueType = expression.getValueType(context); } + + return org.springframework.r2dbc.core.Parameter.fromOrEmpty(value, valueType == null ? Object.class : valueType); } private void bindParameters(BindTarget bindSpec, diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionQuery.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionQuery.java index 3ccd70ea38..23b85b6a60 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionQuery.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/ExpressionQuery.java @@ -15,9 +15,9 @@ */ package org.springframework.data.r2dbc.repository.query; -import java.util.ArrayList; -import java.util.List; +import java.util.Map; +import org.springframework.data.expression.ValueExpression; import org.springframework.data.expression.ValueExpressionParser; import org.springframework.data.repository.query.ValueExpressionQueryRewriter; @@ -34,13 +34,11 @@ class ExpressionQuery { private static final String SYNTHETIC_PARAMETER_TEMPLATE = "__synthetic_%d__"; private final String query; + private final Map parameterMap; - private final List parameterBindings; - - private ExpressionQuery(String query, List parameterBindings) { - + private ExpressionQuery(String query, Map parameterMap) { this.query = query; - this.parameterBindings = parameterBindings; + this.parameterMap = parameterMap; } /** @@ -51,55 +49,25 @@ private ExpressionQuery(String query, List parameterBindings) */ public static ExpressionQuery create(ValueExpressionParser parser, String query) { - List parameterBindings = new ArrayList<>(); - - ValueExpressionQueryRewriter rewriter = ValueExpressionQueryRewriter.of(parser, (counter, expression) -> { - String parameterName = String.format(SYNTHETIC_PARAMETER_TEMPLATE, counter); - parameterBindings.add(new ParameterBinding(parameterName, expression)); - return parameterName; - }, String::concat); + ValueExpressionQueryRewriter rewriter = ValueExpressionQueryRewriter.of(parser, + (counter, expression) -> String.format(SYNTHETIC_PARAMETER_TEMPLATE, counter), String::concat); ValueExpressionQueryRewriter.ParsedQuery parsed = rewriter.parse(query); - return new ExpressionQuery(parsed.getQueryString(), parameterBindings); + return new ExpressionQuery(parsed.getQueryString(), parsed.getParameterMap()); } public String getQuery() { return query; } - public List getBindings() { - return parameterBindings; + public Map getBindings() { + return parameterMap; } - @Override public String toString() { return query; } - /** - * A SpEL parameter binding. - * - * @author Mark Paluch - */ - static class ParameterBinding { - - private final String parameterName; - private final String expression; - - private ParameterBinding(String parameterName, String expression) { - - this.expression = expression; - this.parameterName = parameterName; - } - - String getExpression() { - return expression; - } - - String getParameterName() { - return parameterName; - } - } } diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcSpELExpressionEvaluator.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcSpELExpressionEvaluator.java deleted file mode 100644 index a8522cc568..0000000000 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/R2dbcSpELExpressionEvaluator.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2020-2024 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.r2dbc.repository.query; - -import org.springframework.r2dbc.core.Parameter; - -/** - * SPI for components that can evaluate Spring EL expressions and return {@link Parameter}. - * - * @author Mark Paluch - * @since 1.2 - */ -interface R2dbcSpELExpressionEvaluator { - - /** - * Evaluates the given expression. - * - * @param expression - * @return - */ - Parameter evaluate(String expression); -} diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java index 55c5f34819..26292a9ac3 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/query/StringBasedR2dbcQuery.java @@ -24,6 +24,7 @@ import org.springframework.core.env.StandardEnvironment; import org.springframework.data.expression.ReactiveValueEvaluationContextProvider; +import org.springframework.data.expression.ValueEvaluationContext; import org.springframework.data.expression.ValueEvaluationContextProvider; import org.springframework.data.expression.ValueExpressionParser; import org.springframework.data.r2dbc.convert.R2dbcConverter; @@ -53,6 +54,7 @@ * named parameters (if enabled on {@link DatabaseClient}) and SpEL expressions enclosed with {@code :#{…}}. * * @author Mark Paluch + * @author Marcin Grzejszczak */ public class StringBasedR2dbcQuery extends AbstractR2dbcQuery { @@ -60,8 +62,7 @@ public class StringBasedR2dbcQuery extends AbstractR2dbcQuery { private final ExpressionEvaluatingParameterBinder binder; private final ExpressionDependencies expressionDependencies; private final ReactiveDataAccessStrategy dataAccessStrategy; - private final ValueExpressionDelegate valueExpressionDelegate; - private final ValueEvaluationContextProvider valueContextProvider; + private final ReactiveValueEvaluationContextProvider valueContextProvider; /** * Creates a new {@link StringBasedR2dbcQuery} for the given {@link StringBasedR2dbcQuery}, {@link DatabaseClient}, @@ -132,17 +133,22 @@ public StringBasedR2dbcQuery(String query, R2dbcQueryMethod method, R2dbcEntityO R2dbcConverter converter, ReactiveDataAccessStrategy dataAccessStrategy, ValueExpressionDelegate valueExpressionDelegate) { super(method, entityOperations, converter); - this.valueExpressionDelegate = valueExpressionDelegate; Assert.hasText(query, "Query must not be empty"); this.dataAccessStrategy = dataAccessStrategy; this.expressionQuery = ExpressionQuery.create(valueExpressionDelegate, query); this.binder = new ExpressionEvaluatingParameterBinder(expressionQuery, dataAccessStrategy); - this.valueContextProvider = valueExpressionDelegate.createValueContextProvider( - method.getParameters()); + + ValueEvaluationContextProvider valueContextProvider = valueExpressionDelegate + .createValueContextProvider(method.getParameters()); + Assert.isInstanceOf(ReactiveValueEvaluationContextProvider.class, valueContextProvider, + "ValueEvaluationContextProvider must be reactive"); + + this.valueContextProvider = (ReactiveValueEvaluationContextProvider) valueContextProvider; this.expressionDependencies = createExpressionDependencies(); + if (method.isSliceQuery()) { throw new UnsupportedOperationException( "Slice queries are not supported using string-based queries; Offending method: " + method); @@ -167,9 +173,8 @@ private ExpressionDependencies createExpressionDependencies() { List dependencies = new ArrayList<>(); - for (ExpressionQuery.ParameterBinding binding : expressionQuery.getBindings()) { - dependencies.add(valueExpressionDelegate.parse(binding.getExpression()).getExpressionDependencies()); - } + expressionQuery.getBindings() + .forEach((s, valueExpression) -> dependencies.add(valueExpression.getExpressionDependencies())); return ExpressionDependencies.merged(dependencies); } @@ -191,7 +196,7 @@ protected boolean isExistsQuery() { @Override protected Mono> createQuery(RelationalParameterAccessor accessor) { - return getSpelEvaluator(accessor).map(evaluator -> new ExpandedQuery(accessor, evaluator)); + return getExpressionEvaluator(accessor).map(evaluator -> new ExpandedQuery(accessor, evaluator)); } @Override @@ -201,19 +206,13 @@ Class resolveResultType(ResultProcessor resultProcessor) { return !returnedType.isInterface() ? returnedType : super.resolveResultType(resultProcessor); } - private Mono getSpelEvaluator(RelationalParameterAccessor accessor) { - Assert.isInstanceOf(ReactiveValueEvaluationContextProvider.class, valueContextProvider, "ValueEvaluationContextProvider must be reactive"); - return ((ReactiveValueEvaluationContextProvider) valueContextProvider) - .getEvaluationContextLater(accessor.getValues(), expressionDependencies) - . map( - context -> new DefaultR2dbcSpELExpressionEvaluator(valueExpressionDelegate, context)) - .defaultIfEmpty(DefaultR2dbcSpELExpressionEvaluator.unsupported()); + private Mono getExpressionEvaluator(RelationalParameterAccessor accessor) { + return valueContextProvider.getEvaluationContextLater(accessor.getValues(), expressionDependencies); } @Override public String toString() { - String sb = getClass().getSimpleName() + " [" + expressionQuery.getQuery() + ']'; - return sb; + return getClass().getSimpleName() + " [" + expressionQuery.getQuery() + ']'; } private class ExpandedQuery implements PreparedOperation { @@ -226,10 +225,10 @@ private class ExpandedQuery implements PreparedOperation { private final Map remainderByIndex; - public ExpandedQuery(RelationalParameterAccessor accessor, R2dbcSpELExpressionEvaluator evaluator) { + public ExpandedQuery(RelationalParameterAccessor accessor, ValueEvaluationContext evaluationContext) { this.recordedBindings = new BindTargetRecorder(); - binder.bind(recordedBindings, accessor, evaluator); + binder.bind(recordedBindings, accessor, evaluationContext); remainderByName = new LinkedHashMap<>(recordedBindings.byName); remainderByIndex = new LinkedHashMap<>(recordedBindings.byIndex); diff --git a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java index 03580f746d..01a00e35f1 100644 --- a/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java +++ b/spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/repository/support/R2dbcRepositoryFactory.java @@ -52,6 +52,7 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Marcin Grzejszczak */ public class R2dbcRepositoryFactory extends ReactiveRepositoryFactorySupport { @@ -113,7 +114,7 @@ protected Object getTargetRepository(RepositoryInformation information) { } @Override - protected Optional getQueryLookupStrategy(Key key, + protected Optional getQueryLookupStrategy(@Nullable Key key, ValueExpressionDelegate valueExpressionDelegate) { return Optional.of(new R2dbcQueryLookupStrategy(operations, new CachingValueExpressionDelegate(valueExpressionDelegate), converter, dataAccessStrategy)); } diff --git a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/ExpressionQueryUnitTests.java b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/ExpressionQueryUnitTests.java index 4c080cf13f..b7093340c3 100644 --- a/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/ExpressionQueryUnitTests.java +++ b/spring-data-r2dbc/src/test/java/org/springframework/data/r2dbc/repository/query/ExpressionQueryUnitTests.java @@ -17,8 +17,11 @@ import static org.assertj.core.api.Assertions.*; +import java.util.Map; + import org.junit.jupiter.api.Test; +import org.springframework.data.expression.ValueExpression; import org.springframework.data.expression.ValueExpressionParser; /** @@ -26,11 +29,12 @@ * * @author Mark Paluch * @author Jens Schauder + * @author Marcin Grzejszczak */ class ExpressionQueryUnitTests { - @Test // gh-373 - void bindsMultipleSpelParametersCorrectly() { + @Test // gh-373, gh-1904 + void bindsMultipleExpressionParametersCorrectly() { ExpressionQuery query = ExpressionQuery .create(ValueExpressionParser.create(), "INSERT IGNORE INTO table (x, y) VALUES (:#{#point.x}, :${point.y})"); @@ -38,10 +42,10 @@ void bindsMultipleSpelParametersCorrectly() { assertThat(query.getQuery()) .isEqualTo("INSERT IGNORE INTO table (x, y) VALUES (:__synthetic_0__, :__synthetic_1__)"); - assertThat(query.getBindings()).hasSize(2); - assertThat(query.getBindings().get(0).getExpression()).isEqualTo("#{#point.x}"); - assertThat(query.getBindings().get(0).getParameterName()).isEqualTo("__synthetic_0__"); - assertThat(query.getBindings().get(1).getExpression()).isEqualTo("${point.y}"); - assertThat(query.getBindings().get(1).getParameterName()).isEqualTo("__synthetic_1__"); + Map bindings = query.getBindings(); + assertThat(bindings).hasSize(2); + + assertThat(bindings.get("__synthetic_0__").getExpressionString()).isEqualTo("#point.x"); + assertThat(bindings.get("__synthetic_1__").getExpressionString()).isEqualTo("${point.y}"); } }