From 48a3eb130536a63830fcd764ba062463d0f040c1 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Tue, 17 Jun 2014 12:17:28 +0200 Subject: [PATCH] DATAJPA-554 - Avoid falling back to query derivation for invalid native queries. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now throw a dedicated InvalidJpaQueryMethodException with a more concrete exception message if we encounter a query method that is annotated with @Query(value = "…", native = true) and erroneously uses a Pageable or Sort parameter. Previously we only threw a IllegalStateException which was then recovered by the CreateIfNotFoundQueryLookupStrategy which tried to do query method derivation as a last resort. This resulted in a cryptic exception message to be thrown that indicates the derivation failure rather than the original error. Original pull request: #97. --- .../query/InvalidJpaQueryMethodException.java | 36 +++++++++++++++++++ .../jpa/repository/query/NativeJpaQuery.java | 5 +-- .../JpaQueryLookupStrategyUnitTests.java | 30 +++++++++++++++- .../query/SimpleJpaQueryUnitTests.java | 12 +++++-- 4 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 src/main/java/org/springframework/data/jpa/repository/query/InvalidJpaQueryMethodException.java diff --git a/src/main/java/org/springframework/data/jpa/repository/query/InvalidJpaQueryMethodException.java b/src/main/java/org/springframework/data/jpa/repository/query/InvalidJpaQueryMethodException.java new file mode 100644 index 000000000..47efeaba6 --- /dev/null +++ b/src/main/java/org/springframework/data/jpa/repository/query/InvalidJpaQueryMethodException.java @@ -0,0 +1,36 @@ +/* + * Copyright 2014 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 + * + * http://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; + +/** + * Signals that we encountered an invalid query method. + * + * @author Thomas Darimont + * @author Oliver Gierke + */ +public class InvalidJpaQueryMethodException extends RuntimeException { + + private static final long serialVersionUID = 1L; + + /** + * Creates a new {@link InvalidJpaQueryMethodException} with the given message. + * + * @param message must not be {@literal null} or empty. + */ + public InvalidJpaQueryMethodException(String message) { + super(message); + } +} diff --git a/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java b/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java index 8cd442f04..0a0ebdd53 100644 --- a/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java +++ b/src/main/java/org/springframework/data/jpa/repository/query/NativeJpaQuery.java @@ -1,5 +1,5 @@ /* - * Copyright 2013 the original author or authors. + * Copyright 2013-2014 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. @@ -45,7 +45,8 @@ final class NativeJpaQuery extends AbstractStringBasedJpaQuery { boolean hasPagingOrSortingParameter = parameters.hasPageableParameter() || parameters.hasSortParameter(); if (hasPagingOrSortingParameter) { - throw new IllegalStateException("Cannot use native queries with dynamic sorting and/or pagination!"); + throw new InvalidJpaQueryMethodException( + "Cannot use native queries with dynamic sorting and/or pagination in method " + method); } } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java index 312f8a3ca..8a2fdd7fc 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryLookupStrategyUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2013 the original author or authors. + * Copyright 2012-2014 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. @@ -26,10 +26,14 @@ import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.domain.sample.User; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.Repository; @@ -43,6 +47,7 @@ import org.springframework.data.repository.query.QueryLookupStrategy.Key; * Unit tests for {@link JpaQueryLookupStrategy}. * * @author Oliver Gierke + * @author Thomas Darimont */ @RunWith(MockitoJUnitRunner.class) public class JpaQueryLookupStrategyUnitTests { @@ -52,8 +57,11 @@ public class JpaQueryLookupStrategyUnitTests { @Mock QueryExtractor extractor; @Mock NamedQueries namedQueries; + public @Rule ExpectedException exception = ExpectedException.none(); + @Before public void setUp() { + when(em.getEntityManagerFactory()).thenReturn(emf); when(emf.createEntityManager()).thenReturn(em); } @@ -79,9 +87,29 @@ public class JpaQueryLookupStrategyUnitTests { } } + /** + * @see DATAJPA-554 + */ + @Test + public void sholdThrowMorePreciseExceptionIfTryingToUsePaginationInNativeQueries() throws Exception { + + QueryLookupStrategy strategy = JpaQueryLookupStrategy.create(em, Key.CREATE_IF_NOT_FOUND, extractor); + Method method = UserRepository.class.getMethod("findByInvalidNativeQuery", String.class, Pageable.class); + RepositoryMetadata metadata = new DefaultRepositoryMetadata(UserRepository.class); + + exception.expect(InvalidJpaQueryMethodException.class); + exception.expectMessage("Cannot use native queries with dynamic sorting and/or pagination in method"); + exception.expectMessage(method.toString()); + + strategy.resolveQuery(method, metadata, namedQueries); + } + interface UserRepository extends Repository { @Query("something absurd") User findByFoo(String foo); + + @Query(value = "select u.* from User u", nativeQuery = true) + Page findByInvalidNativeQuery(String param, Pageable page); } } diff --git a/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java b/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java index 3c465c56c..78251ee1b 100644 --- a/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java +++ b/src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2008-2013 the original author or authors. + * Copyright 2008-2014 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. @@ -137,14 +137,20 @@ public class SimpleJpaQueryUnitTests { verify(em).createNativeQuery("SELECT u FROM User u WHERE u.lastname = ?1", User.class); } - @Test(expected = IllegalStateException.class) + /** + * @see DATAJPA-554 + */ + @Test(expected = InvalidJpaQueryMethodException.class) public void rejectsNativeQueryWithDynamicSort() throws Exception { Method method = SampleRepository.class.getMethod("findNativeByLastname", String.class, Sort.class); createJpaQuery(method); } - @Test(expected = IllegalStateException.class) + /** + * @see DATAJPA-554 + */ + @Test(expected = InvalidJpaQueryMethodException.class) public void rejectsNativeQueryWithPageable() throws Exception { Method method = SampleRepository.class.getMethod("findNativeByLastname", String.class, Pageable.class);