From adf182eaa4c47caae94307e9c565d7581b720fe1 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 24 Jul 2024 08:59:56 +0200 Subject: [PATCH] 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 --- .../query/AbstractStringBasedJpaQuery.java | 6 +- .../jpa/repository/query/NativeJpaQuery.java | 7 -- .../query/DefaultQueryEnhancerUnitTests.java | 14 +++ .../JSqlParserQueryEnhancerUnitTests.java | 11 +++ .../JpaQueryLookupStrategyUnitTests.java | 17 ---- .../query/NativeJpaQueryUnitTests.java | 97 +++++++++++++++++++ .../query/SimpleJpaQueryUnitTests.java | 12 +-- 7 files changed, 128 insertions(+), 36 deletions(-) create mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java 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 f219c374d..c60da0442 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 @@ -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); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java index c0ad69de7..7b8a018a8 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java @@ -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 diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancerUnitTests.java index f1d7e707d..da2bb066a 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancerUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancerUnitTests.java @@ -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"); + } } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java index d5dc2b859..a41b54193 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java @@ -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)); } + } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java index fe85ba8ad..7a27c1bfc 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java @@ -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 findByInvalidNativeQuery(String param, Sort sort); - @Query(countName = "foo.count") Page findByNamedQuery(String foo, Pageable pageable); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java new file mode 100644 index 000000000..67c61b930 --- /dev/null +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/NativeJpaQueryUnitTests.java @@ -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 { + + @Query("SELECT e FROM Employee e") + Object find(Sort sort); + } + +} 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 3e77c0cd9..014dee96f 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 @@ -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 findByLastnameNativeAnnotation(String lastname); - @Query(value = "SELECT u FROM User u WHERE u.lastname = ?1", nativeQuery = true) - List findNativeByLastname(String lastname, Sort sort); - @Query(value = "SELECT u FROM User u WHERE u.lastname = ?1", nativeQuery = true) List findNativeByLastname(String lastname, Pageable pageable);