DATAREDIS-547 - Fix query execution when derived criteria is empty.

We now make sure to pipe finder queries without any criteria to the according find all method. This allows usage of `PagingAndSortingRepository.findAllBy(Pageable page)` as well as finders without any criteria like `findTop3By()`.

Original pull request: #216.

Author: christophstrobl

src/main/java/org/springframework/data/redis/core/RedisKeyValueAdapter.java

@@ -60,7 +60,6 @@
 import org.springframework.data.redis.util.ByteUtils;
 import org.springframework.data.util.CloseableIterator;
 import org.springframework.util.Assert;
-import org.springframework.util.NumberUtils;
 import org.springframework.util.ObjectUtils;
 
 /**
@@ -346,6 +345,10 @@ public Void doInRedis(RedisConnection connection) throws DataAccessException {
 	 * @see org.springframework.data.keyvalue.core.KeyValueAdapter#getAllOf(java.io.Serializable)
 	 */
 	public List<?> getAllOf(final Serializable keyspace) {
+		return getAllOf(keyspace, -1, -1);
+	}
+
+	public List<?> getAllOf(final Serializable keyspace, int offset, int rows) {
 
 		final byte[] binKeyspace = toBytes(keyspace);
 
@@ -358,7 +361,15 @@ public Set<byte[]> doInRedis(RedisConnection connection) throws DataAccessExcept
 		});
 
 		List<Object> result = new ArrayList<Object>();
-		for (byte[] key : ids) {
+
+		List<byte[]> keys = new ArrayList<byte[]>(ids);
+
+		offset = Math.max(0, offset);
+		if (offset >= 0 && rows > 0) {
+			keys = keys.subList(offset, Math.min(offset + rows, keys.size()));
+		}
+
+		for (byte[] key : keys) {
 			result.add(get(key, keyspace));
 		}
 		return result;

src/main/java/org/springframework/data/redis/core/RedisQueryEngine.java

@@ -34,6 +34,7 @@
 import org.springframework.data.redis.repository.query.RedisOperationChain;
 import org.springframework.data.redis.repository.query.RedisOperationChain.PathAndValue;
 import org.springframework.data.redis.util.ByteUtils;
+import org.springframework.util.CollectionUtils;
 
 /**
  * Redis specific {@link QueryEngine} implementation.
@@ -67,9 +68,15 @@ public RedisQueryEngine(CriteriaAccessor<RedisOperationChain> criteriaAccessor,
 	 * @see org.springframework.data.keyvalue.core.QueryEngine#execute(java.lang.Object, java.lang.Object, int, int, java.io.Serializable, java.lang.Class)
 	 */
 	@Override
+	@SuppressWarnings("unchecked")
 	public <T> Collection<T> execute(final RedisOperationChain criteria, final Comparator<?> sort, final int offset,
 			final int rows, final Serializable keyspace, Class<T> type) {
 
+		if (criteria == null
+				|| (CollectionUtils.isEmpty(criteria.getOrSismember()) && CollectionUtils.isEmpty(criteria.getSismember()))) {
+			return (Collection<T>) getAdapter().getAllOf(keyspace, offset, rows);
+		}
+
 		RedisCallback<Map<byte[], Map<byte[], byte[]>>> callback = new RedisCallback<Map<byte[], Map<byte[], byte[]>>>() {
 
 			@Override
@@ -99,8 +106,9 @@ public Map<byte[], Map<byte[], byte[]>> doInRedis(RedisConnection connection) th
 					return Collections.emptyMap();
 				}
 
-				if (offset >= 0 && rows > 0) {
-					allKeys = allKeys.subList(Math.max(0, offset), Math.min(offset + rows, allKeys.size()));
+				int offsetToUse = Math.max(0, offset);
+				if (rows > 0) {
+					allKeys = allKeys.subList(Math.max(0, offsetToUse), Math.min(offsetToUse + rows, allKeys.size()));
 				}
 				for (byte[] id : allKeys) {
 
@@ -171,8 +179,8 @@ private byte[][] keys(String prefix, Collection<PathAndValue> source) {
 		int i = 0;
 		for (PathAndValue pathAndValue : source) {
 
-			byte[] convertedValue = getAdapter().getConverter().getConversionService()
-					.convert(pathAndValue.getFirstValue(), byte[].class);
+			byte[] convertedValue = getAdapter().getConverter().getConversionService().convert(pathAndValue.getFirstValue(),
+					byte[].class);
 			byte[] fullPath = getAdapter().getConverter().getConversionService()
 					.convert(prefix + pathAndValue.getPath() + ":", byte[].class);
 

src/main/java/org/springframework/data/redis/repository/query/RedisQueryCreator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 the original author or authors.
+ * Copyright 2015-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@
 import org.springframework.data.repository.query.parser.AbstractQueryCreator;
 import org.springframework.data.repository.query.parser.Part;
 import org.springframework.data.repository.query.parser.PartTree;
+import org.springframework.util.CollectionUtils;
 
 /**
  * Redis specific query creator.
@@ -87,11 +88,13 @@ protected KeyValueQuery<RedisOperationChain> complete(final RedisOperationChain
 
 		KeyValueQuery<RedisOperationChain> query = new KeyValueQuery<RedisOperationChain>(criteria);
 
-		if (query.getCritieria().getSismember().size() == 1 && query.getCritieria().getOrSismember().size() == 1) {
+		if (query.getCritieria() != null && !CollectionUtils.isEmpty(query.getCritieria().getSismember())
+				&& !CollectionUtils.isEmpty(query.getCritieria().getOrSismember()))
+			if (query.getCritieria().getSismember().size() == 1 && query.getCritieria().getOrSismember().size() == 1) {
 
-			query.getCritieria().getOrSismember().add(query.getCritieria().getSismember().iterator().next());
-			query.getCritieria().getSismember().clear();
-		}
+				query.getCritieria().getOrSismember().add(query.getCritieria().getSismember().iterator().next());
+				query.getCritieria().getSismember().clear();
+			}
 
 		if (sort != null) {
 			query.setSort(sort);

src/test/java/org/springframework/data/redis/repository/RedisRepositoryIntegrationTestBase.java

@@ -15,10 +15,9 @@
  */
 package org.springframework.data.redis.repository;
 
+import static org.hamcrest.CoreMatchers.*;
 import static org.hamcrest.collection.IsCollectionWithSize.*;
 import static org.hamcrest.collection.IsIterableContainingInAnyOrder.*;
-import static org.hamcrest.core.Is.*;
-import static org.hamcrest.core.IsCollectionContaining.*;
 import static org.junit.Assert.*;
 
 import java.io.Serializable;
@@ -42,7 +41,7 @@
 import org.springframework.data.redis.core.index.IndexDefinition;
 import org.springframework.data.redis.core.index.Indexed;
 import org.springframework.data.redis.core.index.SimpleIndexDefinition;
-import org.springframework.data.repository.CrudRepository;
+import org.springframework.data.repository.PagingAndSortingRepository;
 
 /**
  * Base for testing Redis repository support in different configurations.
@@ -190,7 +189,76 @@ public void findUsingOrReturnsResultCorrectly() {
 		assertThat(eddardAndJon, containsInAnyOrder(eddard, jon));
 	}
 
-	public static interface PersonRepository extends CrudRepository<Person, String> {
+	/**
+	 * @see DATAREDIS-547
+	 */
+	@Test
+	public void shouldApplyFirstKeywordCorrectly() {
+
+		Person eddard = new Person("eddard", "stark");
+		Person robb = new Person("robb", "stark");
+		Person jon = new Person("jon", "snow");
+
+		repo.save(Arrays.asList(eddard, robb, jon));
+
+		assertThat(repo.findFirstBy(), hasSize(1));
+	}
+
+	/**
+	 * @see DATAREDIS-547
+	 */
+	@Test
+	public void shouldApplyPageableCorrectlyWhenUsingFindAll() {
+
+		Person eddard = new Person("eddard", "stark");
+		Person robb = new Person("robb", "stark");
+		Person jon = new Person("jon", "snow");
+
+		repo.save(Arrays.asList(eddard, robb, jon));
+
+		Page<Person> firstPage = repo.findAll(new PageRequest(0, 2));
+		assertThat(firstPage.getContent(), hasSize(2));
+		assertThat(repo.findAll(firstPage.nextPageable()).getContent(), hasSize(1));
+	}
+
+	/**
+	 * @see DATAREDIS-547
+	 */
+	@Test
+	public void shouldApplyTopKeywordCorrectly() {
+
+		Person eddard = new Person("eddard", "stark");
+		Person robb = new Person("robb", "stark");
+		Person jon = new Person("jon", "snow");
+
+		repo.save(Arrays.asList(eddard, robb, jon));
+
+		assertThat(repo.findTop2By(), hasSize(2));
+	}
+
+	/**
+	 * @see DATAREDIS-547
+	 */
+	@Test
+	public void shouldApplyTopKeywordCorrectlyWhenCriteriaPresent() {
+
+		Person eddard = new Person("eddard", "stark");
+		Person tyrion = new Person("tyrion", "lannister");
+		Person robb = new Person("robb", "stark");
+		Person jon = new Person("jon", "snow");
+		Person arya = new Person("arya", "stark");
+
+		repo.save(Arrays.asList(eddard, tyrion, robb, jon, arya));
+
+		List<Person> result = repo.findTop2ByLastname("stark");
+
+		assertThat(result, hasSize(2));
+		for (Person p : result) {
+			assertThat(p.getLastname(), is("stark"));
+		}
+	}
+
+	public static interface PersonRepository extends PagingAndSortingRepository<Person, String> {
 
 		List<Person> findByFirstname(String firstname);
 
@@ -201,6 +269,12 @@ public static interface PersonRepository extends CrudRepository<Person, String>
 		List<Person> findByFirstnameAndLastname(String firstname, String lastname);
 
 		List<Person> findByFirstnameOrLastname(String firstname, String lastname);
+
+		List<Person> findFirstBy();
+
+		List<Person> findTop2By();
+
+		List<Person> findTop2ByLastname(String lastname);
 	}
 
 	/**