Create ParameterBindings for count query instead of reusing query bindings.

This commit makes sure we create individual parameter bindings for the count query instead of reusing the bindings from the actual query. This ensures we do not miss bindings that are present only in the count query.

Closes: #3293
Original pull request: #3339

Author: christophstrobl

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java

@@ -17,7 +17,6 @@
 
 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.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<ParameterBinder> countParameterBinder;
 
 	/**
 	 * Creates a new {@link AbstractStringBasedJpaQuery} from the given {@link JpaQueryMethod}, {@link EntityManager} and
@@ -78,8 +79,17 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri
 				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 @@ protected ParameterBinder createBinder() {
 				evaluationContextProvider);
 	}
 
+	protected ParameterBinder createCountBinder(DeclaredQuery countQuery) {
+		return ParameterBinderFactory.createQueryAwareBinder(getQueryMethod().getParameters(), countQuery, parser, evaluationContextProvider);
+	}
+
 	@Override
 	protected Query doCreateCountQuery(JpaParametersParameterAccessor accessor) {
 
@@ -125,7 +139,7 @@ protected Query doCreateCountQuery(JpaParametersParameterAccessor accessor) {
 
 		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;
 	}

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java

@@ -111,9 +111,19 @@ public List<ParameterBinding> getParameterBindings() {
 	@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

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java

@@ -42,6 +42,7 @@
 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;
@@ -1747,6 +1748,25 @@ void shouldFindUserByFirstnameAndLastnameWithSpelExpressionInStringBasedQuery()
 		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<User> users = repository.findByWithSpelParameterOnlyUsedForCountQuery("Data", PageRequest.of(0, 2));
+		assertThat(users.getSize()).isEqualTo(2);
+		assertThat(users.getTotalElements()).isEqualTo(10);
+	}
+
 	@Test // DATAJPA-564
 	void shouldFindUserByLastnameWithSpelExpressionInStringBasedQuery() {
 

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java

@@ -28,10 +28,12 @@
 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.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 @@
  * @author Greg Turnquist
  * @author Krzysztof Krason
  * @author Erik Pellizzon
+ * @author Christoph Strobl
  */
 @ExtendWith(MockitoExtension.class)
 @MockitoSettings(strictness = Strictness.LENIENT)
@@ -215,6 +219,23 @@ void createsNativeCountQuery() throws Exception {
 		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<String> 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 {
 
@@ -258,12 +279,21 @@ void resolvesExpressionInCountQuery() throws Exception {
 	}
 
 	private AbstractJpaQuery createJpaQuery(Method method) {
+		return createJpaQuery(method, null);
+	}
 
-		JpaQueryMethod queryMethod = new JpaQueryMethod(method, metadata, factory, extractor);
-		return JpaQueryFactory.INSTANCE.fromMethodWithQueryString(queryMethod, em, queryMethod.getAnnotatedQuery(), 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<String> countQueryString) {
+
+		JpaQueryMethod queryMethod = new JpaQueryMethod(method, metadata, factory, extractor);
+		return createJpaQuery(queryMethod, queryMethod.getAnnotatedQuery(), countQueryString == null ? null : countQueryString.orElse(queryMethod.getCountQuery()));
+	}
+
 	interface SampleRepository {
 
 		@Query(value = "SELECT u FROM User u WHERE u.lastname = ?1", nativeQuery = true)
@@ -293,6 +323,10 @@ interface SampleRepository {
 		@Query(value = "select u from #{#entityName} u", countQuery = "select count(u.id) from #{#entityName} u")
 		List<User> findAllWithExpressionInCountQuery(Pageable pageable);
 
+
+		@Query(value = "select u from User u", countQuery = "select count(u.id) from #{#entityName} u where u.name = :#{#arg0}")
+		List<User> findAllWithBindingsOnlyInCountQuery(String arg0, Pageable pageable);
+
 	}
 
 	interface UserProjection {}

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java

@@ -446,6 +446,9 @@ Window<User> findTop3ByFirstnameStartingWithOrderByFirstnameAscEmailAddressAsc(S
 	@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<User> 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<User> findByWithSpelParameterOnlyUsedForCountQuery(String lastname, Pageable page);
+
 	// DATAJPA-564
 	@Query("select u from User u where u.lastname like %:#{[0]}% and u.lastname like %:lastname%")
 	List<User> findByLastnameWithSpelExpression(@Param("lastname") String lastname);