From 640709ede6d7fca5a4cb575ec90b38b1703cdbe2 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 14 Feb 2018 14:20:10 +0100 Subject: [PATCH] DATACASS-525 - Throw IncorrectResultSizeDataAccessException for repository query methods returning a single element. We now throw IncorrectResultSizeDataAccessException if a single entity query yields more result rows. Previously we gracefully returned the first element of the query without hinting whether the result was unique or not. --- .../query/AbstractCassandraQuery.java | 10 ++++++- .../query/AbstractReactiveCassandraQuery.java | 10 ++++++- .../query/CassandraQueryExecution.java | 18 ++++++++++- .../query/PartTreeCassandraQuery.java | 8 +++++ .../ReactiveCassandraQueryExecution.java | 16 +++++++++- .../query/ReactivePartTreeCassandraQuery.java | 8 +++++ .../ReactiveStringBasedCassandraQuery.java | 8 +++++ .../query/StringBasedCassandraQuery.java | 8 +++++ .../QueryDerivationIntegrationTests.java | 30 +++++++++++++++---- ...veCassandraRepositoryIntegrationTests.java | 19 ++++++++++++ .../reference/cassandra-repositories.adoc | 12 +++++--- .../reactive-cassandra-repositories.adoc | 16 +++++----- 12 files changed, 143 insertions(+), 20 deletions(-) diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractCassandraQuery.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractCassandraQuery.java index d079ebbba..06709d58f 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractCassandraQuery.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractCassandraQuery.java @@ -124,7 +124,15 @@ public abstract class AbstractCassandraQuery extends CassandraRepositoryQuerySup } else if (getQueryMethod().isStreamQuery()) { return new StreamExecution(getOperations(), resultProcessing); } else { - return new SingleEntityExecution(getOperations()); + return new SingleEntityExecution(getOperations(), isLimiting()); } } + + /** + * Return whether the query has an explicit limit set. + * + * @return a boolean value indicating whether the query has an explicit limit set. + * @since 2.0.4 + */ + protected abstract boolean isLimiting(); } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractReactiveCassandraQuery.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractReactiveCassandraQuery.java index bd708555e..88a98bea5 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractReactiveCassandraQuery.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractReactiveCassandraQuery.java @@ -137,6 +137,14 @@ public abstract class AbstractReactiveCassandraQuery extends CassandraRepository /* (non-Javadoc) */ private ReactiveCassandraQueryExecution getExecutionToWrap() { return (getQueryMethod().isCollectionQuery() ? new CollectionExecution(getReactiveCassandraOperations()) - : new SingleEntityExecution(getReactiveCassandraOperations())); + : new SingleEntityExecution(getReactiveCassandraOperations(), isLimiting())); } + + /** + * Return whether the query has an explicit limit set. + * + * @return a boolean value indicating whether the query has an explicit limit set. + * @since 2.0.4 + */ + protected abstract boolean isLimiting(); } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/CassandraQueryExecution.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/CassandraQueryExecution.java index b765ba1be..633578d4b 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/CassandraQueryExecution.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/CassandraQueryExecution.java @@ -15,10 +15,13 @@ */ package org.springframework.data.cassandra.repository.query; +import java.util.List; + import lombok.NonNull; import lombok.RequiredArgsConstructor; import org.springframework.core.convert.converter.Converter; +import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.cassandra.core.CassandraOperations; import org.springframework.data.cassandra.core.mapping.CassandraPersistentEntity; import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty; @@ -122,13 +125,26 @@ interface CassandraQueryExecution { final class SingleEntityExecution implements CassandraQueryExecution { private final @NonNull CassandraOperations operations; + private final boolean limiting; /* (non-Javadoc) * @see org.springframework.data.cassandra.repository.query.CassandraQueryExecution#execute(java.lang.String, java.lang.Class) */ @Override + @SuppressWarnings("unchecked") public Object execute(Statement statement, Class type) { - return operations.selectOne(statement, type); + + List objects = operations.select(statement, (Class) type); + + if (objects.isEmpty()) { + return null; + } + + if (objects.size() == 1 || limiting) { + return objects.get(0); + } + + throw new IncorrectResultSizeDataAccessException(1, objects.size()); } } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/PartTreeCassandraQuery.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/PartTreeCassandraQuery.java index 26da892ba..78216ab00 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/PartTreeCassandraQuery.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/PartTreeCassandraQuery.java @@ -98,4 +98,12 @@ public class PartTreeCassandraQuery extends AbstractCassandraQuery { return getQueryStatementCreator().select(getStatementFactory(), getTree(), getMappingContext(), parameterAccessor); } + + /* (non-Javadoc) + * @see org.springframework.data.cassandra.repository.query.AbstractCassandraQuery#isLimiting() + */ + @Override + protected boolean isLimiting() { + return getTree().isLimiting(); + } } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactiveCassandraQueryExecution.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactiveCassandraQueryExecution.java index 68ead37f9..da9b4bfd0 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactiveCassandraQueryExecution.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactiveCassandraQueryExecution.java @@ -19,6 +19,7 @@ import lombok.NonNull; import lombok.RequiredArgsConstructor; import org.springframework.core.convert.converter.Converter; +import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.cassandra.core.ReactiveCassandraOperations; import org.springframework.data.cassandra.core.mapping.CassandraPersistentEntity; import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty; @@ -70,6 +71,7 @@ interface ReactiveCassandraQueryExecution { final class SingleEntityExecution implements ReactiveCassandraQueryExecution { private final @NonNull ReactiveCassandraOperations operations; + private final boolean limiting; /* * (non-Javadoc) @@ -77,7 +79,19 @@ interface ReactiveCassandraQueryExecution { */ @Override public Object execute(Statement statement, Class type) { - return operations.selectOne(statement, type); + + return operations.select(statement, type).buffer(2).map(objects -> { + + if (objects.isEmpty()) { + return null; + } + + if (objects.size() == 1 || limiting) { + return objects.get(0); + } + + throw new IncorrectResultSizeDataAccessException(1, objects.size()); + }); } } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactivePartTreeCassandraQuery.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactivePartTreeCassandraQuery.java index d1a826f17..9cbf85e7f 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactivePartTreeCassandraQuery.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactivePartTreeCassandraQuery.java @@ -97,4 +97,12 @@ public class ReactivePartTreeCassandraQuery extends AbstractReactiveCassandraQue protected Statement createQuery(CassandraParameterAccessor parameterAccessor) { return getQueryStatementCreator().select(getStatementFactory(), getTree(), getMappingContext(), parameterAccessor); } + + /* (non-Javadoc) + * @see org.springframework.data.cassandra.repository.query.AbstractReactiveCassandraQuery#isLimiting() + */ + @Override + protected boolean isLimiting() { + return getTree().isLimiting(); + } } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactiveStringBasedCassandraQuery.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactiveStringBasedCassandraQuery.java index 39f4f1fe9..ce454b5f6 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactiveStringBasedCassandraQuery.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactiveStringBasedCassandraQuery.java @@ -88,4 +88,12 @@ public class ReactiveStringBasedCassandraQuery extends AbstractReactiveCassandra public SimpleStatement createQuery(CassandraParameterAccessor parameterAccessor) { return getQueryStatementCreator().select(getStringBasedQuery(), parameterAccessor); } + + /* (non-Javadoc) + * @see org.springframework.data.cassandra.repository.query.AbstractReactiveCassandraQuery#isLimiting() + */ + @Override + protected boolean isLimiting() { + return false; + } } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/StringBasedCassandraQuery.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/StringBasedCassandraQuery.java index f7cb50950..33a38eae0 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/StringBasedCassandraQuery.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/StringBasedCassandraQuery.java @@ -84,4 +84,12 @@ public class StringBasedCassandraQuery extends AbstractCassandraQuery { public SimpleStatement createQuery(CassandraParameterAccessor parameterAccessor) { return getQueryStatementCreator().select(getStringBasedQuery(), parameterAccessor); } + + /* (non-Javadoc) + * @see org.springframework.data.cassandra.repository.query.AbstractCassandraQuery#isLimiting() + */ + @Override + protected boolean isLimiting() { + return false; + } } diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/QueryDerivationIntegrationTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/QueryDerivationIntegrationTests.java index f38086a4b..36540db5a 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/QueryDerivationIntegrationTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/QueryDerivationIntegrationTests.java @@ -15,8 +15,8 @@ */ package org.springframework.data.cassandra.repository; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assume.assumeTrue; +import static org.assertj.core.api.Assertions.*; +import static org.junit.Assume.*; import java.time.LocalDate; import java.util.ArrayList; @@ -26,14 +26,15 @@ import java.util.Collections; import java.util.List; import java.util.Set; +import org.assertj.core.api.Assertions; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; - import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.ComponentScan.Filter; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.FilterType; +import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.cassandra.config.SchemaAction; import org.springframework.data.cassandra.core.CassandraOperations; import org.springframework.data.cassandra.core.cql.generator.CreateIndexCqlGenerator; @@ -52,11 +53,10 @@ import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; import org.springframework.data.domain.Sort; import org.springframework.data.util.Version; +import org.springframework.lang.Nullable; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; -import org.assertj.core.api.Assertions; - import com.datastax.driver.core.Session; /** @@ -122,6 +122,21 @@ public class QueryDerivationIntegrationTests extends AbstractSpringDataEmbeddedC assertThat(result).contains(walter, skyler, flynn); } + @Test(expected = IncorrectResultSizeDataAccessException.class) // DATACASS-525 + public void findOneWithManyResultsShouldFail() { + personRepository.findSomeByLastname("White"); + } + + @Test // DATACASS-525 + public void findOneWithNoResultsShouldReturnNull() { + assertThat(personRepository.findSomeByLastname("Foo")).isNull(); + } + + @Test // DATACASS-525 + public void findFirstWithManyResultsShouldReturnResult() { + assertThat(personRepository.findFirstByLastname("White")).isNotNull(); + } + @Test // DATACASS-7 public void shouldFindByLastnameAndDynamicSort() { @@ -327,6 +342,11 @@ public class QueryDerivationIntegrationTests extends AbstractSpringDataEmbeddedC List findByLastname(String lastname); + @Nullable + Person findSomeByLastname(String lastname); + + Person findFirstByLastname(String lastname); + List findByLastname(String lastname, Sort sort); List findByLastnameOrderByFirstnameAsc(String lastname); diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/ReactiveCassandraRepositoryIntegrationTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/ReactiveCassandraRepositoryIntegrationTests.java index cdebf1ca0..d6ed54173 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/ReactiveCassandraRepositoryIntegrationTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/ReactiveCassandraRepositoryIntegrationTests.java @@ -33,6 +33,7 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Configuration; +import org.springframework.dao.IncorrectResultSizeDataAccessException; import org.springframework.data.cassandra.core.ReactiveCassandraOperations; import org.springframework.data.cassandra.domain.Group; import org.springframework.data.cassandra.domain.GroupKey; @@ -127,6 +128,22 @@ public class ReactiveCassandraRepositoryIntegrationTests extends AbstractKeyspac StepVerifier.create(repository.findByLastname(dave.getLastname())).expectNextCount(2).verifyComplete(); } + @Test // DATACASS-525 + public void findOneWithManyResultsShouldFail() { + StepVerifier.create(repository.findOneByLastname(dave.getLastname())) + .expectError(IncorrectResultSizeDataAccessException.class).verify(); + } + + @Test // DATACASS-525 + public void findOneWithNoResultsShouldNotEmitItem() { + StepVerifier.create(repository.findByLastname("foo")).verifyComplete(); + } + + @Test // DATACASS-525 + public void findFirstWithManyResultsShouldEmitFirstItem() { + StepVerifier.create(repository.findFirstByLastname(dave.getLastname())).expectNextCount(1).verifyComplete(); + } + @Test // DATACASS-335 public void shouldFindByIdByLastName() { StepVerifier.create(repository.findOneByLastname(carter.getLastname())).expectNext(carter).verifyComplete(); @@ -168,6 +185,8 @@ public class ReactiveCassandraRepositoryIntegrationTests extends AbstractKeyspac Flux findByLastname(String lastname); + Mono findFirstByLastname(String lastname); + Mono findOneByLastname(String lastname); Mono findByLastname(Publisher lastname); diff --git a/src/main/asciidoc/reference/cassandra-repositories.adoc b/src/main/asciidoc/reference/cassandra-repositories.adoc index 53c3ce786..09b06b77f 100644 --- a/src/main/asciidoc/reference/cassandra-repositories.adoc +++ b/src/main/asciidoc/reference/cassandra-repositories.adoc @@ -186,7 +186,9 @@ public interface PersonRepository extends CrudRepository { Person findByShippingAddress(Address address); <5> - Stream findAllBy(); <6> + Person findFirstByShippingAddress(Address address); <6> + + Stream findAllBy(); <7> } ---- <1> The method shows a query for all people with the given `lastname`. The query will be derived from parsing @@ -196,9 +198,11 @@ a query expression of `SELECT * from person WHERE lastname = 'lastname'`. <3> Passing a `QueryOptions` object will apply the query options to the resulting query before it's execution. <4> Applies dynamic sorting to a query. Just add a `Sort` parameter to your method signature and Spring Data will automatically apply ordering to the query accordingly. -<5> Shows that you can query based on properties which are not a primitive type using registered `Converter`'s -in `CustomConversions`. -<6> Uses a Java 8 `Stream` which reads and converts individual elements while iterating the stream. +<5> Shows that you can query based on properties which are not a primitive type using registered ``Converter``'s +in `CustomConversions`. Throws `IncorrectResultSizeDataAccessException` if more than one match found. +<6> Uses the `First` keyword to restrict the query to the very first result. Unlike 5, this method does +not throw an exception if more than one match was found. +<7> Uses a Java 8 `Stream` which reads and converts individual elements while iterating the stream. ==== NOTE: Querying non-primary key properties requires secondary indexes. diff --git a/src/main/asciidoc/reference/reactive-cassandra-repositories.adoc b/src/main/asciidoc/reference/reactive-cassandra-repositories.adoc index 7451d8832..1a868d170 100644 --- a/src/main/asciidoc/reference/reactive-cassandra-repositories.adoc +++ b/src/main/asciidoc/reference/reactive-cassandra-repositories.adoc @@ -76,16 +76,19 @@ regards properties named "id" as the row id. ---- public interface ReactivePersonRepository extends ReactiveSortingRepository { - @AllowFiltering - Flux findByFirstname(String firstname); + Flux findByFirstname(String firstname); <1> - @AllowFiltering - Flux findByFirstname(Publisher firstname); + Flux findByFirstname(Publisher firstname); <2> - @AllowFiltering - Mono findByFirstnameAndLastname(String firstname, String lastname); + Mono findByFirstnameAndLastname(String firstname, String lastname); <3> + + Mono findFirstByFirstname(String firstname); <4> } ---- +<1> The method shows a query for all people with the given firstname. The query will be derived parsing the method name for constraints which can be concatenated with And and Or. Thus the method name will result in a query expression of `SELECT * FROM person WHERE firstname = :firstname`. +<2> The method shows a query for all people with the given firstname once the firstname is emitted via the given `Publisher`. +<3> Find a single entity for given criteria. Completes with `IncorrectResultSizeDataAccessException` on non unique results. +<4> Unlike 3, the first entity is always emitted even if the query yields more result rows. ==== For JavaConfig, use the `@EnableReactiveCassandraRepositories` annotation. The annotation carries the very same attributes @@ -144,4 +147,3 @@ The following features are supported: * <> NOTE: Query methods must return a reactive type. Resolved types (`User` vs. `Mono`) are not supported. -