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.
This commit is contained in:
Mark Paluch
2018-02-14 14:20:10 +01:00
parent 8efa1bc7a1
commit 847deccccc
12 changed files with 148 additions and 30 deletions

View File

@@ -143,11 +143,11 @@ public abstract class AbstractCassandraQuery extends CassandraRepositoryQuerySup
} else if (getQueryMethod().isStreamQuery()) {
return new StreamExecution(getOperations(), resultProcessing);
} else if (isCountQuery()) {
return ((statement, type) -> new SingleEntityExecution(getOperations()).execute(statement, Long.class));
return ((statement, type) -> new SingleEntityExecution(getOperations(), false).execute(statement, Long.class));
} else if (isExistsQuery()) {
return new ExistsExecution(getOperations());
} else {
return new SingleEntityExecution(getOperations());
return new SingleEntityExecution(getOperations(), isLimiting());
}
}
@@ -167,4 +167,11 @@ public abstract class AbstractCassandraQuery extends CassandraRepositoryQuerySup
*/
protected abstract boolean isExistsQuery();
/**
* 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();
}

View File

@@ -156,11 +156,11 @@ public abstract class AbstractReactiveCassandraQuery extends CassandraRepository
return new CollectionExecution(getReactiveCassandraOperations());
} else if (isCountQuery()) {
return ((statement, type) ->
new SingleEntityExecution(getReactiveCassandraOperations()).execute(statement, Long.class));
new SingleEntityExecution(getReactiveCassandraOperations(), false).execute(statement, Long.class));
} else if (isExistsQuery()) {
return new ExistsExecution(getReactiveCassandraOperations());
} else {
return new SingleEntityExecution(getReactiveCassandraOperations());
return new SingleEntityExecution(getReactiveCassandraOperations(), isLimiting());
}
}
@@ -180,4 +180,11 @@ public abstract class AbstractReactiveCassandraQuery extends CassandraRepository
*/
protected abstract boolean isExistsQuery();
/**
* 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();
}

View File

@@ -19,8 +19,10 @@ import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import java.util.Iterator;
import java.util.List;
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;
@@ -126,13 +128,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<Object> 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());
}
}

View File

@@ -122,4 +122,12 @@ public class PartTreeCassandraQuery extends AbstractCassandraQuery {
protected boolean isExistsQuery() {
return getTree().isExistsProjection();
}
/* (non-Javadoc)
* @see org.springframework.data.cassandra.repository.query.AbstractCassandraQuery#isLimiting()
*/
@Override
protected boolean isLimiting() {
return getTree().isLimiting();
}
}

View File

@@ -15,14 +15,14 @@
*/
package org.springframework.data.cassandra.repository.query;
import java.util.List;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import reactor.core.publisher.Mono;
import java.util.List;
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;
@@ -75,6 +75,7 @@ interface ReactiveCassandraQueryExecution {
final class SingleEntityExecution implements ReactiveCassandraQueryExecution {
private final @NonNull ReactiveCassandraOperations operations;
private final boolean limiting;
/*
* (non-Javadoc)
@@ -82,7 +83,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());
});
}
}

View File

@@ -122,4 +122,12 @@ public class ReactivePartTreeCassandraQuery extends AbstractReactiveCassandraQue
protected boolean isExistsQuery() {
return getTree().isExistsProjection();
}
/* (non-Javadoc)
* @see org.springframework.data.cassandra.repository.query.AbstractReactiveCassandraQuery#isLimiting()
*/
@Override
protected boolean isLimiting() {
return getTree().isLimiting();
}
}

View File

@@ -127,4 +127,12 @@ public class ReactiveStringBasedCassandraQuery extends AbstractReactiveCassandra
protected boolean isExistsQuery() {
return this.isExistsQuery;
}
/* (non-Javadoc)
* @see org.springframework.data.cassandra.repository.query.AbstractReactiveCassandraQuery#isLimiting()
*/
@Override
protected boolean isLimiting() {
return false;
}
}

View File

@@ -123,4 +123,12 @@ public class StringBasedCassandraQuery extends AbstractCassandraQuery {
protected boolean isExistsQuery() {
return this.isExistsQuery;
}
/* (non-Javadoc)
* @see org.springframework.data.cassandra.repository.query.AbstractCassandraQuery#isLimiting()
*/
@Override
protected boolean isLimiting() {
return false;
}
}

View File

@@ -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() {
@@ -342,6 +357,11 @@ public class QueryDerivationIntegrationTests extends AbstractSpringDataEmbeddedC
List<Person> findByLastname(String lastname);
@Nullable
Person findSomeByLastname(String lastname);
Person findFirstByLastname(String lastname);
List<Person> findByLastname(String lastname, Sort sort);
List<Person> findByLastnameOrderByFirstnameAsc(String lastname);

View File

@@ -15,18 +15,17 @@
*/
package org.springframework.data.cassandra.repository;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.reactivestreams.Publisher;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanClassLoaderAware;
@@ -34,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;
@@ -128,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();
@@ -189,6 +205,8 @@ public class ReactiveCassandraRepositoryIntegrationTests extends AbstractKeyspac
Flux<User> findByLastname(String lastname);
Mono<User> findFirstByLastname(String lastname);
Mono<User> findOneByLastname(String lastname);
Mono<User> findByLastname(Publisher<String> lastname);

View File

@@ -186,7 +186,9 @@ public interface PersonRepository extends CrudRepository<Person, String> {
Person findByShippingAddress(Address address); <5>
Stream<Person> findAllBy(); <6>
Person findFirstByShippingAddress(Address address); <6>
Stream<Person> 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.

View File

@@ -76,16 +76,19 @@ regards properties named "id" as the row id.
----
public interface ReactivePersonRepository extends ReactiveSortingRepository<Person, Long> {
@AllowFiltering
Flux<Person> findByFirstname(String firstname);
Flux<Person> findByFirstname(String firstname); <1>
@AllowFiltering
Flux<Person> findByFirstname(Publisher<String> firstname);
Flux<Person> findByFirstname(Publisher<String> firstname); <2>
@AllowFiltering
Mono<Person> findByFirstnameAndLastname(String firstname, String lastname);
Mono<Person> findByFirstnameAndLastname(String firstname, String lastname); <3>
Mono<Person> 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:
* <<projections>>
NOTE: Query methods must return a reactive type. Resolved types (`User` vs. `Mono<User>`) are not supported.