From 323cb3286165fd62aca30c7ad408729d931a520a Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 10 Jan 2018 13:32:54 +0100 Subject: [PATCH] DATAREDIS-605 - Polishing Some minor code and documentation changes. More interestingly obtain the RedisConverter instead of the KeyValueAdapter for IndexResolver resolution in QueryByExampleRedisExecutor and stick to findOne contract by throwing IncorrectResultSizeDataAccessException for queries returning non unique results. Original Pull Request: #301 --- src/main/asciidoc/new-features.adoc | 2 +- .../redis/core/RedisKeyValueTemplate.java | 16 +++---- .../support/QueryByExampleRedisExecutor.java | 44 +++++++++---------- .../query/ExampleQueryMapperUnitTests.java | 8 ++-- .../QueryByExampleRedisExecutorTests.java | 15 +++++++ 5 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/main/asciidoc/new-features.adoc b/src/main/asciidoc/new-features.adoc index ff354c94b..4d5a59ab1 100644 --- a/src/main/asciidoc/new-features.adoc +++ b/src/main/asciidoc/new-features.adoc @@ -8,7 +8,7 @@ New and noteworthy in the latest releases. * Unix domain socket connections using <>. * <> support using Lettuce. -* <> integration +* <> integration. [[new-in-2.0.0]] == New in Spring Data Redis 2.0 diff --git a/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java b/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java index 9ad49a868..9b66b5e73 100644 --- a/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java +++ b/src/main/java/org/springframework/data/redis/core/RedisKeyValueTemplate.java @@ -22,6 +22,7 @@ import java.util.List; import org.springframework.data.keyvalue.core.KeyValueAdapter; import org.springframework.data.keyvalue.core.KeyValueCallback; import org.springframework.data.keyvalue.core.KeyValueTemplate; +import org.springframework.data.redis.core.convert.RedisConverter; import org.springframework.data.redis.core.mapping.RedisMappingContext; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -49,11 +50,13 @@ public class RedisKeyValueTemplate extends KeyValueTemplate { } /** - * @return the {@link RedisKeyValueAdapter}. + * Obtain the underlying redis specific {@link org.springframework.data.convert.EntityConverter}. + * + * @return never {@literal null}. * @since 2.1 */ - public RedisKeyValueAdapter getAdapter() { - return adapter; + public RedisConverter getConverter() { + return adapter.getConverter(); } /* @@ -65,7 +68,6 @@ public class RedisKeyValueTemplate extends KeyValueTemplate { return (RedisMappingContext) super.getMappingContext(); } - /** * Retrieve entities by resolving their {@literal id}s and converting them into required type.
* The callback provides either a single {@literal id} or an {@link Iterable} of {@literal id}s, used for retrieving @@ -104,15 +106,13 @@ public class RedisKeyValueTemplate extends KeyValueTemplate { } Iterable ids = ClassUtils.isAssignable(Iterable.class, callbackResult.getClass()) - ? (Iterable) callbackResult - : Collections.singleton(callbackResult); + ? (Iterable) callbackResult : Collections.singleton(callbackResult); List result = new ArrayList<>(); for (Object id : ids) { String idToUse = adapter.getConverter().getConversionService().canConvert(id.getClass(), String.class) - ? adapter.getConverter().getConversionService().convert(id, String.class) - : id.toString(); + ? adapter.getConverter().getConversionService().convert(id, String.class) : id.toString(); findById(idToUse, type).ifPresent(result::add); } diff --git a/src/main/java/org/springframework/data/redis/repository/support/QueryByExampleRedisExecutor.java b/src/main/java/org/springframework/data/redis/repository/support/QueryByExampleRedisExecutor.java index b42f2fd7e..04436b8a1 100644 --- a/src/main/java/org/springframework/data/redis/repository/support/QueryByExampleRedisExecutor.java +++ b/src/main/java/org/springframework/data/redis/repository/support/QueryByExampleRedisExecutor.java @@ -20,6 +20,7 @@ import java.util.Iterator; import java.util.List; import java.util.Optional; +import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.domain.Example; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; @@ -29,7 +30,6 @@ import org.springframework.data.keyvalue.core.query.KeyValueQuery; import org.springframework.data.redis.core.RedisKeyValueTemplate; import org.springframework.data.redis.core.convert.IndexResolver; import org.springframework.data.redis.core.convert.PathIndexResolver; -import org.springframework.data.redis.core.convert.RedisConverter; import org.springframework.data.redis.repository.query.ExampleQueryMapper; import org.springframework.data.redis.repository.query.RedisOperationChain; import org.springframework.data.repository.core.EntityInformation; @@ -43,6 +43,7 @@ import org.springframework.util.Assert; * methods. * * @author Mark Paluch + * @author Christoph Strobl * @since 2.1 */ @SuppressWarnings("unchecked") @@ -62,17 +63,9 @@ public class QueryByExampleRedisExecutor implements QueryByExampleExecutor public QueryByExampleRedisExecutor(EntityInformation entityInformation, RedisKeyValueTemplate keyValueTemplate) { - Assert.notNull(entityInformation, "EntityInformation must not be null!"); - Assert.notNull(keyValueTemplate, "RedisKeyValueTemplate must not be null!"); - - this.entityInformation = entityInformation; - this.keyValueTemplate = keyValueTemplate; - - RedisConverter converter = keyValueTemplate.getAdapter().getConverter(); - IndexResolver indexResolver = converter.getIndexResolver(); - - this.mapper = new ExampleQueryMapper(keyValueTemplate.getMappingContext(), - indexResolver != null ? indexResolver : new PathIndexResolver(converter.getMappingContext())); + this(entityInformation, keyValueTemplate, + keyValueTemplate.getConverter().getIndexResolver() != null ? keyValueTemplate.getConverter().getIndexResolver() + : new PathIndexResolver(keyValueTemplate.getMappingContext())); } /** @@ -91,8 +84,7 @@ public class QueryByExampleRedisExecutor implements QueryByExampleExecutor this.entityInformation = entityInformation; this.keyValueTemplate = keyValueTemplate; - this.mapper = new ExampleQueryMapper(keyValueTemplate.getMappingContext(), - new PathIndexResolver(keyValueTemplate.getMappingContext())); + this.mapper = new ExampleQueryMapper(keyValueTemplate.getMappingContext(), indexResolver); } /* @@ -102,13 +94,21 @@ public class QueryByExampleRedisExecutor implements QueryByExampleExecutor @Override public Optional findOne(Example example) { - RedisOperationChain operationChain = getQuery(example); + RedisOperationChain operationChain = createQuery(example); KeyValueQuery query = new KeyValueQuery<>(operationChain); - Iterable result = keyValueTemplate.find(query.limit(1), entityInformation.getJavaType()); - Iterator iterator = result.iterator(); + Iterator iterator = keyValueTemplate.find(query.limit(2), entityInformation.getJavaType()).iterator(); - return iterator.hasNext() ? Optional.of((S) iterator.next()) : Optional.empty(); + Optional result = Optional.empty(); + + if (iterator.hasNext()) { + result = Optional.of((S) iterator.next()); + if (iterator.hasNext()) { + throw new IncorrectResultSizeDataAccessException(1); + } + } + + return result; } /* @@ -118,7 +118,7 @@ public class QueryByExampleRedisExecutor implements QueryByExampleExecutor @Override public Iterable findAll(Example example) { - RedisOperationChain operationChain = getQuery(example); + RedisOperationChain operationChain = createQuery(example); return (Iterable) keyValueTemplate.find(new KeyValueQuery<>(operationChain), entityInformation.getJavaType()); } @@ -141,7 +141,7 @@ public class QueryByExampleRedisExecutor implements QueryByExampleExecutor Assert.notNull(pageable, "Pageable must not be null!"); - RedisOperationChain operationChain = getQuery(example); + RedisOperationChain operationChain = createQuery(example); KeyValueQuery query = new KeyValueQuery<>(operationChain); Iterable result = keyValueTemplate.find( @@ -166,7 +166,7 @@ public class QueryByExampleRedisExecutor implements QueryByExampleExecutor @Override public long count(Example example) { - RedisOperationChain operationChain = getQuery(example); + RedisOperationChain operationChain = createQuery(example); return keyValueTemplate.count(new KeyValueQuery<>(operationChain), entityInformation.getJavaType()); } @@ -180,7 +180,7 @@ public class QueryByExampleRedisExecutor implements QueryByExampleExecutor return count(example) > 0; } - private RedisOperationChain getQuery(Example example) { + private RedisOperationChain createQuery(Example example) { Assert.notNull(example, "Example must not be null!"); diff --git a/src/test/java/org/springframework/data/redis/repository/query/ExampleQueryMapperUnitTests.java b/src/test/java/org/springframework/data/redis/repository/query/ExampleQueryMapperUnitTests.java index 5cdb0f315..87ab3137a 100644 --- a/src/test/java/org/springframework/data/redis/repository/query/ExampleQueryMapperUnitTests.java +++ b/src/test/java/org/springframework/data/redis/repository/query/ExampleQueryMapperUnitTests.java @@ -40,6 +40,7 @@ import org.springframework.data.redis.repository.query.RedisOperationChain.PathA * Unit tests for {@link ExampleQueryMapper}. * * @author Mark Paluch + * @author Christoph Strobl */ public class ExampleQueryMapperUnitTests { @@ -186,7 +187,7 @@ public class ExampleQueryMapperUnitTests { } @Data - public static class Person { + static class Person { @Id String id; @@ -204,7 +205,8 @@ public class ExampleQueryMapperUnitTests { Species species; } - public enum Gender { + enum Gender { + MALE, FEMALE { @Override @@ -214,7 +216,7 @@ public class ExampleQueryMapperUnitTests { } } - public static class Species { + static class Species { @Indexed String name; } diff --git a/src/test/java/org/springframework/data/redis/repository/support/QueryByExampleRedisExecutorTests.java b/src/test/java/org/springframework/data/redis/repository/support/QueryByExampleRedisExecutorTests.java index 0a806c4fc..ce7472b15 100644 --- a/src/test/java/org/springframework/data/redis/repository/support/QueryByExampleRedisExecutorTests.java +++ b/src/test/java/org/springframework/data/redis/repository/support/QueryByExampleRedisExecutorTests.java @@ -27,6 +27,7 @@ import java.util.Optional; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.annotation.Id; import org.springframework.data.domain.Example; import org.springframework.data.domain.Page; @@ -48,6 +49,7 @@ import org.springframework.data.redis.repository.core.MappingRedisEntityInformat * Integration tests for {@link QueryByExampleRedisExecutor}. * * @author Mark Paluch + * @author Christoph Strobl */ public class QueryByExampleRedisExecutorTests { @@ -101,6 +103,19 @@ public class QueryByExampleRedisExecutorTests { assertThat(result).contains(walt); } + @Test // DATAREDIS-605 + public void shouldThrowExceptionWhenFindOneByExampleReturnsNonUniqueResult() { + + QueryByExampleRedisExecutor executor = new QueryByExampleRedisExecutor<>(getEntityInformation(Person.class), + kvTemplate); + + Person person = new Person(); + person.setHometown(walt.getHometown()); + + assertThatThrownBy(() -> executor.findOne(Example.of(person))) + .isInstanceOf(IncorrectResultSizeDataAccessException.class); + } + @Test // DATAREDIS-605 public void shouldNotFindOneByExample() {