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
This commit is contained in:
Christoph Strobl
2024-01-22 15:15:09 +01:00
committed by Mark Paluch
parent 8a0b7e29d8
commit 7d089292f5
5 changed files with 89 additions and 8 deletions

View File

@@ -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<ParameterBinder> 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;
}

View File

@@ -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

View File

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

View File

@@ -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<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 {
@@ -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<String> 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<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 {}

View File

@@ -449,6 +449,9 @@ public interface UserRepository extends JpaRepository<User, Integer>, 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<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);