DATAMONGO-1865 - Avoid IncorrectResultSizeDataAccessException for derived findFirst/findTop queries.

We now return the first result when executing findFirst/findTop queries. This fixes a glitch introduced in the Kay release throwing IncorrectResultSizeDataAccessException for single entity executions returning more than one result, which is explicitly not the desired behavior in this case.

Original pull request: #530.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractMongoQuery.java

@@ -17,6 +17,7 @@ package org.springframework.data.mongodb.repository.query;
 
 import org.springframework.data.mongodb.core.ExecutableFindOperation.ExecutableFind;
 import org.springframework.data.mongodb.core.ExecutableFindOperation.FindWithQuery;
+import org.springframework.data.mongodb.core.ExecutableFindOperation.TerminatingFind;
 import org.springframework.data.mongodb.core.MongoOperations;
 import org.springframework.data.mongodb.core.query.Query;
 import org.springframework.data.mongodb.repository.query.MongoQueryExecution.DeleteExecution;
@@ -117,7 +118,11 @@ public abstract class AbstractMongoQuery implements RepositoryQuery {
 		} else if (isExistsQuery()) {
 			return q -> operation.matching(q).exists();
 		} else {
-			return q -> operation.matching(q).oneValue();
+			return q -> {
+
+				TerminatingFind<?> find = operation.matching(q);
+				return isLimiting() ? find.firstValue() : find.oneValue();
+			};
 		}
 	}
 
@@ -172,4 +177,12 @@ public abstract class AbstractMongoQuery implements RepositoryQuery {
 	 * @since 1.5
 	 */
 	protected abstract boolean isDeleteQuery();
+
+	/**
+	 * Return weather the query has an explicit limit set.
+	 *
+	 * @return
+	 * @since 2.0.4
+	 */
+	protected abstract boolean isLimiting();
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AbstractReactiveMongoQuery.java

@@ -22,18 +22,20 @@ import org.reactivestreams.Publisher;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.convert.EntityInstantiators;
 import org.springframework.data.mongodb.core.MongoOperations;
+import org.springframework.data.mongodb.core.ReactiveFindOperation.FindWithProjection;
+import org.springframework.data.mongodb.core.ReactiveFindOperation.FindWithQuery;
+import org.springframework.data.mongodb.core.ReactiveFindOperation.TerminatingFind;
 import org.springframework.data.mongodb.core.ReactiveMongoOperations;
 import org.springframework.data.mongodb.core.query.Query;
-import org.springframework.data.mongodb.repository.query.ReactiveMongoQueryExecution.CollectionExecution;
 import org.springframework.data.mongodb.repository.query.ReactiveMongoQueryExecution.DeleteExecution;
 import org.springframework.data.mongodb.repository.query.ReactiveMongoQueryExecution.GeoNearExecution;
 import org.springframework.data.mongodb.repository.query.ReactiveMongoQueryExecution.ResultProcessingConverter;
 import org.springframework.data.mongodb.repository.query.ReactiveMongoQueryExecution.ResultProcessingExecution;
-import org.springframework.data.mongodb.repository.query.ReactiveMongoQueryExecution.SingleEntityExecution;
 import org.springframework.data.mongodb.repository.query.ReactiveMongoQueryExecution.TailExecution;
 import org.springframework.data.repository.query.ParameterAccessor;
 import org.springframework.data.repository.query.RepositoryQuery;
 import org.springframework.data.repository.query.ResultProcessor;
+import org.springframework.data.repository.query.ReturnedType;
 import org.springframework.util.Assert;
 
 /**
@@ -48,6 +50,7 @@ public abstract class AbstractReactiveMongoQuery implements RepositoryQuery {
 	private final ReactiveMongoQueryMethod method;
 	private final ReactiveMongoOperations operations;
 	private final EntityInstantiators instantiators;
+	private final FindWithProjection<?> findOperationWithProjection;
 
 	/**
 	 * Creates a new {@link AbstractReactiveMongoQuery} from the given {@link MongoQueryMethod} and
@@ -64,6 +67,12 @@ public abstract class AbstractReactiveMongoQuery implements RepositoryQuery {
 		this.method = method;
 		this.operations = operations;
 		this.instantiators = new EntityInstantiators();
+
+		ReturnedType returnedType = method.getResultProcessor().getReturnedType();
+
+		this.findOperationWithProjection = operations//
+				.query(returnedType.getDomainType())//
+				.inCollection(method.getEntityInformation().getCollectionName());
 	}
 
 	/*
@@ -103,10 +112,16 @@ public abstract class AbstractReactiveMongoQuery implements RepositoryQuery {
 		applyQueryMetaAttributesWhenPresent(query);
 
 		ResultProcessor processor = method.getResultProcessor().withDynamicProjection(parameterAccessor);
+		Class<?> typeToRead = processor.getReturnedType().getTypeToRead();
+
+		FindWithQuery<?> find = typeToRead == null //
+				? findOperationWithProjection //
+				: findOperationWithProjection.as(typeToRead);
+
 		String collection = method.getEntityInformation().getCollectionName();
 
 		ReactiveMongoQueryExecution execution = getExecution(query, parameterAccessor,
-				new ResultProcessingConverter(processor, operations, instantiators));
+				new ResultProcessingConverter(processor, operations, instantiators), find);
 
 		return execution.execute(query, processor.getReturnedType().getDomainType(), collection);
 	}
@@ -120,11 +135,11 @@ public abstract class AbstractReactiveMongoQuery implements RepositoryQuery {
 	 * @return
 	 */
 	private ReactiveMongoQueryExecution getExecution(Query query, MongoParameterAccessor accessor,
-			Converter<Object, Object> resultProcessing) {
-		return new ResultProcessingExecution(getExecutionToWrap(accessor), resultProcessing);
+			Converter<Object, Object> resultProcessing, FindWithQuery<?> operation) {
+		return new ResultProcessingExecution(getExecutionToWrap(accessor, operation), resultProcessing);
 	}
 
-	private ReactiveMongoQueryExecution getExecutionToWrap(MongoParameterAccessor accessor) {
+	private ReactiveMongoQueryExecution getExecutionToWrap(MongoParameterAccessor accessor, FindWithQuery<?> operation) {
 
 		if (isDeleteQuery()) {
 			return new DeleteExecution(operations, method);
@@ -133,9 +148,20 @@ public abstract class AbstractReactiveMongoQuery implements RepositoryQuery {
 		} else if (isTailable(method)) {
 			return new TailExecution(operations, accessor.getPageable());
 		} else if (method.isCollectionQuery()) {
-			return new CollectionExecution(operations, accessor.getPageable());
+			return (q, t, c) -> operation.matching(q.with(accessor.getPageable())).all();
+		} else if (isCountQuery()) {
+			return (q, t, c) -> operation.matching(q).count();
 		} else {
-			return new SingleEntityExecution(operations, isCountQuery());
+			return (q, t, c) -> {
+
+				TerminatingFind<?> find = operation.matching(q);
+
+				if (isCountQuery()) {
+					return find.count();
+				}
+
+				return isLimiting() ? find.first() : find.one();
+			};
 		}
 	}
 
@@ -186,4 +212,12 @@ public abstract class AbstractReactiveMongoQuery implements RepositoryQuery {
 	 * @since 1.5
 	 */
 	protected abstract boolean isDeleteQuery();
+
+	/**
+	 * Return weather the query has an explicit limit set.
+	 *
+	 * @return
+	 * @since 2.0.4
+	 */
+	protected abstract boolean isLimiting();
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/PartTreeMongoQuery.java

@@ -160,4 +160,13 @@ public class PartTreeMongoQuery extends AbstractMongoQuery {
 	protected boolean isDeleteQuery() {
 		return tree.isDelete();
 	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery#isLimiting()
+	 */
+	@Override
+	protected boolean isLimiting() {
+		return tree.isLimiting();
+	}
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveMongoQueryExecution.java

@@ -44,29 +44,13 @@ import com.mongodb.client.result.DeleteResult;
  * various flavors.
  *
  * @author Mark Paluch
+ * @author Christoph Strobl
  * @since 2.0
  */
 interface ReactiveMongoQueryExecution {
 
 	Object execute(Query query, Class<?> type, String collection);
 
-	/**
-	 * {@link ReactiveMongoQueryExecution} for collection returning queries.
-	 *
-	 * @author Mark Paluch
-	 */
-	@RequiredArgsConstructor
-	final class CollectionExecution implements ReactiveMongoQueryExecution {
-
-		private final @NonNull ReactiveMongoOperations operations;
-		private final Pageable pageable;
-
-		@Override
-		public Object execute(Query query, Class<?> type, String collection) {
-			return operations.find(query.with(pageable), type, collection);
-		}
-	}
-
 	/**
 	 * {@link ReactiveMongoQueryExecution} for collection returning queries using tailable cursors.
 	 *
@@ -84,23 +68,6 @@ interface ReactiveMongoQueryExecution {
 		}
 	}
 
-	/**
-	 * {@link ReactiveMongoQueryExecution} to return a single entity.
-	 *
-	 * @author Mark Paluch
-	 */
-	@RequiredArgsConstructor
-	final class SingleEntityExecution implements ReactiveMongoQueryExecution {
-
-		private final ReactiveMongoOperations operations;
-		private final boolean countProjection;
-
-		@Override
-		public Object execute(Query query, Class<?> type, String collection) {
-			return countProjection ? operations.count(query, type, collection) : operations.findOne(query, type, collection);
-		}
-	}
-
 	/**
 	 * {@link MongoQueryExecution} to execute geo-near queries.
 	 *

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactivePartTreeMongoQuery.java

@@ -118,7 +118,7 @@ public class ReactivePartTreeMongoQuery extends AbstractReactiveMongoQuery {
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery#createCountQuery(org.springframework.data.mongodb.repository.query.ConvertingParameterAccessor)
+	 * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#createCountQuery(org.springframework.data.mongodb.repository.query.ConvertingParameterAccessor)
 	 */
 	@Override
 	protected Query createCountQuery(ConvertingParameterAccessor accessor) {
@@ -127,7 +127,7 @@ public class ReactivePartTreeMongoQuery extends AbstractReactiveMongoQuery {
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery#isCountQuery()
+	 * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#isCountQuery()
 	 */
 	@Override
 	protected boolean isCountQuery() {
@@ -136,10 +136,19 @@ public class ReactivePartTreeMongoQuery extends AbstractReactiveMongoQuery {
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery#isDeleteQuery()
+	 * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#isDeleteQuery()
 	 */
 	@Override
 	protected boolean isDeleteQuery() {
 		return tree.isDelete();
 	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#isLimiting()
+	 */
+	@Override
+	protected boolean isLimiting() {
+		return tree.isLimiting();
+	}
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedMongoQuery.java

@@ -104,7 +104,7 @@ public class ReactiveStringBasedMongoQuery extends AbstractReactiveMongoQuery {
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery#createQuery(org.springframework.data.mongodb.repository.query.ConvertingParameterAccessor)
+	 * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#createQuery(org.springframework.data.mongodb.repository.query.ConvertingParameterAccessor)
 	 */
 	@Override
 	protected Query createQuery(ConvertingParameterAccessor accessor) {
@@ -125,7 +125,7 @@ public class ReactiveStringBasedMongoQuery extends AbstractReactiveMongoQuery {
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery#isCountQuery()
+	 * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#isCountQuery()
 	 */
 	@Override
 	protected boolean isCountQuery() {
@@ -134,11 +134,20 @@ public class ReactiveStringBasedMongoQuery extends AbstractReactiveMongoQuery {
 
 	/*
 	 * (non-Javadoc)
-	 * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery#isDeleteQuery()
+	 * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#isDeleteQuery()
 	 */
 	@Override
 	protected boolean isDeleteQuery() {
 		return this.isDeleteQuery;
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mongodb.repository.query.AbstractReactiveMongoQuery#isLimiting()
+	 */
+	@Override
+	protected boolean isLimiting() {
+		return false;
+	}
+
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQuery.java

@@ -174,6 +174,15 @@ public class StringBasedMongoQuery extends AbstractMongoQuery {
 		return countBooleanValues(isCountQuery, isExistsQuery, isDeleteQuery) > 1;
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mongodb.repository.query.AbstractMongoQuery#isLimiting()
+	 */
+	@Override
+	protected boolean isLimiting() {
+		return false;
+	}
+
 	private static int countBooleanValues(boolean... values) {
 
 		int count = 0;

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java

@@ -38,6 +38,7 @@ import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.dao.DuplicateKeyException;
+import org.springframework.dao.IncorrectResultSizeDataAccessException;
 import org.springframework.data.domain.Example;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
@@ -1177,4 +1178,19 @@ public abstract class AbstractPersonRepositoryIntegrationTests {
 	public void readsClosedProjection() {
 		assertThat(repository.findClosedProjectionBy()).isNotEmpty();
 	}
+
+	@Test // DATAMONGO-1865
+	public void findFirstEntityReturnsFirstResultEvenForNonUniqueMatches() {
+		repository.findFirstBy();
+	}
+
+	@Test(expected = IncorrectResultSizeDataAccessException.class) // DATAMONGO-1865
+	public void findSingleEntityThrowsErrorWhenNotUnique() {
+		repository.findPersonByLastnameLike(dave.getLastname());
+	}
+
+	@Test(expected = IncorrectResultSizeDataAccessException.class) // DATAMONGO-1865
+	public void findOptionalSingleEntityThrowsErrorWhenNotUnique() {
+		repository.findOptionalPersonByLastnameLike(dave.getLastname());
+	}
 }

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java

@@ -18,6 +18,7 @@ package org.springframework.data.mongodb.repository;
 import java.util.Collection;
 import java.util.Date;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Stream;
 
 import org.springframework.data.domain.Page;
@@ -286,6 +287,15 @@ public interface PersonRepository extends MongoRepository<Person, String>, Query
 	// DATAMONGO-950
 	Page<Person> findTop3ByLastnameStartingWith(String lastname, Pageable pageRequest);
 
+	// DATAMONGO-1865
+	Person findFirstBy(); // limits to 1 result if more, just return the first one
+
+	// DATAMONGO-1865
+	Person findPersonByLastnameLike(String firstname); // single person, error if more than one
+
+	// DATAMONGO-1865
+	Optional<Person> findOptionalPersonByLastnameLike(String firstname); // optional still, error when more than one
+
 	// DATAMONGO-1030
 	PersonSummaryDto findSummaryByLastname(String lastname);
 

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/ReactiveMongoRepositoryTests.java

@@ -39,6 +39,7 @@ import org.springframework.beans.factory.BeanClassLoaderAware;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.dao.IncorrectResultSizeDataAccessException;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
@@ -300,6 +301,17 @@ public class ReactiveMongoRepositoryTests implements BeanClassLoaderAware, BeanF
 				.verifyComplete();
 	}
 
+	@Test // DATAMONGO-1865
+	public void shouldErrorOnFindOneWithNonUniqueResult() {
+		StepVerifier.create(repository.findOneByLastname(dave.getLastname()))
+				.expectError(IncorrectResultSizeDataAccessException.class).verify();
+	}
+
+	@Test // DATAMONGO-1865
+	public void shouldReturnFirstFindFirstWithMoreResults() {
+		StepVerifier.create(repository.findFirstByLastname(dave.getLastname())).expectNextCount(1).verifyComplete();
+	}
+
 	interface ReactivePersonRepository extends ReactiveMongoRepository<Person, String> {
 
 		Flux<Person> findByLastname(String lastname);
@@ -326,6 +338,8 @@ public class ReactiveMongoRepositoryTests implements BeanClassLoaderAware, BeanF
 		Flux<GeoResult<Person>> findByLocationNear(Point point, Distance maxDistance, Pageable pageable);
 
 		Flux<Person> findPersonByLocationNear(Point point, Distance maxDistance);
+
+		Mono<Person> findFirstByLastname(String lastname);
 	}
 
 	interface ReactiveCappedCollectionRepository extends Repository<Capped, String> {

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/AbstractMongoQueryUnitTests.java

@@ -261,6 +261,18 @@ public class AbstractMongoQueryUnitTests {
 		assertThat(query.execute(new Object[] { "lastname" }), is(reference));
 	}
 
+	@Test // DATAMONGO-1865
+	public void limitingSingleEntityQueryCallsFirst() {
+
+		Person reference = new Person();
+
+		doReturn(reference).when(withQueryMock).firstValue();
+
+		AbstractMongoQuery query = createQueryForMethod("findFirstByLastname", String.class).setLimitingQuery(true);
+
+		assertThat(query.execute(new Object[] { "lastname" }), is(reference));
+	}
+
 	@Test // DATAMONGO-1872
 	public void doesNotFixCollectionOnPreparation() {
 
@@ -294,6 +306,7 @@ public class AbstractMongoQueryUnitTests {
 	private static class MongoQueryFake extends AbstractMongoQuery {
 
 		private boolean isDeleteQuery;
+		private boolean isLimitingQuery;
 
 		public MongoQueryFake(MongoQueryMethod method, MongoOperations operations) {
 			super(method, operations);
@@ -319,10 +332,21 @@ public class AbstractMongoQueryUnitTests {
 			return isDeleteQuery;
 		}
 
+		@Override
+		protected boolean isLimiting() {
+			return isLimitingQuery;
+		}
+
 		public MongoQueryFake setDeleteQuery(boolean isDeleteQuery) {
 			this.isDeleteQuery = isDeleteQuery;
 			return this;
 		}
+
+		public MongoQueryFake setLimitingQuery(boolean limitingQuery) {
+
+			isLimitingQuery = limitingQuery;
+			return this;
+		}
 	}
 
 	private interface Repo extends MongoRepository<Person, Long> {
@@ -344,6 +368,8 @@ public class AbstractMongoQueryUnitTests {
 		Slice<Person> findByLastname(String lastname, Pageable page);
 
 		Optional<Person> findByLastname(String lastname);
+
+		Person findFirstByLastname(String lastname);
 	}
 
 	// DATAMONGO-1872

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/PartTreeMongoQueryUnitTests.java

@@ -185,6 +185,16 @@ public class PartTreeMongoQueryUnitTests {
 		assertThat(query.getFieldsObject(), is(new Document()));
 	}
 
+	@Test // DATAMONGO-1865
+	public void limitingReturnsTrueIfTreeIsLimiting() {
+		assertThat(createQueryForMethod("findFirstBy").isLimiting(), is(true));
+	}
+
+	@Test // DATAMONGO-1865
+	public void limitingReturnsFalseIfTreeIsNotLimiting() {
+		assertThat(createQueryForMethod("findPersonBy").isLimiting(), is(false));
+	}
+
 	private org.springframework.data.mongodb.core.query.Query deriveQueryFromMethod(String method, Object... args) {
 
 		Class<?>[] types = new Class<?>[args.length];
@@ -245,6 +255,8 @@ public class PartTreeMongoQueryUnitTests {
 		List<Person> findBySex(Sex sex);
 
 		OpenProjection findAllBy();
+
+		Person findFirstBy();
 	}
 
 	interface PersonProjection {

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedMongoQueryUnitTests.java

@@ -18,6 +18,7 @@ package org.springframework.data.mongodb.repository.query;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.any;
 
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
@@ -35,6 +36,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.data.mongodb.core.ReactiveFindOperation.ReactiveFind;
 import org.springframework.data.mongodb.core.ReactiveMongoOperations;
 import org.springframework.data.mongodb.core.convert.DbRefResolver;
 import org.springframework.data.mongodb.core.convert.DefaultMongoTypeMapper;
@@ -56,6 +58,7 @@ import org.springframework.expression.spel.standard.SpelExpressionParser;
  * Unit tests for {@link ReactiveStringBasedMongoQuery}.
  *
  * @author Mark Paluch
+ * @author Christoph Strobl
  */
 @RunWith(MockitoJUnitRunner.class)
 public class ReactiveStringBasedMongoQueryUnitTests {
@@ -64,11 +67,16 @@ public class ReactiveStringBasedMongoQueryUnitTests {
 
 	@Mock ReactiveMongoOperations operations;
 	@Mock DbRefResolver factory;
+	@Mock ReactiveFind reactiveFind;
 
 	MongoConverter converter;
 
 	@Before
 	public void setUp() {
+
+		when(operations.query(any())).thenReturn(reactiveFind);
+		when(reactiveFind.inCollection(anyString())).thenReturn(reactiveFind);
+
 		this.converter = new MappingMongoConverter(factory, new MongoMappingContext());
 	}