Take Spring Data Pageable's Sort into account

Fixes quarkusio#5406
geoand · Nov 12, 2019 · 09bc0f9 · 09bc0f9
1 parent 8c589a1
commit 09bc0f9
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 6 deletions.
diff --git a/...loyment/src/main/java/io/quarkus/spring/data/deployment/generate/DerivedMethodsAdder.java b/...loyment/src/main/java/io/quarkus/spring/data/deployment/generate/DerivedMethodsAdder.java
@@ -10,6 +10,8 @@
 import org.jboss.jandex.IndexView;
 import org.jboss.jandex.MethodInfo;
 import org.jboss.jandex.Type;
+import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
 
 import io.quarkus.gizmo.ClassCreator;
 import io.quarkus.gizmo.FieldDescriptor;
@@ -106,6 +108,16 @@ public void add(ClassCreator classCreator, FieldDescriptor entityClassFieldDescr
                                 methodCreator.getMethodParam(sortParameterIndex));
                     } else if (parseResult.getSort() != null) {
                         finalQuery += JpaOperations.toOrderBy(parseResult.getSort());
+                    } else if (pageableParameterIndex != null) {
+                        ResultHandle pageable = methodCreator.getMethodParam(pageableParameterIndex);
+                        ResultHandle pageableSort = methodCreator.invokeInterfaceMethod(
+                                MethodDescriptor.ofMethod(Pageable.class, "getSort", Sort.class),
+                                pageable);
+                        sort = methodCreator.invokeStaticMethod(
+                                MethodDescriptor.ofMethod(TypesConverter.class, "toPanacheSort",
+                                        io.quarkus.panache.common.Sort.class,
+                                        org.springframework.data.domain.Sort.class),
+                                pageableSort);
                     }
 
                     // call JpaOperations.find()

diff --git a/...eployment/src/main/java/io/quarkus/spring/data/deployment/generate/StockMethodsAdder.java b/...eployment/src/main/java/io/quarkus/spring/data/deployment/generate/StockMethodsAdder.java
@@ -33,6 +33,7 @@
 import org.springframework.data.domain.Sort;
 
 import io.quarkus.deployment.bean.JavaBeanUtil;
+import io.quarkus.gizmo.AssignableResultHandle;
 import io.quarkus.gizmo.BranchResult;
 import io.quarkus.gizmo.BytecodeCreator;
 import io.quarkus.gizmo.ClassCreator;
@@ -544,17 +545,44 @@ private void generateFindAllWithPageable(ClassCreator classCreator, FieldDescrip
                             "(Lorg/springframework/data/domain/Pageable;)Lorg/springframework/data/domain/Page<L%s;>;",
                             entityTypeStr.replace('.', '/')));
 
-                    ResultHandle page = findAll.invokeStaticMethod(
+                    ResultHandle pageable = findAll.getMethodParam(0);
+                    ResultHandle pageableSort = findAll.invokeInterfaceMethod(
+                            MethodDescriptor.ofMethod(Pageable.class, "getSort", Sort.class),
+                            pageable);
+
+                    ResultHandle panachePage = findAll.invokeStaticMethod(
                             MethodDescriptor.ofMethod(TypesConverter.class, "toPanachePage",
                                     io.quarkus.panache.common.Page.class, Pageable.class),
-                            findAll.getMethodParam(0));
-                    ResultHandle panacheQuery = findAll.invokeStaticMethod(
+                            pageable);
+                    ResultHandle panacheSort = findAll.invokeStaticMethod(
+                            MethodDescriptor.ofMethod(TypesConverter.class, "toPanacheSort",
+                                    io.quarkus.panache.common.Sort.class,
+                                    org.springframework.data.domain.Sort.class),
+                            pageableSort);
+
+                    // depending on whether there was a io.quarkus.panache.common.Sort returned, we need to execute a different findAll method
+                    BranchResult sortNullBranch = findAll.ifNull(panacheSort);
+                    BytecodeCreator sortNullTrue = sortNullBranch.trueBranch();
+                    BytecodeCreator sortNullFalse = sortNullBranch.falseBranch();
+                    AssignableResultHandle panacheQueryVar = findAll.createVariable(PanacheQuery.class);
+
+                    ResultHandle panacheQueryWithoutSort = sortNullTrue.invokeStaticMethod(
                             ofMethod(JpaOperations.class, "findAll", PanacheQuery.class, Class.class),
-                            findAll.readInstanceField(entityClassFieldDescriptor, findAll.getThis()));
-                    panacheQuery = findAll.invokeInterfaceMethod(
+                            sortNullTrue.readInstanceField(entityClassFieldDescriptor, sortNullTrue.getThis()));
+                    sortNullTrue.assign(panacheQueryVar, panacheQueryWithoutSort);
+                    sortNullTrue.breakScope();
+
+                    ResultHandle panacheQueryWithSort = sortNullFalse.invokeStaticMethod(
+                            ofMethod(JpaOperations.class, "findAll", PanacheQuery.class, Class.class,
+                                    io.quarkus.panache.common.Sort.class),
+                            sortNullFalse.readInstanceField(entityClassFieldDescriptor, sortNullFalse.getThis()), panacheSort);
+                    sortNullFalse.assign(panacheQueryVar, panacheQueryWithSort);
+                    sortNullFalse.breakScope();
+
+                    ResultHandle panacheQuery = findAll.invokeInterfaceMethod(
                             MethodDescriptor.ofMethod(PanacheQuery.class, "page", PanacheQuery.class,
                                     io.quarkus.panache.common.Page.class),
-                            panacheQuery, page);
+                            panacheQueryVar, panachePage);
                     ResultHandle list = findAll.invokeInterfaceMethod(
                             MethodDescriptor.ofMethod(PanacheQuery.class, "list", List.class),
                             panacheQuery);

diff --git a/...on-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/CountryResource.java b/...on-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/CountryResource.java
@@ -2,6 +2,7 @@
 
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 import javax.persistence.NoResultException;
 import javax.ws.rs.GET;
@@ -36,6 +37,14 @@ public String page(@PathParam("size") int pageSize, @PathParam("num") int pageNu
         return page.hasPrevious() + " - " + page.hasNext() + " / " + page.getNumberOfElements();
     }
 
+    @GET
+    @Path("/page-sorted/{size}/{num}")
+    @Produces("text/plain")
+    public String pageSorted(@PathParam("size") int pageSize, @PathParam("num") int pageNum) {
+        Page<Country> page = countryRepository.findAll(PageRequest.of(pageNum, pageSize, Sort.by(Sort.Direction.DESC, "id")));
+        return page.stream().map(Country::getId).map(Object::toString).collect(Collectors.joining(","));
+    }
+
     @GET
     @Path("/new/{name}/{iso3}")
     @Produces("application/json")

diff --git a/...n-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PersonRepository.java b/...n-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PersonRepository.java
@@ -17,6 +17,8 @@ public interface PersonRepository extends CrudRepository<Person, Long>, PersonFr
 
     List<Person> findByName(String name);
 
+    List<Person> findByName(String name, Pageable pageable);
+
     List<Person> findByName(String name, Sort sort);
 
     Page<Person> findByNameOrderByJoined(String name, Pageable pageable);

diff --git a/...ion-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PersonResource.java b/...ion-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PersonResource.java
@@ -122,6 +122,14 @@ public List<Person> byName(@PathParam("name") String name) {
         return personRepository.findByName(name);
     }
 
+    @GET
+    @Path("/name-pageable/{name}")
+    @Produces("text/plain")
+    public String byNamePageable(@PathParam("name") String name) {
+        return personRepository.findByName(name, PageRequest.of(0, 2, Sort.by(new Sort.Order(Sort.Direction.DESC, "id"))))
+                .stream().map(Person::getId).map(Object::toString).collect(Collectors.joining(","));
+    }
+
     @GET
     @Path("/name/joinedOrder/{name}/page/{size}/{num}")
     public String byNamePage(@PathParam("name") String name, @PathParam("size") int pageSize, @PathParam("num") int pageNum) {

diff --git a/...ests/spring-data-jpa/src/test/java/io/quarkus/it/spring/data/jpa/CountryResourceTest.java b/...ests/spring-data-jpa/src/test/java/io/quarkus/it/spring/data/jpa/CountryResourceTest.java
@@ -9,9 +9,11 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.junit.jupiter.api.Test;
 
@@ -53,6 +55,15 @@ void testPage() {
                 .body(is("true - false / 0"));
     }
 
+    @Test
+    void testPageSorted() {
+        String response = when().get("/country/page-sorted/2/0").then()
+                .statusCode(200)
+                .extract().response().asString();
+        assertThat(Arrays.stream(response.split(",")).map(Long::parseLong).collect(Collectors.toList()))
+                .isSortedAccordingTo(Comparator.reverseOrder());
+    }
+
     @Test
     void testGetOne() {
         when().get("/country/getOne/1").then()

diff --git a/...tests/spring-data-jpa/src/test/java/io/quarkus/it/spring/data/jpa/PersonResourceTest.java b/...tests/spring-data-jpa/src/test/java/io/quarkus/it/spring/data/jpa/PersonResourceTest.java
@@ -7,6 +7,7 @@
 import static org.hamcrest.Matchers.is;
 
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -73,6 +74,15 @@ void testFindByName() {
                 .body("size()", is(3));
     }
 
+    @Test
+    void testFindByNamePageSorted() {
+        String response = when().get("/person/name-pageable/DeMar").then()
+                .statusCode(200)
+                .extract().response().asString();
+        assertThat(Arrays.stream(response.split(",")).map(Long::parseLong).collect(Collectors.toList()))
+                .isSortedAccordingTo(Comparator.reverseOrder());
+    }
+
     @Test
     void testFindBySortedByJoinedDesc() {
         List<Person> people = when().get("/person/name/DeMar/order/joined").then()