Skip to content

Support for SpEL expressions containing table names in annotated queries #1863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-1856-tablename-spel-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data Relational Parent</name>
Expand Down
2 changes: 1 addition & 1 deletion spring-data-jdbc-distribution/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-1856-tablename-spel-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
4 changes: 2 additions & 2 deletions spring-data-jdbc/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-jdbc</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-1856-tablename-spel-SNAPSHOT</version>

<name>Spring Data JDBC</name>
<description>Spring Data module for JDBC repositories.</description>
Expand All @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-1856-tablename-spel-SNAPSHOT</version>
</parent>

<properties>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ String getDeclaredQuery() {
return StringUtils.hasText(annotatedValue) ? annotatedValue : getNamedQuery();
}

String getRequiredQuery() {
public String getRequiredQuery() {

String query = getDeclaredQuery();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.springframework.data.jdbc.core.mapping.JdbcValue;
import org.springframework.data.jdbc.support.JdbcUtil;
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
import org.springframework.data.relational.repository.query.QueryPreprocessor;
import org.springframework.data.relational.repository.query.RelationalParameterAccessor;
import org.springframework.data.relational.repository.query.RelationalParametersParameterAccessor;
import org.springframework.data.repository.query.Parameter;
Expand Down Expand Up @@ -103,11 +104,33 @@ public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOpera
* @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 2.3
* @deprecated use alternative constructor
*/
@Deprecated(since = "3.4")
public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
RowMapperFactory rowMapperFactory, JdbcConverter converter,
QueryMethodEvaluationContextProvider evaluationContextProvider) {
this(queryMethod, operations, rowMapperFactory, converter, evaluationContextProvider, QueryPreprocessor.NOOP.transform(queryMethod.getRequiredQuery()));
}

/**
* 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 evaluationContextProvider must not be {@literal null}.
* @param query
* @since 3.4
*/
public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With already the constructor being changed, I wonder what difference it makes handing in the processed query vs. a QueryPreprocessor that we're going to remove from the signature anyways.

RowMapperFactory rowMapperFactory, JdbcConverter converter,
QueryMethodEvaluationContextProvider evaluationContextProvider, String query) {

super(queryMethod, operations);

Expand All @@ -116,6 +139,7 @@ public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOpera
this.converter = converter;
this.rowMapperFactory = rowMapperFactory;


if (queryMethod.isSliceQuery()) {
throw new UnsupportedOperationException(
"Slice queries are not supported using string-based queries; Offending method: " + queryMethod);
Expand All @@ -140,9 +164,9 @@ public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOpera
.of((counter, expression) -> String.format("__$synthetic$__%d", counter + 1), String::concat)
.withEvaluationContextProvider(evaluationContextProvider);

this.query = queryMethod.getRequiredQuery();
this.spelEvaluator = queryContext.parse(query, getQueryMethod().getParameters());
this.containsSpelExpressions = !this.spelEvaluator.getQueryString().equals(queryContext);
this.query = query;
this.spelEvaluator = queryContext.parse(this.query, getQueryMethod().getParameters());
this.containsSpelExpressions = !this.spelEvaluator.getQueryString().equals(this.query);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
import org.springframework.data.relational.core.mapping.event.AfterConvertCallback;
import org.springframework.data.relational.core.mapping.event.AfterConvertEvent;
import org.springframework.data.relational.repository.support.RelationalQueryLookupStrategy;
import org.springframework.data.repository.core.NamedQueries;
import org.springframework.data.repository.core.RepositoryMetadata;
import org.springframework.data.repository.query.QueryLookupStrategy;
Expand All @@ -60,7 +61,7 @@
* @author Diego Krupitza
* @author Christopher Klein
*/
abstract class JdbcQueryLookupStrategy implements QueryLookupStrategy {
abstract class JdbcQueryLookupStrategy extends RelationalQueryLookupStrategy {

private static final Log LOG = LogFactory.getLog(JdbcQueryLookupStrategy.class);

Expand All @@ -79,8 +80,10 @@ abstract class JdbcQueryLookupStrategy implements QueryLookupStrategy {
QueryMappingConfiguration queryMappingConfiguration, NamedParameterJdbcOperations operations,
@Nullable BeanFactory beanfactory, QueryMethodEvaluationContextProvider evaluationContextProvider) {

super(context, dialect);

Assert.notNull(publisher, "ApplicationEventPublisher must not be null");
Assert.notNull(context, "RelationalMappingContextPublisher must not be null");
Assert.notNull(context, "RelationalMappingContext 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");
Expand Down Expand Up @@ -156,8 +159,10 @@ public RepositoryQuery resolveQuery(Method method, RepositoryMetadata repository
"Query method %s is annotated with both, a query and a query name; Using the declared query", method));
}

String queryString = evaluateTableExpressions(repositoryMetadata, queryMethod.getRequiredQuery());

StringBasedJdbcQuery query = new StringBasedJdbcQuery(queryMethod, getOperations(), this::createMapper,
getConverter(), evaluationContextProvider);
getConverter(), evaluationContextProvider, queryString);
query.setBeanFactory(getBeanFactory());
return query;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/*
* Copyright 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.jdbc.repository;

import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;

import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.data.annotation.Id;
import org.springframework.data.jdbc.core.convert.DataAccessStrategy;
import org.springframework.data.jdbc.core.convert.DefaultJdbcTypeFactory;
import org.springframework.data.jdbc.core.convert.DelegatingDataAccessStrategy;
import org.springframework.data.jdbc.core.convert.JdbcConverter;
import org.springframework.data.jdbc.core.convert.JdbcCustomConversions;
import org.springframework.data.jdbc.core.convert.MappingJdbcConverter;
import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
import org.springframework.data.jdbc.repository.query.Query;
import org.springframework.data.jdbc.repository.support.JdbcRepositoryFactory;
import org.springframework.data.relational.core.dialect.Dialect;
import org.springframework.data.relational.core.dialect.HsqlDbDialect;
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
import org.springframework.data.relational.core.mapping.Table;
import org.springframework.data.repository.CrudRepository;
import org.springframework.jdbc.core.RowMapper;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations;
import org.springframework.jdbc.core.namedparam.SqlParameterSource;
import org.springframework.lang.Nullable;

/**
* Extracts the SQL statement that results from declared queries of a repository and perform assertions on it.
*
* @author Jens Schauder
*/
public class DeclaredQueryRepositoryUnitTests {

private NamedParameterJdbcOperations operations = mock(NamedParameterJdbcOperations.class, RETURNS_DEEP_STUBS);

@Test // GH-1856
void plainSql() {

repository(DummyEntityRepository.class).plainQuery();

assertThat(query()).isEqualTo("select * from someTable");
}

@Test // GH-1856
void tableNameQuery() {

repository(DummyEntityRepository.class).tableNameQuery();

assertThat(query()).isEqualTo("select * from \"DUMMY_ENTITY\"");
}

@Test // GH-1856
void renamedTableNameQuery() {

repository(RenamedEntityRepository.class).tableNameQuery();

assertThat(query()).isEqualTo("select * from \"ReNamed\"");
}

@Test // GH-1856
void fullyQualifiedTableNameQuery() {

repository(RenamedEntityRepository.class).qualifiedTableNameQuery();

assertThat(query()).isEqualTo("select * from \"someSchema\".\"ReNamed\"");
}

private String query() {

ArgumentCaptor<String> queryCaptor = ArgumentCaptor.forClass(String.class);
verify(operations).queryForObject(queryCaptor.capture(), any(SqlParameterSource.class), any(RowMapper.class));
return queryCaptor.getValue();
}

private @NotNull <T extends CrudRepository> T repository(Class<T> repositoryInterface) {

Dialect dialect = HsqlDbDialect.INSTANCE;

RelationalMappingContext context = new JdbcMappingContext();

DelegatingDataAccessStrategy delegatingDataAccessStrategy = new DelegatingDataAccessStrategy();
JdbcConverter converter = new MappingJdbcConverter(context, delegatingDataAccessStrategy,
new JdbcCustomConversions(), new DefaultJdbcTypeFactory(operations.getJdbcOperations()));

DataAccessStrategy dataAccessStrategy = mock(DataAccessStrategy.class);
ApplicationEventPublisher publisher = mock(ApplicationEventPublisher.class);

JdbcRepositoryFactory factory = new JdbcRepositoryFactory(dataAccessStrategy, context, converter, dialect,
publisher, operations);

return factory.getRepository(repositoryInterface);
}

@Table
record DummyEntity(@Id Long id, String name) {
}

interface DummyEntityRepository extends CrudRepository<DummyEntity, Long> {

@Nullable
@Query("select * from someTable")
DummyEntity plainQuery();

@Nullable
@Query("select * from #{#tableName}")
DummyEntity tableNameQuery();
}

@Table(name = "ReNamed", schema = "someSchema")
record RenamedEntity(@Id Long id, String name) {
}

interface RenamedEntityRepository extends CrudRepository<RenamedEntity, Long> {

@Nullable
@Query("select * from #{#tableName}")
DummyEntity tableNameQuery();

@Nullable
@Query("select * from #{#qualifiedTableName}")
DummyEntity qualifiedTableNameQuery();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;

import org.springframework.beans.factory.BeanFactory;
import org.springframework.core.convert.converter.Converter;
import org.springframework.dao.DataAccessException;
Expand Down Expand Up @@ -330,14 +329,13 @@ void appliesConverterToIterable() {
@Test // GH-1323
void queryByListOfTuples() {

String[][] tuples = {new String[]{"Albert", "Einstein"}, new String[]{"Richard", "Feynman"}};
String[][] tuples = { new String[] { "Albert", "Einstein" }, new String[] { "Richard", "Feynman" } };

SqlParameterSource parameterSource = forMethod("findByListOfTuples", List.class) //
.withArguments(Arrays.asList(tuples))
.withArguments(Arrays.asList(tuples)) //
.extractParameterSource();

assertThat(parameterSource.getValue("tuples"))
.asInstanceOf(LIST)
assertThat(parameterSource.getValue("tuples")).asInstanceOf(LIST) //
.containsExactly(tuples);

assertThat(parameterSource.getSqlType("tuples")).isEqualTo(JdbcUtil.TYPE_UNKNOWN.getVendorTypeNumber());
Expand All @@ -348,12 +346,38 @@ void queryByListOfConvertableTuples() {

SqlParameterSource parameterSource = forMethod("findByListOfTuples", List.class) //
.withCustomConverters(DirectionToIntegerConverter.INSTANCE) //
.withArguments(Arrays.asList(new Object[]{Direction.LEFT, "Einstein"}, new Object[]{Direction.RIGHT, "Feynman"}))
.withArguments(
Arrays.asList(new Object[] { Direction.LEFT, "Einstein" }, new Object[] { Direction.RIGHT, "Feynman" }))
.extractParameterSource();

assertThat(parameterSource.getValue("tuples"))
.asInstanceOf(LIST)
.containsExactly(new Object[][]{new Object[]{-1, "Einstein"}, new Object[]{1, "Feynman"}});
assertThat(parameterSource.getValue("tuples")).asInstanceOf(LIST) //
.containsExactly(new Object[][] { new Object[] { -1, "Einstein" }, new Object[] { 1, "Feynman" } });
}

@Test // GH-619
void spelCanBeUsedInsideQueries() {

JdbcQueryMethod queryMethod = createMethod("findBySpelExpression", Object.class);

List<EvaluationContextExtension> list = new ArrayList<>();
list.add(new MyEvaluationContextProvider());
QueryMethodEvaluationContextProvider evaluationContextProviderImpl = new ExtensionAwareQueryMethodEvaluationContextProvider(
list);

StringBasedJdbcQuery sut = new StringBasedJdbcQuery(queryMethod, operations, defaultRowMapper, converter,
evaluationContextProviderImpl);

ArgumentCaptor<SqlParameterSource> paramSource = ArgumentCaptor.forClass(SqlParameterSource.class);
ArgumentCaptor<String> query = ArgumentCaptor.forClass(String.class);

sut.execute(new Object[] { "myValue" });

verify(this.operations).queryForObject(query.capture(), paramSource.capture(), any(RowMapper.class));

assertThat(query.getValue())
.isEqualTo("SELECT * FROM table WHERE c = :__$synthetic$__1 AND c2 = :__$synthetic$__2");
assertThat(paramSource.getValue().getValue("__$synthetic$__1")).isEqualTo("test-value1");
assertThat(paramSource.getValue().getValue("__$synthetic$__2")).isEqualTo("test-value2");
}

QueryFixture forMethod(String name, Class... paramTypes) {
Expand Down Expand Up @@ -486,32 +510,6 @@ interface MyRepository extends Repository<Object, Long> {
Object findByListOfTuples(@Param("tuples") List<Object[]> tuples);
}

@Test // GH-619
public void spelCanBeUsedInsideQueries() {

JdbcQueryMethod queryMethod = createMethod("findBySpelExpression", Object.class);

List<EvaluationContextExtension> list = new ArrayList<>();
list.add(new MyEvaluationContextProvider());
QueryMethodEvaluationContextProvider evaluationContextProviderImpl = new ExtensionAwareQueryMethodEvaluationContextProvider(
list);

StringBasedJdbcQuery sut = new StringBasedJdbcQuery(queryMethod, operations, defaultRowMapper, converter,
evaluationContextProviderImpl);

ArgumentCaptor<SqlParameterSource> paramSource = ArgumentCaptor.forClass(SqlParameterSource.class);
ArgumentCaptor<String> query = ArgumentCaptor.forClass(String.class);

sut.execute(new Object[] { "myValue" });

verify(this.operations).queryForObject(query.capture(), paramSource.capture(), any(RowMapper.class));

assertThat(query.getValue())
.isEqualTo("SELECT * FROM table WHERE c = :__$synthetic$__1 AND c2 = :__$synthetic$__2");
assertThat(paramSource.getValue().getValue("__$synthetic$__1")).isEqualTo("test-value1");
assertThat(paramSource.getValue().getValue("__$synthetic$__2")).isEqualTo("test-value2");
}

private static class CustomRowMapper implements RowMapper<Object> {

@Override
Expand Down
Loading
Loading