Skip to content

Commit

Permalink
DATAJPA-928 - Enabled native query with Pageable.
Browse files Browse the repository at this point in the history
Just removed the check that was actively preventing the use of Pageable. Migrated to tests to AssertJ where applicable.

Original pull request: #246.
  • Loading branch information
schauder authored and odrotbohm committed Feb 19, 2018
1 parent 5dacc87 commit 8bf8e3a
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
*
* @author Thomas Darimont
* @author Oliver Gierke
* @author Jens Schauder
*/
final class NativeJpaQuery extends AbstractStringBasedJpaQuery {

Expand All @@ -53,13 +54,10 @@ public NativeJpaQuery(JpaQueryMethod method, EntityManager em, String queryStrin
super(method, em, queryString, evaluationContextProvider, parser);

Parameters<?, ?> parameters = method.getParameters();
boolean hasPagingOrSortingParameter = parameters.hasPageableParameter() || parameters.hasSortParameter();
boolean containsPageableOrSortInQueryExpression = queryString.contains("#pageable")
|| queryString.contains("#sort");

if (hasPagingOrSortingParameter && !containsPageableOrSortInQueryExpression) {
if (parameters.hasSortParameter() && !queryString.contains("#sort")) {
throw new InvalidJpaQueryMethodException(
"Cannot use native queries with dynamic sorting and/or pagination in method " + method);
"Cannot use native queries with dynamic sorting in method " + method);
}

this.resultType = getTypeToQueryFor();
Expand Down
22 changes: 18 additions & 4 deletions src/test/java/org/springframework/data/jpa/domain/sample/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import javax.persistence.CascadeType;
import javax.persistence.Column;
import javax.persistence.ColumnResult;
import javax.persistence.ElementCollection;
import javax.persistence.Embedded;
import javax.persistence.Entity;
Expand All @@ -34,11 +35,15 @@
import javax.persistence.NamedAttributeNode;
import javax.persistence.NamedEntityGraph;
import javax.persistence.NamedEntityGraphs;
import javax.persistence.NamedNativeQueries;
import javax.persistence.NamedNativeQuery;
import javax.persistence.NamedQuery;
import javax.persistence.NamedStoredProcedureQueries;
import javax.persistence.NamedStoredProcedureQuery;
import javax.persistence.NamedSubgraph;
import javax.persistence.ParameterMode;
import javax.persistence.SqlResultSetMapping;
import javax.persistence.SqlResultSetMappings;
import javax.persistence.StoredProcedureParameter;
import javax.persistence.Table;
import javax.persistence.Temporal;
Expand All @@ -51,6 +56,7 @@
* @author Oliver Gierke
* @author Thomas Darimont
* @author Christoph Strobl
* @author Jens Schauder
*/
@Entity
@NamedEntityGraphs({ @NamedEntityGraph(name = "User.overview", attributeNodes = { @NamedAttributeNode("roles") }),
Expand All @@ -61,11 +67,10 @@
attributeNodes = { @NamedAttributeNode("roles"), @NamedAttributeNode("manager"),
@NamedAttributeNode("colleagues") }),
@NamedEntityGraph(name = "User.withSubGraph",
attributeNodes = { @NamedAttributeNode("roles"),
@NamedAttributeNode(value = "colleagues", subgraph = "User.colleagues") },
attributeNodes = { @NamedAttributeNode("roles"), @NamedAttributeNode(value = "colleagues",
subgraph = "User.colleagues") },
subgraphs = { @NamedSubgraph(name = "User.colleagues",
attributeNodes = { @NamedAttributeNode("colleagues"),
@NamedAttributeNode("roles") }) }),
attributeNodes = { @NamedAttributeNode("colleagues"), @NamedAttributeNode("roles") }) }),
@NamedEntityGraph(name = "User.deepGraph",
attributeNodes = { @NamedAttributeNode("roles"),
@NamedAttributeNode(value = "colleagues", subgraph = "User.colleagues") },
Expand All @@ -84,6 +89,15 @@
@NamedStoredProcedureQuery(name = "User.plus1IO", procedureName = "plus1inout",
parameters = { @StoredProcedureParameter(mode = ParameterMode.IN, name = "arg", type = Integer.class),
@StoredProcedureParameter(mode = ParameterMode.OUT, name = "res", type = Integer.class) })

// Annotations for native Query with pageable
@SqlResultSetMappings({
@SqlResultSetMapping(name = "SqlResultSetMapping.count", columns = @ColumnResult(name = "cnt")) })
@NamedNativeQueries({
@NamedNativeQuery(name = "User.findByNativeNamedQueryWithPageable", resultClass = User.class,
query = "SELECT * FROM SD_USER ORDER BY UCASE(firstname)"),
@NamedNativeQuery(name = "User.findByNativeNamedQueryWithPageable.count",
resultSetMapping = "SqlResultSetMapping.count", query = "SELECT count(*) AS cnt FROM SD_USER") })
@Table(name = "SD_User")
public class User {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import javax.persistence.criteria.Predicate;
import javax.persistence.criteria.Root;

import org.assertj.core.api.SoftAssertions;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
Expand Down Expand Up @@ -2109,6 +2110,56 @@ public void handlesCountQueriesWithLessParametersMoreThanOneIndexed() {
repository.findAllOrderedBySpecialNameMultipleParamsIndexed("Oliver", "x", PageRequest.of(2, 3));
}

// DATAJPA-928
@Test
public void executeNativeQueryWithPage() {

flushTestUsers();

Page<User> firstPage = repository.findByNativeNamedQueryWithPageable(new PageRequest(0, 3));
Page<User> secondPage = repository.findByNativeNamedQueryWithPageable(new PageRequest(1, 3));

SoftAssertions softly = new SoftAssertions();

assertThat(firstPage.getTotalElements()).isEqualTo(4L);
assertThat(firstPage.getNumberOfElements()).isEqualTo(3);
assertThat(firstPage.getContent()) //
.extracting(User::getFirstname) //
.containsExactly("Dave", "Joachim", "kevin");

assertThat(secondPage.getTotalElements()).isEqualTo(4L);
assertThat(secondPage.getNumberOfElements()).isEqualTo(1);
assertThat(secondPage.getContent()) //
.extracting(User::getFirstname) //
.containsExactly("Oliver");

softly.assertAll();
}

// DATAJPA-928
@Test
public void executeNativeQueryWithPageWorkaround() {

flushTestUsers();

Page<String> firstPage = repository.findByNativeQueryWithPageable(new PageRequest(0, 3));
Page<String> secondPage = repository.findByNativeQueryWithPageable(new PageRequest(1, 3));

SoftAssertions softly = new SoftAssertions();

assertThat(firstPage.getTotalElements()).isEqualTo(4L);
assertThat(firstPage.getNumberOfElements()).isEqualTo(3);
assertThat(firstPage.getContent()) //
.containsExactly("Dave", "Joachim", "kevin");

assertThat(secondPage.getTotalElements()).isEqualTo(4L);
assertThat(secondPage.getNumberOfElements()).isEqualTo(1);
assertThat(secondPage.getContent()) //
.containsExactly("Oliver");

softly.assertAll();
}

private Page<User> executeSpecWithSort(Sort sort) {

flushTestUsers();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,23 @@
*/
package org.springframework.data.jpa.repository.query;

import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;

import java.lang.reflect.Method;
import java.util.List;

import javax.persistence.EntityManager;
import javax.persistence.EntityManagerFactory;
import javax.persistence.metamodel.Metamodel;

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.junit.MockitoJUnitRunner;
import org.springframework.data.domain.Page;
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.Query;
Expand All @@ -53,6 +50,7 @@
*
* @author Oliver Gierke
* @author Thomas Darimont
* @author Jens Schauder
*/
@RunWith(MockitoJUnitRunner.class)
public class JpaQueryLookupStrategyUnitTests {
Expand All @@ -65,8 +63,6 @@ public class JpaQueryLookupStrategyUnitTests {
@Mock Metamodel metamodel;
@Mock ProjectionFactory projectionFactory;

public @Rule ExpectedException exception = ExpectedException.none();

@Before
public void setUp() {

Expand All @@ -87,27 +83,23 @@ public void invalidAnnotatedQueryCausesException() throws Exception {
Throwable reference = new RuntimeException();
when(em.createQuery(anyString())).thenThrow(reference);

try {
strategy.resolveQuery(method, metadata, projectionFactory, namedQueries);
} catch (Exception e) {
assertThat(e, is(instanceOf(IllegalArgumentException.class)));
assertThat(e.getCause(), is(reference));
}
assertThatExceptionOfType(IllegalArgumentException.class)
.isThrownBy(() -> strategy.resolveQuery(method, metadata, projectionFactory, namedQueries))
.withCause(reference);
}

@Test // DATAJPA-554
public void sholdThrowMorePreciseExceptionIfTryingToUsePaginationInNativeQueries() throws Exception {

QueryLookupStrategy strategy = JpaQueryLookupStrategy.create(em, Key.CREATE_IF_NOT_FOUND, extractor,
EVALUATION_CONTEXT_PROVIDER);
Method method = UserRepository.class.getMethod("findByInvalidNativeQuery", String.class, Pageable.class);
Method method = UserRepository.class.getMethod("findByInvalidNativeQuery", String.class, Sort.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, projectionFactory, namedQueries);
assertThatExceptionOfType(InvalidJpaQueryMethodException.class)
.isThrownBy(() -> strategy.resolveQuery(method, metadata, projectionFactory, namedQueries))
.withMessageContaining("Cannot use native queries with dynamic sorting in method")
.withMessageContaining(method.toString());
}

interface UserRepository extends Repository<User, Long> {
Expand All @@ -116,6 +108,6 @@ interface UserRepository extends Repository<User, Long> {
User findByFoo(String foo);

@Query(value = "select u.* from User u", nativeQuery = true)
Page<User> findByInvalidNativeQuery(String param, Pageable page);
List<User> findByInvalidNativeQuery(String param, Sort sort);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
*
* @author Oliver Gierke
* @author Thomas Darimont
* @author Jens Schauder
*/
@RunWith(MockitoJUnitRunner.Silent.class)
public class SimpleJpaQueryUnitTests {
Expand Down Expand Up @@ -153,13 +154,6 @@ public void rejectsNativeQueryWithDynamicSort() throws Exception {
createJpaQuery(method);
}

@Test(expected = InvalidJpaQueryMethodException.class) // DATAJPA-554
public void rejectsNativeQueryWithPageable() throws Exception {

Method method = SampleRepository.class.getMethod("findNativeByLastname", String.class, Pageable.class);
createJpaQuery(method);
}

@Test // DATAJPA-352
@SuppressWarnings("unchecked")
public void doesNotValidateCountQueryIfNotPagingMethod() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,13 @@ List<User> findUsersByFirstnameForSpELExpressionWithParameterIndexOnlyWithEntity
@Query(value = "SELECT u FROM User u WHERE ?2 = 'x' ORDER BY CASE WHEN (u.firstname >= ?1) THEN 0 ELSE 1 END, u.firstname")
Page<User> findAllOrderedBySpecialNameMultipleParamsIndexed(String name, String other, Pageable page);

// DATAJPA-928
Page<User> findByNativeNamedQueryWithPageable(Pageable pageable);

// DATAJPA-928
@Query(value = "SELECT firstname FROM SD_User ORDER BY UCASE(firstname)", countQuery = "SELECT count(*) FROM SD_User", nativeQuery = true)
Page<String> findByNativeQueryWithPageable(@Param("pageable") Pageable pageable);

interface RolesAndFirstname {

String getFirstname();
Expand Down

0 comments on commit 8bf8e3a

Please sign in to comment.