Skip to content

Commit a93f81f

Browse files
christophstroblmp911de
christophstrobl
authored and
mp911de
committed
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.
1 parent 253fca7 commit a93f81f

File tree

4 files changed

+78
-29
lines changed

4 files changed

+78
-29
lines changed

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

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -338,32 +338,33 @@ public Void doInRedis(RedisConnection connection) throws DataAccessException {
338338
* @see org.springframework.data.keyvalue.core.KeyValueAdapter#getAllOf(java.io.Serializable)
339339
*/
340340
public List<?> getAllOf(final Serializable keyspace) {
341+
return getAllOf(keyspace, -1, -1);
342+
}
343+
344+
public List<?> getAllOf(final Serializable keyspace, int offset, int rows) {
341345

342346
final byte[] binKeyspace = toBytes(keyspace);
343347

344-
List<Map<byte[], byte[]>> raw = redisOps.execute(new RedisCallback<List<Map<byte[], byte[]>>>() {
348+
Set<byte[]> ids = redisOps.execute(new RedisCallback<Set<byte[]>>() {
345349

346350
@Override
347-
public List<Map<byte[], byte[]>> doInRedis(RedisConnection connection) throws DataAccessException {
348-
349-
final List<Map<byte[], byte[]>> rawData = new ArrayList<Map<byte[], byte[]>>();
350-
351-
Set<byte[]> members = connection.sMembers(binKeyspace);
352-
353-
for (byte[] id : members) {
354-
rawData.add(connection
355-
.hGetAll(createKey(asString(keyspace), getConverter().getConversionService().convert(id, String.class))));
356-
}
357-
358-
return rawData;
351+
public Set<byte[]> doInRedis(RedisConnection connection) throws DataAccessException {
352+
return connection.sMembers(binKeyspace);
359353
}
360354
});
361355

362-
List<Object> result = new ArrayList<Object>(raw.size());
363-
for (Map<byte[], byte[]> rawData : raw) {
364-
result.add(converter.read(Object.class, new RedisData(rawData)));
356+
List<Object> result = new ArrayList<Object>();
357+
358+
List<byte[]> keys = new ArrayList<byte[]>(ids);
359+
360+
offset = Math.max(0, offset);
361+
if (offset >= 0 && rows > 0) {
362+
keys = keys.subList(offset, Math.min(offset + rows, keys.size()));
365363
}
366364

365+
for (byte[] key : keys) {
366+
result.add(get(key, keyspace));
367+
}
367368
return result;
368369
}
369370

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.springframework.data.redis.repository.query.RedisOperationChain;
3535
import org.springframework.data.redis.repository.query.RedisOperationChain.PathAndValue;
3636
import org.springframework.data.redis.util.ByteUtils;
37+
import org.springframework.util.CollectionUtils;
3738

3839
/**
3940
* Redis specific {@link QueryEngine} implementation.
@@ -67,9 +68,15 @@ public RedisQueryEngine(CriteriaAccessor<RedisOperationChain> criteriaAccessor,
6768
* @see org.springframework.data.keyvalue.core.QueryEngine#execute(java.lang.Object, java.lang.Object, int, int, java.io.Serializable, java.lang.Class)
6869
*/
6970
@Override
71+
@SuppressWarnings("unchecked")
7072
public <T> Collection<T> execute(final RedisOperationChain criteria, final Comparator<?> sort, final int offset,
7173
final int rows, final Serializable keyspace, Class<T> type) {
7274

75+
if (criteria == null
76+
|| (CollectionUtils.isEmpty(criteria.getOrSismember()) && CollectionUtils.isEmpty(criteria.getSismember()))) {
77+
return (Collection<T>) getAdapter().getAllOf(keyspace, offset, rows);
78+
}
79+
7380
RedisCallback<Map<byte[], Map<byte[], byte[]>>> callback = new RedisCallback<Map<byte[], Map<byte[], byte[]>>>() {
7481

7582
@Override
@@ -99,8 +106,9 @@ public Map<byte[], Map<byte[], byte[]>> doInRedis(RedisConnection connection) th
99106
return Collections.emptyMap();
100107
}
101108

102-
if (offset >= 0 && rows > 0) {
103-
allKeys = allKeys.subList(Math.max(0, offset), Math.min(offset + rows, allKeys.size()));
109+
int offsetToUse = Math.max(0, offset);
110+
if (rows > 0) {
111+
allKeys = allKeys.subList(Math.max(0, offsetToUse), Math.min(offsetToUse + rows, allKeys.size()));
104112
}
105113
for (byte[] id : allKeys) {
106114

@@ -171,8 +179,8 @@ private byte[][] keys(String prefix, Collection<PathAndValue> source) {
171179
int i = 0;
172180
for (PathAndValue pathAndValue : source) {
173181

174-
byte[] convertedValue = getAdapter().getConverter().getConversionService()
175-
.convert(pathAndValue.getFirstValue(), byte[].class);
182+
byte[] convertedValue = getAdapter().getConverter().getConversionService().convert(pathAndValue.getFirstValue(),
183+
byte[].class);
176184
byte[] fullPath = getAdapter().getConverter().getConversionService()
177185
.convert(prefix + pathAndValue.getPath() + ":", byte[].class);
178186

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015 the original author or authors.
2+
* Copyright 2015-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@
2323
import org.springframework.data.repository.query.parser.AbstractQueryCreator;
2424
import org.springframework.data.repository.query.parser.Part;
2525
import org.springframework.data.repository.query.parser.PartTree;
26+
import org.springframework.util.CollectionUtils;
2627

2728
/**
2829
* Redis specific query creator.
@@ -87,11 +88,13 @@ protected KeyValueQuery<RedisOperationChain> complete(final RedisOperationChain
8788

8889
KeyValueQuery<RedisOperationChain> query = new KeyValueQuery<RedisOperationChain>(criteria);
8990

90-
if (query.getCritieria().getSismember().size() == 1 && query.getCritieria().getOrSismember().size() == 1) {
91+
if (query.getCritieria() != null && !CollectionUtils.isEmpty(query.getCritieria().getSismember())
92+
&& !CollectionUtils.isEmpty(query.getCritieria().getOrSismember()))
93+
if (query.getCritieria().getSismember().size() == 1 && query.getCritieria().getOrSismember().size() == 1) {
9194

92-
query.getCritieria().getOrSismember().add(query.getCritieria().getSismember().iterator().next());
93-
query.getCritieria().getSismember().clear();
94-
}
95+
query.getCritieria().getOrSismember().add(query.getCritieria().getSismember().iterator().next());
96+
query.getCritieria().getSismember().clear();
97+
}
9598

9699
if (sort != null) {
97100
query.setSort(sort);

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

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,9 @@
1515
*/
1616
package org.springframework.data.redis.repository;
1717

18+
import static org.hamcrest.CoreMatchers.*;
1819
import static org.hamcrest.collection.IsCollectionWithSize.*;
1920
import static org.hamcrest.collection.IsIterableContainingInAnyOrder.*;
20-
import static org.hamcrest.core.Is.*;
21-
import static org.hamcrest.core.IsCollectionContaining.*;
2221
import static org.junit.Assert.*;
2322

2423
import java.io.Serializable;
@@ -42,7 +41,7 @@
4241
import org.springframework.data.redis.core.index.IndexDefinition;
4342
import org.springframework.data.redis.core.index.Indexed;
4443
import org.springframework.data.redis.core.index.SimpleIndexDefinition;
45-
import org.springframework.data.repository.CrudRepository;
44+
import org.springframework.data.repository.PagingAndSortingRepository;
4645

4746
/**
4847
* Base for testing Redis repository support in different configurations.
@@ -190,7 +189,43 @@ public void findUsingOrReturnsResultCorrectly() {
190189
assertThat(eddardAndJon, containsInAnyOrder(eddard, jon));
191190
}
192191

193-
public static interface PersonRepository extends CrudRepository<Person, String> {
192+
/**
193+
* @see DATAREDIS-547
194+
*/
195+
@Test
196+
public void shouldApplyPageableCorrectlyWhenUsingFindAll() {
197+
198+
Person eddard = new Person("eddard", "stark");
199+
Person robb = new Person("robb", "stark");
200+
Person jon = new Person("jon", "snow");
201+
202+
repo.save(Arrays.asList(eddard, robb, jon));
203+
204+
Page<Person> firstPage = repo.findAll(new PageRequest(0, 2));
205+
assertThat(firstPage.getContent(), hasSize(2));
206+
assertThat(repo.findAll(firstPage.nextPageable()).getContent(), hasSize(1));
207+
}
208+
209+
/**
210+
* @see DATAREDIS-547
211+
*/
212+
@Test
213+
public void shouldApplyReturnResultsCorrectlyWhenNoCriteriaPresent() {
214+
215+
Person eddard = new Person("eddard", "stark");
216+
Person tyrion = new Person("tyrion", "lannister");
217+
Person robb = new Person("robb", "stark");
218+
Person jon = new Person("jon", "snow");
219+
Person arya = new Person("arya", "stark");
220+
221+
repo.save(Arrays.asList(eddard, tyrion, robb, jon, arya));
222+
223+
List<Person> result = repo.findBy();
224+
225+
assertThat(result, hasSize(5));
226+
}
227+
228+
public static interface PersonRepository extends PagingAndSortingRepository<Person, String> {
194229

195230
List<Person> findByFirstname(String firstname);
196231

@@ -201,6 +236,8 @@ public static interface PersonRepository extends CrudRepository<Person, String>
201236
List<Person> findByFirstnameAndLastname(String firstname, String lastname);
202237

203238
List<Person> findByFirstnameOrLastname(String firstname, String lastname);
239+
240+
List<Person> findBy();
204241
}
205242

206243
/**

0 commit comments

Comments
 (0)