Fix result sorting using Redis repository query methods

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-redis</artifactId>
-	<version>2.5.2-SNAPSHOT</version>
+	<version>2.5.2-2080-SNAPSHOT</version>
 
 	<name>Spring Data Redis</name>
 

src/main/asciidoc/reference/redis-repositories.adoc

@@ -673,7 +673,6 @@ public interface PersonRepository extends CrudRepository<Person, String> {
 ----
 ====
 
-
 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<Person, String> {
+
+  List<Person> findByFirstnameSortByAgeDesc(String firstname); <1>
+
+  List<Person> 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
 

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<RedisKeyValueAdapter, RedisOperationC
 	 * Creates new {@link RedisQueryEngine} with defaults.
 	 */
 	RedisQueryEngine() {
-		this(new RedisCriteriaAccessor(), null);
+		this(new RedisCriteriaAccessor(), new SpelSortAccessor(new SpelExpressionParser()));
 	}
 
 	/**
@@ -76,6 +78,16 @@ private RedisQueryEngine(CriteriaAccessor<RedisOperationChain> criteriaAccessor,
 	@SuppressWarnings("unchecked")
 	public <T> Collection<T> execute(RedisOperationChain criteria, Comparator<?> sort, long offset, int rows,
 			String keyspace, Class<T> type) {
+		List<T> result = doFind(criteria, offset, rows, keyspace, type);
+
+		if (sort != null) {
+			result.sort((Comparator<? super T>) sort);
+		}
+
+		return result;
+	}
+
+	private <T> List<T> doFind(RedisOperationChain criteria, long offset, int rows, String keyspace, Class<T> type) {
 
 		if (criteria == null
 				|| (CollectionUtils.isEmpty(criteria.getOrSismember()) && CollectionUtils.isEmpty(criteria.getSismember()))

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<Person> findByLastname(String lastname);
 
+		List<Person> findByLastname(String lastname, Sort sort);
+
+		List<Person> findByLastnameOrderByFirstnameAsc(String lastname);
+
+		List<Person> findByLastnameOrderByFirstnameDesc(String lastname);
+
 		Page<Person> findPersonByLastname(String lastname, Pageable page);
 
 		List<Person> findPersonByAliveIsTrue();