From a4e3f40527eca4214650aac5122bddbab930d439 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 15 Jun 2021 09:48:51 +0200 Subject: [PATCH 1/2] Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 43b9449793..34022534ca 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-redis - 2.5.2-SNAPSHOT + 2.5.2-2080-SNAPSHOT Spring Data Redis From 2a4be41f700c0b1c3affc63e3d1ea340dbed8faa Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 15 Jun 2021 10:12:06 +0200 Subject: [PATCH 2/2] Fix result sorting using Redis repository query methods. We now correctly apply sorting when calling a repository query method with a Sort parameter or when defining the sort order as part of the method name. --- .../reference/redis-repositories.adoc | 21 +++++++- .../data/redis/core/RedisQueryEngine.java | 14 +++++- .../RedisRepositoryIntegrationTestBase.java | 48 ++++++++++++++++++- 3 files changed, 80 insertions(+), 3 deletions(-) diff --git a/src/main/asciidoc/reference/redis-repositories.adoc b/src/main/asciidoc/reference/redis-repositories.adoc index 3c46cad69f..c598c0cd97 100644 --- a/src/main/asciidoc/reference/redis-repositories.adoc +++ b/src/main/asciidoc/reference/redis-repositories.adoc @@ -673,7 +673,6 @@ public interface PersonRepository extends CrudRepository { ---- ==== - NOTE: Please make sure properties used in finder methods are set up for indexing. NOTE: Query methods for Redis repositories support only queries for entities and collections of entities with paging. @@ -711,6 +710,26 @@ The following table provides an overview of the keywords supported for Redis and |=============== ==== +[[redis.repositories.queries.sort]] +=== Sorting Query Method results + +Redis repositories allow various approaches to define sorting order. Redis itself does not support in-flight sorting when retrieving hashes or sets. Therefore, Redis repository query methods construct a `Comparator` that is applied to the result before returning results as `List`. Let's take a look at the following example: + +.Sorting Query Results +==== +[source,java] +---- +interface PersonRepository extends RedisRepository { + + List findByFirstnameSortByAgeDesc(String firstname); <1> + + List findByFirstname(String firstname, Sort sort); <2> +} +---- +<1> Static sorting derived from method name. +<2> Dynamic sorting using a method argument. +==== + [[redis.repositories.cluster]] == Redis Repositories Running on a Cluster diff --git a/src/main/java/org/springframework/data/redis/core/RedisQueryEngine.java b/src/main/java/org/springframework/data/redis/core/RedisQueryEngine.java index 358e266b29..77de56d926 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisQueryEngine.java +++ b/src/main/java/org/springframework/data/redis/core/RedisQueryEngine.java @@ -29,6 +29,7 @@ import org.springframework.data.keyvalue.core.CriteriaAccessor; import org.springframework.data.keyvalue.core.QueryEngine; import org.springframework.data.keyvalue.core.SortAccessor; +import org.springframework.data.keyvalue.core.SpelSortAccessor; import org.springframework.data.keyvalue.core.query.KeyValueQuery; import org.springframework.data.redis.connection.RedisGeoCommands.GeoLocation; import org.springframework.data.redis.core.convert.GeoIndexedPropertyValue; @@ -37,6 +38,7 @@ import org.springframework.data.redis.repository.query.RedisOperationChain.NearPath; import org.springframework.data.redis.repository.query.RedisOperationChain.PathAndValue; import org.springframework.data.redis.util.ByteUtils; +import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; @@ -53,7 +55,7 @@ class RedisQueryEngine extends QueryEngine criteriaAccessor, @SuppressWarnings("unchecked") public Collection execute(RedisOperationChain criteria, Comparator sort, long offset, int rows, String keyspace, Class type) { + List result = doFind(criteria, offset, rows, keyspace, type); + + if (sort != null) { + result.sort((Comparator) sort); + } + + return result; + } + + private List doFind(RedisOperationChain criteria, long offset, int rows, String keyspace, Class type) { if (criteria == null || (CollectionUtils.isEmpty(criteria.getOrSismember()) && CollectionUtils.isEmpty(criteria.getSismember())) diff --git a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java index 1e0faefd12..e8a22ab6f6 100644 --- a/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java +++ b/src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java @@ -19,13 +19,13 @@ import lombok.Data; import lombok.Value; +import lombok.With; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; -import lombok.With; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -36,6 +36,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; import org.springframework.data.geo.Distance; import org.springframework.data.geo.Metrics; import org.springframework.data.geo.Point; @@ -117,6 +118,45 @@ void simpleFindByMultipleProperties() { assertThat(repo.findByFirstnameAndLastname("egwene", "al'vere").get(0)).isEqualTo(egwene); } + @Test // GH-2080 + void simpleFindAndSort() { + + Person egwene = new Person(); + egwene.firstname = "egwene"; + egwene.lastname = "al'vere"; + + Person marin = new Person(); + marin.firstname = "marin"; + marin.lastname = "al'vere"; + + repo.saveAll(Arrays.asList(egwene, marin)); + + assertThat(repo.findByLastname("al'vere", Sort.by(Sort.Direction.ASC, "firstname"))).containsSequence(egwene, + marin); + assertThat(repo.findByLastname("al'vere", Sort.by(Sort.Direction.DESC, "firstname"))).containsSequence(marin, + egwene); + + assertThat(repo.findByLastnameOrderByFirstnameAsc("al'vere")).containsSequence(egwene, marin); + assertThat(repo.findByLastnameOrderByFirstnameDesc("al'vere")).containsSequence(marin, egwene); + } + + @Test // GH-2080 + void simpleFindAllWithSort() { + + Person egwene = new Person(); + egwene.firstname = "egwene"; + egwene.lastname = "al'vere"; + + Person marin = new Person(); + marin.firstname = "marin"; + marin.lastname = "al'vere"; + + repo.saveAll(Arrays.asList(egwene, marin)); + + assertThat(repo.findAll(Sort.by(Sort.Direction.ASC, "firstname"))).containsSequence(egwene, marin); + assertThat(repo.findAll(Sort.by(Sort.Direction.DESC, "firstname"))).containsSequence(marin, egwene); + } + @Test // DATAREDIS-425 void findReturnsReferenceDataCorrectly() { @@ -441,6 +481,12 @@ public static interface PersonRepository List findByLastname(String lastname); + List findByLastname(String lastname, Sort sort); + + List findByLastnameOrderByFirstnameAsc(String lastname); + + List findByLastnameOrderByFirstnameDesc(String lastname); + Page findPersonByLastname(String lastname, Pageable page); List findPersonByAliveIsTrue();