Skip to content

Commit 28f44d6

Browse files
committed
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
1 parent c19d100 commit 28f44d6

File tree

7 files changed

+128
-36
lines changed

7 files changed

+128
-36
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public AbstractStringBasedJpaQuery(JpaQueryMethod method, EntityManager em, Stri
117117
public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
118118

119119
Sort sort = accessor.getSort();
120-
String sortedQueryString = querySortRewriter.getSorted(query, sort);
120+
String sortedQueryString = getSortedQueryString(sort);
121121

122122
ResultProcessor processor = getQueryMethod().getResultProcessor().withDynamicProjection(accessor);
123123

@@ -130,6 +130,10 @@ public Query doCreateQuery(JpaParametersParameterAccessor accessor) {
130130
return parameterBinder.get().bindAndPrepare(query, metadata, accessor);
131131
}
132132

133+
String getSortedQueryString(Sort sort) {
134+
return querySortRewriter.getSorted(query, sort);
135+
}
136+
133137
@Override
134138
protected ParameterBinder createBinder() {
135139
return createBinder(query);

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.springframework.data.domain.Pageable;
2323
import org.springframework.data.domain.Sort;
2424
import org.springframework.data.jpa.repository.QueryRewriter;
25-
import org.springframework.data.repository.query.Parameters;
2625
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
2726
import org.springframework.data.repository.query.RepositoryQuery;
2827
import org.springframework.data.repository.query.ReturnedType;
@@ -56,12 +55,6 @@ public NativeJpaQuery(JpaQueryMethod method, EntityManager em, String queryStrin
5655
SpelExpressionParser parser) {
5756

5857
super(method, em, queryString, countQueryString, rewriter, evaluationContextProvider, parser);
59-
60-
Parameters<?, ?> parameters = method.getParameters();
61-
62-
if (parameters.hasSortParameter() && !queryString.contains("#sort")) {
63-
throw new InvalidJpaQueryMethodException("Cannot use native queries with dynamic sorting in method " + method);
64-
}
6558
}
6659

6760
@Override

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,13 @@
1515
*/
1616
package org.springframework.data.jpa.repository.query;
1717

18+
import static org.assertj.core.api.Assertions.*;
19+
1820
import org.junit.jupiter.api.Disabled;
1921
import org.junit.jupiter.api.Test;
2022

23+
import org.springframework.data.domain.Sort;
24+
2125
/**
2226
* TCK Tests for {@link DefaultQueryEnhancer}.
2327
*
@@ -34,4 +38,14 @@ QueryEnhancer createQueryEnhancer(DeclaredQuery declaredQuery) {
3438
@Test // GH-2511, GH-2773
3539
@Disabled("Not properly supported by QueryUtils")
3640
void shouldDeriveNativeCountQueryWithVariable(String query, String expected) {}
41+
42+
@Test // GH-3546
43+
void shouldApplySorting() {
44+
45+
QueryEnhancer enhancer = createQueryEnhancer(DeclaredQuery.of("SELECT e FROM Employee e", true));
46+
47+
String sql = enhancer.applySorting(Sort.by("foo", "bar"));
48+
49+
assertThat(sql).isEqualTo("SELECT e FROM Employee e order by e.foo asc, e.bar asc");
50+
}
3751
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ QueryEnhancer createQueryEnhancer(DeclaredQuery declaredQuery) {
4141
return new JSqlParserQueryEnhancer(declaredQuery);
4242
}
4343

44+
@Test // GH-3546
45+
void shouldApplySorting() {
46+
47+
QueryEnhancer enhancer = createQueryEnhancer(DeclaredQuery.of("SELECT e FROM Employee e", true));
48+
49+
String sql = enhancer.applySorting(Sort.by("foo", "bar"));
50+
51+
assertThat(sql).isEqualTo("SELECT e FROM Employee e ORDER BY e.foo ASC, e.bar ASC");
52+
}
53+
4454
@Override
4555
@ParameterizedTest // GH-2773
4656
@MethodSource("jpqlCountQueries")
@@ -237,4 +247,5 @@ static Stream<Arguments> mergeStatementWorksSource() {
237247
"merge into a using (select id2, value from b) on (id = id2) when matched then update set a.value = value",
238248
null));
239249
}
250+
240251
}

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -102,20 +102,6 @@ void invalidAnnotatedQueryCausesException() throws Exception {
102102
.withCause(reference);
103103
}
104104

105-
@Test // DATAJPA-554
106-
void sholdThrowMorePreciseExceptionIfTryingToUsePaginationInNativeQueries() throws Exception {
107-
108-
QueryLookupStrategy strategy = JpaQueryLookupStrategy.create(em, queryMethodFactory, Key.CREATE_IF_NOT_FOUND,
109-
EVALUATION_CONTEXT_PROVIDER, new BeanFactoryQueryRewriterProvider(beanFactory), EscapeCharacter.DEFAULT);
110-
Method method = UserRepository.class.getMethod("findByInvalidNativeQuery", String.class, Sort.class);
111-
RepositoryMetadata metadata = new DefaultRepositoryMetadata(UserRepository.class);
112-
113-
assertThatExceptionOfType(InvalidJpaQueryMethodException.class)
114-
.isThrownBy(() -> strategy.resolveQuery(method, metadata, projectionFactory, namedQueries))
115-
.withMessageContaining("Cannot use native queries with dynamic sorting in method")
116-
.withMessageContaining(method.toString());
117-
}
118-
119105
@Test // GH-2217
120106
void considersNamedCountQuery() throws Exception {
121107

@@ -236,9 +222,6 @@ interface UserRepository extends Repository<User, Integer> {
236222
@Query("something absurd")
237223
User findByFoo(String foo);
238224

239-
@Query(value = "select u.* from User u", nativeQuery = true)
240-
List<User> findByInvalidNativeQuery(String param, Sort sort);
241-
242225
@Query(countName = "foo.count")
243226
Page<User> findByNamedQuery(String foo, Pageable pageable);
244227

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* Copyright 2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jpa.repository.query;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
import static org.mockito.Mockito.*;
20+
21+
import jakarta.persistence.EntityManager;
22+
import jakarta.persistence.EntityManagerFactory;
23+
import jakarta.persistence.metamodel.Metamodel;
24+
25+
import java.lang.reflect.Method;
26+
27+
import org.junit.jupiter.api.BeforeEach;
28+
import org.junit.jupiter.api.Test;
29+
import org.mockito.Mock;
30+
import org.mockito.junit.jupiter.MockitoSettings;
31+
import org.mockito.quality.Strictness;
32+
33+
import org.springframework.core.annotation.AnnotatedElementUtils;
34+
import org.springframework.data.domain.Sort;
35+
import org.springframework.data.jpa.provider.QueryExtractor;
36+
import org.springframework.data.jpa.repository.Query;
37+
import org.springframework.data.jpa.repository.QueryRewriter;
38+
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
39+
import org.springframework.data.repository.Repository;
40+
import org.springframework.data.repository.core.RepositoryMetadata;
41+
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
42+
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
43+
import org.springframework.expression.spel.standard.SpelExpressionParser;
44+
import org.springframework.util.ReflectionUtils;
45+
46+
/**
47+
* Unit tests for {@link NativeJpaQuery}.
48+
*
49+
* @author Mark Paluch
50+
*/
51+
@MockitoSettings(strictness = Strictness.LENIENT)
52+
class NativeJpaQueryUnitTests {
53+
54+
@Mock EntityManager em;
55+
@Mock EntityManagerFactory emf;
56+
@Mock Metamodel metamodel;
57+
58+
@BeforeEach
59+
void setUp() {
60+
61+
when(em.getMetamodel()).thenReturn(metamodel);
62+
when(em.getEntityManagerFactory()).thenReturn(emf);
63+
when(em.getDelegate()).thenReturn(em);
64+
}
65+
66+
@Test // GH-3546
67+
void shouldApplySorting() {
68+
69+
NativeJpaQuery query = getQuery(TestRepo.class, "find", Sort.class);
70+
String sql = query.getSortedQueryString(Sort.by("foo", "bar"));
71+
72+
assertThat(sql).isEqualTo("SELECT e FROM Employee e order by e.foo asc, e.bar asc");
73+
}
74+
75+
private NativeJpaQuery getQuery(Class<?> repository, String method, Class<?>... args) {
76+
Method respositoryMethod = ReflectionUtils.findMethod(repository, method, args);
77+
RepositoryMetadata repositoryMetadata = new DefaultRepositoryMetadata(repository);
78+
SpelAwareProxyProjectionFactory projectionFactory = mock(SpelAwareProxyProjectionFactory.class);
79+
QueryExtractor queryExtractor = mock(QueryExtractor.class);
80+
JpaQueryMethod queryMethod = new JpaQueryMethod(respositoryMethod, repositoryMetadata, projectionFactory,
81+
queryExtractor);
82+
83+
Query annotation = AnnotatedElementUtils.getMergedAnnotation(respositoryMethod, Query.class);
84+
85+
NativeJpaQuery query = new NativeJpaQuery(queryMethod, em, annotation.value(), annotation.countQuery(),
86+
QueryRewriter.IdentityQueryRewriter.INSTANCE, QueryMethodEvaluationContextProvider.DEFAULT,
87+
new SpelExpressionParser());
88+
return query;
89+
}
90+
91+
interface TestRepo extends Repository<Object, Object> {
92+
93+
@Query("SELECT e FROM Employee e")
94+
Object find(Sort sort);
95+
}
96+
97+
}

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

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@
3939
import org.mockito.junit.jupiter.MockitoExtension;
4040
import org.mockito.junit.jupiter.MockitoSettings;
4141
import org.mockito.quality.Strictness;
42+
4243
import org.springframework.data.domain.Page;
4344
import org.springframework.data.domain.PageRequest;
4445
import org.springframework.data.domain.Pageable;
45-
import org.springframework.data.domain.Sort;
4646
import org.springframework.data.jpa.domain.sample.User;
4747
import org.springframework.data.jpa.provider.QueryExtractor;
4848
import org.springframework.data.jpa.repository.Query;
@@ -164,13 +164,6 @@ void discoversNativeQuery() throws Exception {
164164
verify(em).createNativeQuery("SELECT u FROM User u WHERE u.lastname = ?1", User.class);
165165
}
166166

167-
@Test // DATAJPA-554
168-
void rejectsNativeQueryWithDynamicSort() throws Exception {
169-
170-
Method method = SampleRepository.class.getMethod("findNativeByLastname", String.class, Sort.class);
171-
assertThatExceptionOfType(InvalidJpaQueryMethodException.class).isThrownBy(() -> createJpaQuery(method));
172-
}
173-
174167
@Test // DATAJPA-352
175168
@SuppressWarnings("unchecked")
176169
void doesNotValidateCountQueryIfNotPagingMethod() throws Exception {
@@ -301,9 +294,6 @@ interface SampleRepository {
301294
@Query(value = "SELECT u FROM User u WHERE u.lastname = ?1", nativeQuery = true)
302295
List<User> findNativeByLastname(String lastname);
303296

304-
@Query(value = "SELECT u FROM User u WHERE u.lastname = ?1", nativeQuery = true)
305-
List<User> findNativeByLastname(String lastname, Sort sort);
306-
307297
@Query(value = "SELECT u FROM User u WHERE u.lastname = ?1", nativeQuery = true)
308298
List<User> findNativeByLastname(String lastname, Pageable pageable);
309299

0 commit comments

Comments
 (0)