Remove #sort check for sorted native queries.

We now no longer check for #sort in native queries to apply sorting directly. This was a leftover from earlier query rewriting.

Closes #3546
This commit is contained in:
Mark Paluch
2024-07-24 08:59:56 +02:00
parent 2d5146a028
commit adf182eaa4
7 changed files with 128 additions and 36 deletions

View File

@@ -117,7 +117,7 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
Sort sort = accessor.getSort();
String sortedQueryString = querySortRewriter.getSorted(query, sort);
String sortedQueryString = getSortedQueryString(sort);
ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor);
@@ -130,6 +130,10 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
return parameterBinder.get().bindAndPrepare(query, metadata, accessor);
}
String getSortedQueryString(Sort sort) {
return querySortRewriter.getSorted(query, sort);
}
@Override
protected ParameterBinder createBinder() {
return createBinder(query);

View File

@@ -25,7 +25,6 @@ import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.data.jpa.repository.NativeQuery;
import org.springframework.data.jpa.repository.QueryRewriter;
import org.springframework.data.repository.query.Parameters;
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
import org.springframework.data.repository.query.RepositoryQuery;
import org.springframework.data.repository.query.ReturnedType;
@@ -70,12 +69,6 @@ final class NativeJpaQuery extends AbstractStringBasedJpaQuery {
this.sqlResultSetMapping = annotation.isPresent() ? annotation.getString("sqlResultSetMapping") : null;
this.queryForEntity = getQueryMethod().isQueryForEntity();
Parameters<?, ?> parameters = method.getParameters();
if (parameters.hasSortParameter() && !queryString.contains("#sort")) {
throw new InvalidJpaQueryMethodException("Cannot use native queries with dynamic sorting in method " + method);
}
}
@Override

View File

@@ -15,9 +15,13 @@
*/
package org.springframework.data.jpa.repository.query;
import static org.assertj.core.api.Assertions.*;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.springframework.data.domain.Sort;
/**
* TCK Tests for {@link DefaultQueryEnhancer}.
*
@@ -34,4 +38,14 @@ public class DefaultQueryEnhancerUnitTests extends QueryEnhancerTckTests {
@Test // GH-2511, GH-2773
@Disabled("Not properly supported by QueryUtils")
void shouldDeriveNativeCountQueryWithVariable(String query, String expected) {}
@Test // GH-3546
void shouldApplySorting() {
QueryEnhancer enhancer = createQueryEnhancer(DeclaredQuery.of("SELECT e FROM Employee e", true));
String sql = enhancer.applySorting(Sort.by("foo", "bar"));
assertThat(sql).isEqualTo("SELECT e FROM Employee e order by e.foo asc, e.bar asc");
}
}

View File

@@ -41,6 +41,16 @@ public class JSqlParserQueryEnhancerUnitTests extends QueryEnhancerTckTests {
return new JSqlParserQueryEnhancer(declaredQuery);
}
@Test // GH-3546
void shouldApplySorting() {
QueryEnhancer enhancer = createQueryEnhancer(DeclaredQuery.of("SELECT e FROM Employee e", true));
String sql = enhancer.applySorting(Sort.by("foo", "bar"));
assertThat(sql).isEqualTo("SELECT e FROM Employee e ORDER BY e.foo ASC, e.bar ASC");
}
@Override
@ParameterizedTest // GH-2773
@MethodSource("jpqlCountQueries")
@@ -230,4 +240,5 @@ public class JSqlParserQueryEnhancerUnitTests extends QueryEnhancerTckTests {
"merge into a using (select id2, value from b) on (id = id2) when matched then update set a.value = value",
null));
}
}

View File

@@ -97,20 +97,6 @@ class JpaQueryLookupStrategyUnitTests {
.isThrownBy(() -> strategy.resolveQuery(method, metadata, projectionFactory, namedQueries));
}
@Test // DATAJPA-554
void sholdThrowMorePreciseExceptionIfTryingToUsePaginationInNativeQueries() throws Exception {
QueryLookupStrategy strategy = JpaQueryLookupStrategy.create(em, queryMethodFactory, Key.CREATE_IF_NOT_FOUND,
EVALUATION_CONTEXT_PROVIDER, new BeanFactoryQueryRewriterProvider(beanFactory), EscapeCharacter.DEFAULT);
Method method = UserRepository.class.getMethod("findByInvalidNativeQuery", String.class, Sort.class);
RepositoryMetadata metadata = new DefaultRepositoryMetadata(UserRepository.class);
assertThatExceptionOfType(InvalidJpaQueryMethodException.class)
.isThrownBy(() -> strategy.resolveQuery(method, metadata, projectionFactory, namedQueries))
.withMessageContaining("Cannot use native queries with dynamic sorting in method")
.withMessageContaining(method.toString());
}
@Test // GH-2217
void considersNamedCountQuery() throws Exception {
@@ -231,9 +217,6 @@ class JpaQueryLookupStrategyUnitTests {
@Query("something absurd")
User findByFoo(String foo);
@Query(value = "select u.* from User u", nativeQuery = true)
List<User> findByInvalidNativeQuery(String param, Sort sort);
@Query(countName = "foo.count")
Page<User> findByNamedQuery(String foo, Pageable pageable);

View File

@@ -0,0 +1,97 @@
/*
* 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.jpa.repository.query;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.metamodel.Metamodel;
import java.lang.reflect.Method;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.data.domain.Sort;
import org.springframework.data.jpa.provider.QueryExtractor;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.jpa.repository.QueryRewriter;
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
import org.springframework.data.repository.Repository;
import org.springframework.data.repository.core.RepositoryMetadata;
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.util.ReflectionUtils;
/**
* Unit tests for {@link NativeJpaQuery}.
*
* @author Mark Paluch
*/
@MockitoSettings(strictness = Strictness.LENIENT)
class NativeJpaQueryUnitTests {
@Mock EntityManager em;
@Mock EntityManagerFactory emf;
@Mock Metamodel metamodel;
@BeforeEach
void setUp() {
when(em.getMetamodel()).thenReturn(metamodel);
when(em.getEntityManagerFactory()).thenReturn(emf);
when(em.getDelegate()).thenReturn(em);
}
@Test // GH-3546
void shouldApplySorting() {
NativeJpaQuery query = getQuery(TestRepo.class, "find", Sort.class);
String sql = query.getSortedQueryString(Sort.by("foo", "bar"));
assertThat(sql).isEqualTo("SELECT e FROM Employee e order by e.foo asc, e.bar asc");
}
private NativeJpaQuery getQuery(Class<?> repository, String method, Class<?>... args) {
Method respositoryMethod = ReflectionUtils.findMethod(repository, method, args);
RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(repository);
SpelAwareProxyProjectionFactory projectionFactory = mock(SpelAwareProxyProjectionFactory.class);
QueryExtractor queryExtractor = mock(QueryExtractor.class);
JpaQueryMethod queryMethod = new JpaQueryMethod(respositoryMethod, repositoryMetadata, projectionFactory,
queryExtractor);
Query annotation = AnnotatedElementUtils.getMergedAnnotation(respositoryMethod, Query.class);
NativeJpaQuery query = new NativeJpaQuery(queryMethod, em, annotation.value(), annotation.countQuery(),
QueryRewriter.IdentityQueryRewriter.INSTANCE, QueryMethodEvaluationContextProvider.DEFAULT,
new SpelExpressionParser());
return query;
}
interface TestRepo extends Repository<Object, Object> {
@Query("SELECT e FROM Employee e")
Object find(Sort sort);
}
}

View File

@@ -38,10 +38,10 @@ import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;
import org.springframework.data.jpa.domain.sample.User;
import org.springframework.data.jpa.provider.QueryExtractor;
import org.springframework.data.jpa.repository.NativeQuery;
@@ -186,13 +186,6 @@ class SimpleJpaQueryUnitTests {
verify(em).createNativeQuery("SELECT u FROM User u WHERE u.lastname = ?1", User.class);
}
@Test // DATAJPA-554
void rejectsNativeQueryWithDynamicSort() throws Exception {
Method method = SampleRepository.class.getMethod("findNativeByLastname", String.class, Sort.class);
assertThatExceptionOfType(InvalidJpaQueryMethodException.class).isThrownBy(() -> createJpaQuery(method));
}
@Test // DATAJPA-352
@SuppressWarnings("unchecked")
void doesNotValidateCountQueryIfNotPagingMethod() throws Exception {
@@ -326,9 +319,6 @@ class SimpleJpaQueryUnitTests {
@NativeQuery(value = "SELECT u FROM User u WHERE u.lastname = ?1")
List<User> findByLastnameNativeAnnotation(String lastname);
@Query(value = "SELECT u FROM User u WHERE u.lastname = ?1", nativeQuery = true)
List<User> findNativeByLastname(String lastname, Sort sort);
@Query(value = "SELECT u FROM User u WHERE u.lastname = ?1", nativeQuery = true)
List<User> findNativeByLastname(String lastname, Pageable pageable);