From b2f8443ac28344b130e6072b86bffccf6f2ae653 Mon Sep 17 00:00:00 2001 From: Jose Date: Fri, 1 Jul 2022 22:46:45 +0200 Subject: [PATCH] Support Panache Sort Null Precedence Allow users sorting by either nulls first or nulls last. Example of usage: ```java Sort.by("foo", Sort.Direction.Ascending, Sort.NullPrecedence.NULLS_FIRST); ``` (See more examples in tests) This PR also adds support of nulls handling for the Spring Data JPA Quarkus extension: it translates the Spring Sort object to the new Panache Sort object. Finally, as the Panache Sort API is also used by the Mongo Quarkus extension and Mongo does not easily support sorting by nulls, I decided to simply print a WARNING message if users try to use it. Fix https://github.com/quarkusio/quarkus/issues/26172 --- .../main/asciidoc/hibernate-orm-panache.adoc | 5 +- .../asciidoc/hibernate-reactive-panache.adoc | 5 +- .../test/JpaOperationsSortTest.java | 12 +++ .../runtime/ReactiveMongoOperations.java | 3 + .../common/runtime/MongoOperations.java | 3 + .../java/io/quarkus/panache/common/Sort.java | 85 ++++++++++++++++++- .../common/runtime/PanacheJpaUtil.java | 12 ++- .../BasicTypeDataRepositoryTest.java | 23 +++++ .../spring/data/runtime/TypesConverter.java | 17 +++- .../io/quarkus/it/panache/TestEndpoint.java | 24 ++++++ .../it/panache/PanacheFunctionalityTest.java | 5 ++ .../it/panache/reactive/TestEndpoint.java | 27 ++++++ .../reactive/PanacheFunctionalityTest.java | 5 ++ .../resources/PersonRepositoryResource.java | 7 ++ .../panache/MongodbPanacheResourceTest.java | 7 ++ 15 files changed, 234 insertions(+), 6 deletions(-) diff --git a/docs/src/main/asciidoc/hibernate-orm-panache.adoc b/docs/src/main/asciidoc/hibernate-orm-panache.adoc index 250b1f18024f4..50520be3c429c 100644 --- a/docs/src/main/asciidoc/hibernate-orm-panache.adoc +++ b/docs/src/main/asciidoc/hibernate-orm-panache.adoc @@ -606,9 +606,12 @@ List persons = Person.list(Sort.by("name").and("birth")); // and with more restrictions List persons = Person.list("status", Sort.by("name").and("birth"), Status.Alive); + +// and list first the entries with null values in the field "birth" +List persons = Person.list(Sort.by("birth", Sort.NullPrecedence.NULLS_FIRST)); ---- -The `Sort` class has plenty of methods for adding columns and specifying sort direction. +The `Sort` class has plenty of methods for adding columns and specifying sort direction or the null precedence. === Simplified queries diff --git a/docs/src/main/asciidoc/hibernate-reactive-panache.adoc b/docs/src/main/asciidoc/hibernate-reactive-panache.adoc index 649fefebf6ca9..0d930e35be73f 100644 --- a/docs/src/main/asciidoc/hibernate-reactive-panache.adoc +++ b/docs/src/main/asciidoc/hibernate-reactive-panache.adoc @@ -491,9 +491,12 @@ Uni> persons = Person.list(Sort.by("name").and("birth")); // and with more restrictions Uni> persons = Person.list("status", Sort.by("name").and("birth"), Status.Alive); + +// and list first the entries with null values in the field "birth" +Uni> persons = Person.list(Sort.by("birth", Sort.NullPrecedence.NULLS_FIRST)); ---- -The `Sort` class has plenty of methods for adding columns and specifying sort direction. +The `Sort` class has plenty of methods for adding columns and specifying sort direction or the null precedence. === Simplified queries diff --git a/extensions/panache/hibernate-orm-panache/deployment/src/test/java/io/quarkus/hibernate/orm/panache/deployment/test/JpaOperationsSortTest.java b/extensions/panache/hibernate-orm-panache/deployment/src/test/java/io/quarkus/hibernate/orm/panache/deployment/test/JpaOperationsSortTest.java index 48e75fd971053..30e13c7a45c5f 100644 --- a/extensions/panache/hibernate-orm-panache/deployment/src/test/java/io/quarkus/hibernate/orm/panache/deployment/test/JpaOperationsSortTest.java +++ b/extensions/panache/hibernate-orm-panache/deployment/src/test/java/io/quarkus/hibernate/orm/panache/deployment/test/JpaOperationsSortTest.java @@ -27,4 +27,16 @@ public void testEmptySortEmptyYieldsEmptyString() { assertEquals("", PanacheJpaUtil.toOrderBy(emptySort)); } + @Test + public void testSortByNullsFirst() { + Sort emptySort = Sort.by("foo", Sort.Direction.Ascending, Sort.NullPrecedence.NULLS_FIRST); + assertEquals(" ORDER BY foo NULLS FIRST", PanacheJpaUtil.toOrderBy(emptySort)); + } + + @Test + public void testSortByNullsLast() { + Sort emptySort = Sort.by("foo", Sort.Direction.Descending, Sort.NullPrecedence.NULLS_LAST); + assertEquals(" ORDER BY foo DESC NULLS LAST", PanacheJpaUtil.toOrderBy(emptySort)); + } + } diff --git a/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/reactive/runtime/ReactiveMongoOperations.java b/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/reactive/runtime/ReactiveMongoOperations.java index 1bc547dd25f34..2ba4c3a035b3f 100644 --- a/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/reactive/runtime/ReactiveMongoOperations.java +++ b/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/reactive/runtime/ReactiveMongoOperations.java @@ -567,6 +567,9 @@ private Document sortToDocument(Sort sort) { Document sortDoc = new Document(); for (Sort.Column col : sort.getColumns()) { sortDoc.append(col.getName(), col.getDirection() == Sort.Direction.Ascending ? 1 : -1); + if (col.getNullPrecedence() != null) { + throw new UnsupportedOperationException("Cannot sort by nulls first or nulls last"); + } } return sortDoc; } diff --git a/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/runtime/MongoOperations.java b/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/runtime/MongoOperations.java index bca0e045f31fb..27ef1a5c68452 100644 --- a/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/runtime/MongoOperations.java +++ b/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/runtime/MongoOperations.java @@ -638,6 +638,9 @@ private Document sortToDocument(Sort sort) { Document sortDoc = new Document(); for (Sort.Column col : sort.getColumns()) { sortDoc.append(col.getName(), col.getDirection() == Sort.Direction.Ascending ? 1 : -1); + if (col.getNullPrecedence() != null) { + throw new UnsupportedOperationException("Cannot sort by nulls first or nulls last"); + } } return sortDoc; } diff --git a/extensions/panache/panache-common/runtime/src/main/java/io/quarkus/panache/common/Sort.java b/extensions/panache/panache-common/runtime/src/main/java/io/quarkus/panache/common/Sort.java index 4dfafee123bbb..d19ca3a7904d2 100644 --- a/extensions/panache/panache-common/runtime/src/main/java/io/quarkus/panache/common/Sort.java +++ b/extensions/panache/panache-common/runtime/src/main/java/io/quarkus/panache/common/Sort.java @@ -40,9 +40,24 @@ public enum Direction { Descending; } + /** + * Represents the order of null columns. + */ + public enum NullPrecedence { + /** + * Sort by the null columns listed first in the resulting query. + */ + NULLS_FIRST, + /** + * Sort by the null columns listed last in the resulting query. + */ + NULLS_LAST; + } + public static class Column { private String name; private Direction direction; + private NullPrecedence nullPrecedence; public Column(String name) { this(name, Direction.Ascending); @@ -53,6 +68,12 @@ public Column(String name, Direction direction) { this.direction = direction; } + public Column(String name, Direction direction, NullPrecedence nullPrecedence) { + this.name = name; + this.direction = direction; + this.nullPrecedence = nullPrecedence; + } + public String getName() { return name; } @@ -68,6 +89,14 @@ public Direction getDirection() { public void setDirection(Direction direction) { this.direction = direction; } + + public NullPrecedence getNullPrecedence() { + return nullPrecedence; + } + + public void setNullPrecedence(NullPrecedence nullPrecedence) { + this.nullPrecedence = nullPrecedence; + } } private List columns = new ArrayList<>(); @@ -91,7 +120,7 @@ public static Sort by(String column) { * Sort by the given column, in the given order. * * @param column the column to sort on, in the given order. - * @param direction the direction to sort on + * @param direction the direction to sort on. * @return a new Sort instance which sorts on the given column in the given order. * @see #by(String) * @see #by(String...) @@ -100,6 +129,33 @@ public static Sort by(String column, Direction direction) { return new Sort().and(column, direction); } + /** + * Sort by the given column, in the given order and in the given null precedence. + * + * @param column the column to sort on, in the given order. + * @param nullPrecedence the null precedence to use. + * @return a new Sort instance which sorts on the given column in the given order and null precedence. + * @see #by(String) + * @see #by(String...) + */ + public static Sort by(String column, NullPrecedence nullPrecedence) { + return by(column, Direction.Ascending, nullPrecedence); + } + + /** + * Sort by the given column, in the given order and in the given null precedence. + * + * @param column the column to sort on, in the given order. + * @param direction the direction to sort on. + * @param nullPrecedence the null precedence to use. + * @return a new Sort instance which sorts on the given column in the given order and null precedence. + * @see #by(String) + * @see #by(String...) + */ + public static Sort by(String column, Direction direction, NullPrecedence nullPrecedence) { + return new Sort().and(column, direction, nullPrecedence); + } + /** * Sort by the given columns, in ascending order. Equivalent to {@link #ascending(String...)}. * @@ -202,6 +258,7 @@ public Sort and(String name) { * Adds a sort column, in the given order. * * @param name the new column to sort on, in the given order. + * @param direction the direction to sort on. * @return this instance, modified. * @see #and(String) */ @@ -210,6 +267,32 @@ public Sort and(String name, Direction direction) { return this; } + /** + * Adds a sort column, in the given null precedence. + * + * @param name the new column to sort on, in the given null precedence. + * @param nullPrecedence the null precedence to use. + * @return this instance, modified. + * @see #and(String) + */ + public Sort and(String name, NullPrecedence nullPrecedence) { + return and(name, Direction.Ascending, nullPrecedence); + } + + /** + * Adds a sort column, in the given order and null precedence. + * + * @param name the new column to sort on, in the given order and null precedence. + * @param direction the direction to sort on. + * @param nullPrecedence the null precedence to use. + * @return this instance, modified. + * @see #and(String) + */ + public Sort and(String name, Direction direction, NullPrecedence nullPrecedence) { + columns.add(new Column(name, direction, nullPrecedence)); + return this; + } + /** * Get the sort columns * diff --git a/extensions/panache/panache-hibernate-common/runtime/src/main/java/io/quarkus/panache/hibernate/common/runtime/PanacheJpaUtil.java b/extensions/panache/panache-hibernate-common/runtime/src/main/java/io/quarkus/panache/hibernate/common/runtime/PanacheJpaUtil.java index 08a930cbaef3f..06b6eba3b6721 100644 --- a/extensions/panache/panache-hibernate-common/runtime/src/main/java/io/quarkus/panache/hibernate/common/runtime/PanacheJpaUtil.java +++ b/extensions/panache/panache-hibernate-common/runtime/src/main/java/io/quarkus/panache/hibernate/common/runtime/PanacheJpaUtil.java @@ -177,8 +177,18 @@ public static String toOrderBy(Sort sort) { if (i > 0) sb.append(" , "); sb.append(column.getName()); - if (column.getDirection() != Sort.Direction.Ascending) + if (column.getDirection() != Sort.Direction.Ascending) { sb.append(" DESC"); + } + + if (column.getNullPrecedence() != null) { + if (column.getNullPrecedence() == Sort.NullPrecedence.NULLS_FIRST) { + sb.append(" NULLS FIRST"); + } else { + sb.append(" NULLS LAST"); + } + } + } return sb.toString(); } diff --git a/extensions/spring-data-jpa/deployment/src/test/java/io/quarkus/spring/data/deployment/BasicTypeDataRepositoryTest.java b/extensions/spring-data-jpa/deployment/src/test/java/io/quarkus/spring/data/deployment/BasicTypeDataRepositoryTest.java index 9ad3e63ca6608..512d9a2b57493 100644 --- a/extensions/spring-data-jpa/deployment/src/test/java/io/quarkus/spring/data/deployment/BasicTypeDataRepositoryTest.java +++ b/extensions/spring-data-jpa/deployment/src/test/java/io/quarkus/spring/data/deployment/BasicTypeDataRepositoryTest.java @@ -29,6 +29,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Slice; +import org.springframework.data.domain.Sort; import io.quarkus.test.QuarkusUnitTest; @@ -124,6 +125,28 @@ public void testMapInterfacesUsingSlice() { assertEquals(0, nextPage.getNumberOfElements()); } + @Test + @Order(8) + @Transactional + public void testListOrderByNullHandling() throws MalformedURLException { + // Insert row with null in url + UUID uuidForTheNullUrlRecord = UUID.randomUUID(); + BasicTypeData item = populateData(new BasicTypeData()); + item.setUuid(uuidForTheNullUrlRecord); + item.setUrl(null); + repo.save(item); + // At this moment, there should be at least two records, one inserted in the first test, + // and the second with the "url" column to null. + + // Check Nulls first + List list = repo.findAll(Sort.by(Sort.Order.by("url").nullsFirst())); + assertEquals(uuidForTheNullUrlRecord, list.get(0).getUuid()); + + // Check Nulls last + list = repo.findAll(Sort.by(Sort.Order.by("url").nullsLast())); + assertEquals(uuidForTheNullUrlRecord, list.get(list.size() - 1).getUuid()); + } + private BasicTypeData populateData(BasicTypeData basicTypeData) throws MalformedURLException { basicTypeData.setDoubleValue(Math.PI); basicTypeData.setBigDecimalValue(BigDecimal.valueOf(Math.PI * 2.0)); diff --git a/extensions/spring-data-jpa/runtime/src/main/java/io/quarkus/spring/data/runtime/TypesConverter.java b/extensions/spring-data-jpa/runtime/src/main/java/io/quarkus/spring/data/runtime/TypesConverter.java index 5963a8e894803..84bfd77757fb9 100644 --- a/extensions/spring-data-jpa/runtime/src/main/java/io/quarkus/spring/data/runtime/TypesConverter.java +++ b/extensions/spring-data-jpa/runtime/src/main/java/io/quarkus/spring/data/runtime/TypesConverter.java @@ -18,14 +18,14 @@ public static io.quarkus.panache.common.Sort toPanacheSort(org.springframework.d } org.springframework.data.domain.Sort.Order firstOrder = orders.get(0); - Sort result = Sort.by(firstOrder.getProperty(), getDirection(firstOrder)); + Sort result = Sort.by(firstOrder.getProperty(), getDirection(firstOrder), getNullPrecedence(firstOrder)); if (orders.size() == 1) { return result; } for (int i = 1; i < orders.size(); i++) { org.springframework.data.domain.Sort.Order order = orders.get(i); - result = result.and(order.getProperty(), getDirection(order)); + result = result.and(order.getProperty(), getDirection(order), getNullPrecedence(order)); } return result; } @@ -35,6 +35,19 @@ private static Sort.Direction getDirection(org.springframework.data.domain.Sort. : Sort.Direction.Descending; } + private static Sort.NullPrecedence getNullPrecedence(org.springframework.data.domain.Sort.Order order) { + if (order.getNullHandling() != null) { + switch (order.getNullHandling()) { + case NULLS_FIRST: + return Sort.NullPrecedence.NULLS_FIRST; + case NULLS_LAST: + return Sort.NullPrecedence.NULLS_LAST; + } + } + + return null; + } + public static io.quarkus.panache.common.Page toPanachePage(org.springframework.data.domain.Pageable pageable) { // only generate queries with paging if param is actually paged (ex. Unpaged.INSTANCE is a Pageable not paged) if (pageable.isPaged()) { diff --git a/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java index 2daf1aa31ebd7..90ff34b95be9f 100644 --- a/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java +++ b/integration-tests/hibernate-orm-panache/src/main/java/io/quarkus/it/panache/TestEndpoint.java @@ -1519,4 +1519,28 @@ public String testFilterWithCollections() { return "OK"; } + + @GET + @Path("testSortByNullPrecedence") + @Transactional + public String testSortByNullPrecedence() { + Person.deleteAll(); + + Person stefPerson = new Person(); + stefPerson.name = "Stef"; + stefPerson.persist(); + + Person josePerson = new Person(); + josePerson.name = null; + josePerson.persist(); + + List persons = Person.findAll(Sort.by("name", Sort.NullPrecedence.NULLS_FIRST)).list(); + assertEquals(josePerson.id, persons.get(0).id); + persons = Person.findAll(Sort.by("name", Sort.NullPrecedence.NULLS_LAST)).list(); + assertEquals(josePerson.id, persons.get(persons.size() - 1).id); + + Person.deleteAll(); + + return "OK"; + } } diff --git a/integration-tests/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java b/integration-tests/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java index 9beeb594209db..8b8a84ef9d5ed 100644 --- a/integration-tests/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java +++ b/integration-tests/hibernate-orm-panache/src/test/java/io/quarkus/it/panache/PanacheFunctionalityTest.java @@ -85,6 +85,11 @@ public void testFilterWithCollections() { RestAssured.when().get("/test/testFilterWithCollections").then().body(is("OK")); } + @Test + public void testSortByNullPrecedence() { + RestAssured.when().get("/test/testSortByNullPrecedence").then().body(is("OK")); + } + @Test public void testJaxbAnnotationTransfer() { RestAssured.when() diff --git a/integration-tests/hibernate-reactive-panache/src/main/java/io/quarkus/it/panache/reactive/TestEndpoint.java b/integration-tests/hibernate-reactive-panache/src/main/java/io/quarkus/it/panache/reactive/TestEndpoint.java index 4a9ca0cb80f73..6a8931ea36564 100644 --- a/integration-tests/hibernate-reactive-panache/src/main/java/io/quarkus/it/panache/reactive/TestEndpoint.java +++ b/integration-tests/hibernate-reactive-panache/src/main/java/io/quarkus/it/panache/reactive/TestEndpoint.java @@ -1878,4 +1878,31 @@ public Uni testBug9036() { }).map(v -> "OK"); }); } + + @GET + @Path("testSortByNullPrecedence") + @ReactiveTransactional + public Uni testSortByNullPrecedence() { + return Person.deleteAll() + .flatMap(v -> { + Person stefPerson = new Person(); + stefPerson.name = "Stef"; + stefPerson.uniqueName = "stef"; + + Person josePerson = new Person(); + josePerson.name = null; + josePerson.uniqueName = "jose"; + return Person.persist(stefPerson, josePerson); + }).flatMap(p -> { + return Person.findAll(Sort.by("name", Sort.NullPrecedence.NULLS_FIRST)).list(); + }).flatMap(list -> { + assertEquals("jose", ((Person) list.get(0)).uniqueName); + + return Person.findAll(Sort.by("name", Sort.NullPrecedence.NULLS_LAST)).list(); + }).flatMap(list -> { + assertEquals("jose", ((Person) list.get(list.size() - 1)).uniqueName); + + return Person.deleteAll(); + }).map(v -> "OK"); + } } diff --git a/integration-tests/hibernate-reactive-panache/src/test/java/io/quarkus/it/panache/reactive/PanacheFunctionalityTest.java b/integration-tests/hibernate-reactive-panache/src/test/java/io/quarkus/it/panache/reactive/PanacheFunctionalityTest.java index ca07721d4cb16..bb1746121b4ad 100644 --- a/integration-tests/hibernate-reactive-panache/src/test/java/io/quarkus/it/panache/reactive/PanacheFunctionalityTest.java +++ b/integration-tests/hibernate-reactive-panache/src/test/java/io/quarkus/it/panache/reactive/PanacheFunctionalityTest.java @@ -151,6 +151,11 @@ public void testBug9036() { RestAssured.when().get("/test/9036").then().body(is("OK")); } + @Test + public void testSortByNullPrecedence() { + RestAssured.when().get("/test/testSortByNullPrecedence").then().body(is("OK")); + } + @DisabledOnNativeImage @ReactiveTransactional @Test diff --git a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/person/resources/PersonRepositoryResource.java b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/person/resources/PersonRepositoryResource.java index 3a614f7f29a05..3d0219aa63a5d 100644 --- a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/person/resources/PersonRepositoryResource.java +++ b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/person/resources/PersonRepositoryResource.java @@ -104,4 +104,11 @@ public Response rename(@QueryParam("previousName") String previousName, @QueryPa personRepository.update("lastname", newName).where("lastname", previousName); return Response.ok().build(); } + + @GET + @Path("/search/by/nulls/precedence") + public Response searchPersonsByNullsPrecedence() { + personRepository.listAll(Sort.by("lastname", Sort.NullPrecedence.NULLS_FIRST)); + return Response.ok().build(); + } } diff --git a/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java b/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java index 06c8ed04e0b61..b6bfc702e3040 100644 --- a/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java +++ b/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java @@ -11,6 +11,7 @@ import java.util.GregorianCalendar; import java.util.List; +import org.apache.http.HttpStatus; import org.hamcrest.CoreMatchers; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -67,6 +68,12 @@ public void testPersonRepository() { callPersonEndpoint("/persons/repository"); } + @Test + public void testShouldThrowExceptionWhenUsingNullsPrecedence() { + get("/persons/repository/search/by/nulls/precedence") + .then().statusCode(HttpStatus.SC_INTERNAL_SERVER_ERROR); + } + private void callBookEndpoint(String endpoint) { RestAssured.defaultParser = Parser.JSON; RestAssured.config