diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java index 4502ac526..135faddd8 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java @@ -17,7 +17,6 @@ package org.springframework.data.jpa.repository.query; import jakarta.persistence.EntityManager; import jakarta.persistence.Query; - import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.repository.QueryRewriter; @@ -28,6 +27,7 @@ import org.springframework.data.util.Lazy; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * Base class for {@link String} based JPA queries. @@ -49,6 +49,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { private final SpelExpressionParser parser; private final QueryParameterSetter.QueryMetadataCache metadataCache = new QueryParameterSetter.QueryMetadataCache(); private final QueryRewriter queryRewriter; + private final Lazy countParameterBinder; /** * Creates a new {@link AbstractStringBasedJpaQuery} from the given {@link JpaQueryMethod}, {@link EntityManager} and @@ -78,8 +79,17 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { method.isNativeQuery()); this.countQuery = Lazy.of(() -> { - DeclaredQuery countQuery = query.deriveCountQuery(countQueryString, method.getCountQueryProjection()); - return ExpressionBasedStringQuery.from(countQuery, method.getEntityInformation(), parser, method.isNativeQuery()); + + if(StringUtils.hasText(countQueryString)) { + + return new ExpressionBasedStringQuery(countQueryString, method.getEntityInformation(), parser, + method.isNativeQuery()); + } + return query.deriveCountQuery(null, method.getCountQueryProjection()); + }); + + this.countParameterBinder = Lazy.of(() -> { + return this.createCountBinder(this.countQuery.get()); }); this.parser = parser; @@ -113,6 +123,10 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { evaluationContextProvider); } + protected ParameterBinder createCountBinder(DeclaredQuery countQuery) { + return ParameterBinderFactory.createQueryAwareBinder(getQueryMethod().getParameters(), countQuery, parser, evaluationContextProvider); + } + @Override protected Query doCreateCountQuery(JpaParametersParameterAccessor accessor) { @@ -125,7 +139,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery { QueryParameterSetter.QueryMetadata metadata = metadataCache.getMetadata(queryString, query); - parameterBinder.get().bind(metadata.withQuery(query), accessor, QueryParameterSetter.ErrorHandling.LENIENT); + countParameterBinder.get().bind(metadata.withQuery(query), accessor, QueryParameterSetter.ErrorHandling.LENIENT); return query; } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java index d1a3696c7..bb7fb3c2a 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java @@ -111,9 +111,19 @@ class StringQuery implements DeclaredQuery { @Override public DeclaredQuery deriveCountQuery(@Nullable String countQuery, @Nullable String countQueryProjection) { - return DeclaredQuery.of( // - countQuery != null ? countQuery : this.queryEnhancer.createCountQueryFor(countQueryProjection), // + if(StringUtils.hasText(countQuery)) { + return new StringQuery(countQuery, this.isNative); + } + + StringQuery stringQuery = new StringQuery(this.queryEnhancer.createCountQueryFor(countQueryProjection), // this.isNative); + + if(this.hasParameterBindings() && !this.getParameterBindings().equals(stringQuery.getParameterBindings())) { + stringQuery.getParameterBindings().clear(); + stringQuery.getParameterBindings().addAll(this.bindings); + } + + return stringQuery; } @Override diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java index 54496092b..4b802f977 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java @@ -41,6 +41,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.IntStream; import java.util.stream.Stream; import org.assertj.core.api.SoftAssertions; @@ -1743,6 +1744,25 @@ class UserRepositoryTests { assertThat(users).containsOnly(firstUser); } + @Test // GH-2393 + void bindsSpELParameterOnlyUsedInCountQuery() { + + flushTestUsers(); // add some noise + + IntStream.range(0, 10).mapToObj(counter -> { + User source = new User(); + source.setFirstname("%d-Spring".formatted(counter)); + source.setLastname("Data"); + source.setEmailAddress("spring-%s@data.org".formatted(counter)); + return source; + }).forEach(repository::save); + em.flush(); + + Page users = repository.findByWithSpelParameterOnlyUsedForCountQuery("Data", PageRequest.of(0, 2)); + assertThat(users.getSize()).isEqualTo(2); + assertThat(users.getTotalElements()).isEqualTo(10); + } + @Test // DATAJPA-564 void shouldFindUserByLastnameWithSpelExpressionInStringBasedQuery() { diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java index 16d5b3358..6cc283540 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java @@ -28,10 +28,12 @@ import jakarta.persistence.metamodel.Metamodel; import java.lang.reflect.Method; import java.util.Collection; import java.util.List; +import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @@ -53,6 +55,7 @@ import org.springframework.data.repository.query.QueryMethodEvaluationContextPro import org.springframework.data.repository.query.RepositoryQuery; import org.springframework.data.util.TypeInformation; import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.lang.Nullable; /** * Unit test for {@link SimpleJpaQuery}. @@ -65,6 +68,7 @@ import org.springframework.expression.spel.standard.SpelExpressionParser; * @author Greg Turnquist * @author Krzysztof Krason * @author Erik Pellizzon + * @author Christoph Strobl */ @ExtendWith(MockitoExtension.class) @MockitoSettings(strictness = Strictness.LENIENT) @@ -217,6 +221,23 @@ class SimpleJpaQueryUnitTests { verify(em).createNativeQuery(anyString()); } + @Test // GH-3293 + void allowsCountQueryUsingParametersNotInOriginalQuery() throws Exception { + + when(em.createNativeQuery(anyString())).thenReturn(query); + + AbstractJpaQuery jpaQuery = createJpaQuery( + SampleRepository.class.getMethod("findAllWithBindingsOnlyInCountQuery", String.class, Pageable.class), Optional.empty()); + + jpaQuery.doCreateCountQuery(new JpaParametersParameterAccessor(jpaQuery.getQueryMethod().getParameters(), + new Object[]{"data", PageRequest.of(0, 10)})); + + ArgumentCaptor queryStringCaptor = ArgumentCaptor.forClass(String.class); + verify(em).createQuery(queryStringCaptor.capture(), eq(Long.class)); + + assertThat(queryStringCaptor.getValue()).startsWith("select count(u.id) from User u where u.name ="); + } + @Test // DATAJPA-885 void projectsWithManuallyDeclaredQuery() throws Exception { @@ -260,10 +281,19 @@ class SimpleJpaQueryUnitTests { } private AbstractJpaQuery createJpaQuery(Method method) { + return createJpaQuery(method, null); + } + + private AbstractJpaQuery createJpaQuery(JpaQueryMethod queryMethod, @Nullable String queryString, @Nullable String countQueryString) { + + return JpaQueryFactory.INSTANCE.fromMethodWithQueryString(queryMethod, em, queryString, countQueryString, + QueryRewriter.IdentityQueryRewriter.INSTANCE, EVALUATION_CONTEXT_PROVIDER); + } + + private AbstractJpaQuery createJpaQuery(Method method, @Nullable Optional countQueryString) { JpaQueryMethod queryMethod = new JpaQueryMethod(method, metadata, factory, extractor); - return JpaQueryFactory.INSTANCE.fromMethodWithQueryString(queryMethod, em, queryMethod.getAnnotatedQuery(), null, - QueryRewriter.IdentityQueryRewriter.INSTANCE, EVALUATION_CONTEXT_PROVIDER); + return createJpaQuery(queryMethod, queryMethod.getAnnotatedQuery(), countQueryString == null ? null : countQueryString.orElse(queryMethod.getCountQuery())); } interface SampleRepository { @@ -295,6 +325,10 @@ class SimpleJpaQueryUnitTests { @Query(value = "select u from #{#entityName} u", countQuery = "select count(u.id) from #{#entityName} u") List findAllWithExpressionInCountQuery(Pageable pageable); + + @Query(value = "select u from User u", countQuery = "select count(u.id) from #{#entityName} u where u.name = :#{#arg0}") + List findAllWithBindingsOnlyInCountQuery(String arg0, Pageable pageable); + } interface UserProjection {} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java index ab0e8221d..beaf23c97 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java @@ -449,6 +449,9 @@ public interface UserRepository extends JpaRepository, JpaSpecifi @Query("select u from User u where u.firstname = ?#{[0]} and u.firstname = ?1 and u.lastname like %?#{[1]}% and u.lastname like %?2%") List findByFirstnameAndLastnameWithSpelExpression(String firstname, String lastname); + @Query(value = "select * from SD_User", countQuery = "select count(1) from SD_User u where u.lastname = :#{#lastname}", nativeQuery = true) + Page findByWithSpelParameterOnlyUsedForCountQuery(String lastname, Pageable page); + // DATAJPA-564 @Query("select u from User u where u.lastname like %:#{[0]}% and u.lastname like %:lastname%") List findByLastnameWithSpelExpression(@Param("lastname") String lastname);