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

pom.xml

@@ -5,7 +5,7 @@
 
 	<groupId>org.springframework.data</groupId>
 	<artifactId>spring-data-redis</artifactId>
-	<version>1.8.0.BUILD-SNAPSHOT</version>
+	<version>1.8.0.DATAREDIS-547-SNAPSHOT</version>
 
 	<name>Spring Data Redis</name>
 
@@ -17,7 +17,7 @@
 
 	<properties>
 		<dist.key>DATAREDIS</dist.key>
-		<springdata.keyvalue>1.2.0.BUILD-SNAPSHOT</springdata.keyvalue>
+		<springdata.keyvalue>1.2.0.DATAKV-142-SNAPSHOT</springdata.keyvalue>
 		<jta>1.1</jta>
 		<beanutils>1.9.2</beanutils>
 		<xstream>1.4.8</xstream>

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

@@ -579,6 +579,7 @@ Here's an overview of the keywords supported for Redis and what a method contain
 |`And`|`findByLastnameAndFirstname`|`SINTER …:firstname:rand …:lastname:al’thor`
 |`Or`|`findByLastnameOrFirstname`|`SUNION …:firstname:rand …:lastname:al’thor`
 |`Is,Equals`|`findByFirstname`,`findByFirstnameIs`,`findByFirstnameEquals`|`SINTER …:firstname:rand`
+|`Top,First`|`findFirst10ByFirstname`,`findTop5ByFirstname`|
 |===============
 ====
 

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

@@ -18,6 +18,7 @@
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -60,7 +61,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 +346,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 +362,20 @@ 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);
+
+
+		if (keys.isEmpty() || keys.size() < offset) {
+			return Collections.emptyList();
+		}
+
+		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
@@ -95,12 +102,13 @@ public Map<byte[], Map<byte[], byte[]>> doInRedis(RedisConnection connection) th
 
 				final Map<byte[], Map<byte[], byte[]>> rawData = new LinkedHashMap<byte[], Map<byte[], byte[]>>();
 
-				if (allKeys.size() == 0 || allKeys.size() < offset) {
+				if (allKeys.isEmpty() || allKeys.size() < offset) {
 					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,12 +41,13 @@
 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.
  * 
  * @author Christoph Strobl
+ * @author Mark Paluch
  */
 public abstract class RedisRepositoryIntegrationTestBase {
 
@@ -190,7 +190,117 @@ 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 shouldReturnEmptyListWhenPageableOutOfBoundsUsingFindAll() {
+
+		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(100, 2));
+		assertThat(firstPage.getContent(), hasSize(0));
+	}
+
+	/**
+	 * @see DATAREDIS-547
+	 */
+	@Test
+	public void shouldReturnEmptyListWhenPageableOutOfBoundsUsingQueryMethod() {
+
+		Person eddard = new Person("eddard", "stark");
+		Person robb = new Person("robb", "stark");
+		Person sansa = new Person("sansa", "stark");
+
+		repo.save(Arrays.asList(eddard, robb, sansa));
+
+		Page<Person> page1 = repo.findPersonByLastname("stark", new PageRequest(1, 3));
+
+		assertThat(page1.getNumberOfElements(), is(0));
+		assertThat(page1.getContent(), hasSize(0));
+		assertThat(page1.getTotalElements(), is(3L));
+
+		Page<Person> page2 = repo.findPersonByLastname("stark", new PageRequest(2, 3));
+
+		assertThat(page2.getNumberOfElements(), is(0));
+		assertThat(page2.getContent(), hasSize(0));
+		assertThat(page2.getTotalElements(), is(3L));
+	}
+
+	/**
+	 * @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 +311,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);
 	}
 
 	/**