DATAMONGO-1872 - Repository query execution doesn't prematurely fix collection to be queried.

We now avoid calling ….inCollection(…) with a fixed, one-time calculated collection name to make sure we dynamically resolve the collections. That's necessary to make sure SpEL expressions in @Document are evaluated for every query execution.

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

@@ -15,7 +15,7 @@
  */
 package org.springframework.data.mongodb.repository.query;
 
-import org.springframework.data.mongodb.core.ExecutableFindOperation.FindWithProjection;
+import org.springframework.data.mongodb.core.ExecutableFindOperation.ExecutableFind;
 import org.springframework.data.mongodb.core.ExecutableFindOperation.FindWithQuery;
 import org.springframework.data.mongodb.core.MongoOperations;
 import org.springframework.data.mongodb.core.query.Query;
@@ -27,7 +27,6 @@ import org.springframework.data.mongodb.repository.query.MongoQueryExecution.Sli
 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;
 
 /**
@@ -42,7 +41,7 @@ public abstract class AbstractMongoQuery implements RepositoryQuery {
 
 	private final MongoQueryMethod method;
 	private final MongoOperations operations;
-	private final FindWithProjection<?> findOperationWithProjection;
+	private final ExecutableFind<?> executableFind;
 
 	/**
 	 * Creates a new {@link AbstractMongoQuery} from the given {@link MongoQueryMethod} and {@link MongoOperations}.
@@ -58,11 +57,10 @@ public abstract class AbstractMongoQuery implements RepositoryQuery {
 		this.method = method;
 		this.operations = operations;
 
-		ReturnedType returnedType = method.getResultProcessor().getReturnedType();
+		MongoEntityMetadata<?> metadata = method.getEntityInformation();
+		Class<?> type = metadata.getCollectionEntity().getType();
 
-		this.findOperationWithProjection = operations//
+		this.executableFind = operations.query(type);
-				.query(returnedType.getDomainType())//
-				.inCollection(method.getEntityInformation().getCollectionName());
 	}
 
 	/*
@@ -90,8 +88,8 @@ public abstract class AbstractMongoQuery implements RepositoryQuery {
 		Class<?> typeToRead = processor.getReturnedType().getTypeToRead();
 
 		FindWithQuery<?> find = typeToRead == null //
-				? findOperationWithProjection //
+				? executableFind //
-				: findOperationWithProjection.as(typeToRead);
+				: executableFind.as(typeToRead);
 
 		MongoQueryExecution execution = getExecution(accessor, find);
 

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

@@ -15,6 +15,7 @@
  */
 package org.springframework.data.mongodb.repository.query;
 
+import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
 import org.springframework.data.repository.core.EntityMetadata;
 
 /**
@@ -30,4 +31,12 @@ public interface MongoEntityMetadata<T> extends EntityMetadata<T> {
 	 * @return
 	 */
 	String getCollectionName();
+
+	/**
+	 * Returns the {@link MongoPersistentEntity} that supposed to determine the collection to be queried.
+	 * 
+	 * @return
+	 * @since 2.0.5
+	 */
+	MongoPersistentEntity<?> getCollectionEntity();
 }

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

@@ -15,6 +15,8 @@
  */
 package org.springframework.data.mongodb.repository.query;
 
+import lombok.Getter;
+
 import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
 import org.springframework.util.Assert;
 
@@ -26,7 +28,7 @@ import org.springframework.util.Assert;
 class SimpleMongoEntityMetadata<T> implements MongoEntityMetadata<T> {
 
 	private final Class<T> type;
-	private final MongoPersistentEntity<?> collectionEntity;
+	private final @Getter MongoPersistentEntity<?> collectionEntity;
 
 	/**
 	 * Creates a new {@link SimpleMongoEntityMetadata} using the given type and {@link MongoPersistentEntity} to use for

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

@@ -17,8 +17,8 @@ package org.springframework.data.mongodb.repository.query;
 
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
+import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.*;
 
 import java.lang.reflect.Method;
@@ -26,6 +26,7 @@ import java.util.List;
 import java.util.Optional;
 
 import org.bson.Document;
+import org.bson.types.ObjectId;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -40,7 +41,6 @@ import org.springframework.data.domain.Slice;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.mongodb.MongoDbFactory;
 import org.springframework.data.mongodb.core.ExecutableFindOperation.ExecutableFind;
-import org.springframework.data.mongodb.core.ExecutableFindOperation.FindWithProjection;
 import org.springframework.data.mongodb.core.ExecutableFindOperation.FindWithQuery;
 import org.springframework.data.mongodb.core.MongoOperations;
 import org.springframework.data.mongodb.core.Person;
@@ -55,6 +55,7 @@ import org.springframework.data.mongodb.repository.Meta;
 import org.springframework.data.mongodb.repository.MongoRepository;
 import org.springframework.data.projection.ProjectionFactory;
 import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
+import org.springframework.data.repository.Repository;
 import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
 
 import com.mongodb.client.result.DeleteResult;
@@ -71,8 +72,7 @@ import com.mongodb.client.result.DeleteResult;
 public class AbstractMongoQueryUnitTests {
 
 	@Mock MongoOperations mongoOperationsMock;
-	@Mock ExecutableFind<?> findOperationMock;
+	@Mock ExecutableFind<?> executableFind;
-	@Mock FindWithProjection<?> withProjectionMock;
 	@Mock FindWithQuery<?> withQueryMock;
 	@Mock BasicMongoPersistentEntity<?> persitentEntityMock;
 	@Mock MongoMappingContext mappingContextMock;
@@ -91,9 +91,8 @@ public class AbstractMongoQueryUnitTests {
 		converter.afterPropertiesSet();
 
 		doReturn(converter).when(mongoOperationsMock).getConverter();
-		doReturn(findOperationMock).when(mongoOperationsMock).query(any());
+		doReturn(executableFind).when(mongoOperationsMock).query(any());
-		doReturn(withProjectionMock).when(findOperationMock).inCollection(any());
+		doReturn(withQueryMock).when(executableFind).as(any());
-		doReturn(withQueryMock).when(withProjectionMock).as(any());
 		doReturn(withQueryMock).when(withQueryMock).matching(any());
 	}
 
@@ -144,9 +143,8 @@ public class AbstractMongoQueryUnitTests {
 
 		ArgumentCaptor<Query> captor = ArgumentCaptor.forClass(Query.class);
 
-		verify(withProjectionMock).as(Person.class);
+		verify(executableFind).as(Person.class);
 		verify(withQueryMock).matching(captor.capture());
-		verify(findOperationMock).inCollection("persons");
 
 		assertThat(captor.getValue().getMeta().getComment(), nullValue());
 	}
@@ -159,9 +157,8 @@ public class AbstractMongoQueryUnitTests {
 
 		ArgumentCaptor<Query> captor = ArgumentCaptor.forClass(Query.class);
 
-		verify(withProjectionMock).as(Person.class);
+		verify(executableFind).as(Person.class);
 		verify(withQueryMock).matching(captor.capture());
-		verify(findOperationMock).inCollection("persons");
 
 		assertThat(captor.getValue().getMeta().getComment(), is("comment"));
 	}
@@ -174,9 +171,8 @@ public class AbstractMongoQueryUnitTests {
 
 		ArgumentCaptor<Query> captor = ArgumentCaptor.forClass(Query.class);
 
-		verify(withProjectionMock).as(Person.class);
+		verify(executableFind).as(Person.class);
 		verify(withQueryMock).matching(captor.capture());
-		verify(findOperationMock).inCollection("persons");
 
 		assertThat(captor.getValue().getMeta().getComment(), is("comment"));
 	}
@@ -189,9 +185,8 @@ public class AbstractMongoQueryUnitTests {
 
 		ArgumentCaptor<Query> captor = ArgumentCaptor.forClass(Query.class);
 
-		verify(withProjectionMock).as(Person.class);
+		verify(executableFind).as(Person.class);
 		verify(withQueryMock).matching(captor.capture());
-		verify(findOperationMock).inCollection("persons");
 
 		assertThat(captor.getValue().getMeta().getComment(), is("comment"));
 	}
@@ -208,9 +203,8 @@ public class AbstractMongoQueryUnitTests {
 
 		ArgumentCaptor<Query> captor = ArgumentCaptor.forClass(Query.class);
 
-		verify(withProjectionMock, times(2)).as(Person.class);
+		verify(executableFind, times(2)).as(Person.class);
 		verify(withQueryMock, times(2)).matching(captor.capture());
-		verify(findOperationMock).inCollection("persons");
 
 		assertThat(captor.getAllValues().get(0).getSkip(), is(0L));
 		assertThat(captor.getAllValues().get(1).getSkip(), is(10L));
@@ -228,9 +222,8 @@ public class AbstractMongoQueryUnitTests {
 
 		ArgumentCaptor<Query> captor = ArgumentCaptor.forClass(Query.class);
 
-		verify(withProjectionMock, times(2)).as(Person.class);
+		verify(executableFind, times(2)).as(Person.class);
 		verify(withQueryMock, times(2)).matching(captor.capture());
-		verify(findOperationMock).inCollection("persons");
 
 		assertThat(captor.getAllValues().get(0).getLimit(), is(11));
 		assertThat(captor.getAllValues().get(1).getLimit(), is(11));
@@ -248,9 +241,8 @@ public class AbstractMongoQueryUnitTests {
 
 		ArgumentCaptor<Query> captor = ArgumentCaptor.forClass(Query.class);
 
-		verify(withProjectionMock, times(2)).as(Person.class);
+		verify(executableFind, times(2)).as(Person.class);
 		verify(withQueryMock, times(2)).matching(captor.capture());
-		verify(findOperationMock).inCollection("persons");
 
 		Document expectedSortObject = new Document().append("bar", -1);
 		assertThat(captor.getAllValues().get(0).getSortObject(), is(expectedSortObject));
@@ -269,17 +261,31 @@ public class AbstractMongoQueryUnitTests {
 		assertThat(query.execute(new Object[] { "lastname" }), is(reference));
 	}
 
+	@Test // DATAMONGO-1872
+	public void doesNotFixCollectionOnPreparation() {
+
+		AbstractMongoQuery query = createQueryForMethod(DynamicallyMappedRepository.class, "findBy");
+
+		query.execute(new Object[0]);
+
+		verify(executableFind, never()).inCollection(anyString());
+		verify(executableFind).as(DynamicallyMapped.class);
+	}
+
 	private MongoQueryFake createQueryForMethod(String methodName, Class<?>... paramTypes) {
+		return createQueryForMethod(Repo.class, methodName, paramTypes);
+	}
+
+	private MongoQueryFake createQueryForMethod(Class<?> repository, String methodName, Class<?>... paramTypes) {
 
 		try {
 
-			Method method = Repo.class.getMethod(methodName, paramTypes);
+			Method method = repository.getMethod(methodName, paramTypes);
 			ProjectionFactory factory = new SpelAwareProxyProjectionFactory();
-			MongoQueryMethod queryMethod = new MongoQueryMethod(method, new DefaultRepositoryMetadata(Repo.class), factory,
+			MongoQueryMethod queryMethod = new MongoQueryMethod(method, new DefaultRepositoryMetadata(repository), factory,
 					mappingContextMock);
 
 			return new MongoQueryFake(queryMethod, mongoOperationsMock);
-
 		} catch (Exception e) {
 			throw new IllegalArgumentException(e.getMessage(), e);
 		}
@@ -339,4 +345,13 @@ public class AbstractMongoQueryUnitTests {
 
 		Optional<Person> findByLastname(String lastname);
 	}
+
+	// DATAMONGO-1872
+
+	@org.springframework.data.mongodb.core.mapping.Document(collection = "#{T(java.lang.Math).random()}")
+	static class DynamicallyMapped {}
+
+	interface DynamicallyMappedRepository extends Repository<DynamicallyMapped, ObjectId> {
+		DynamicallyMapped findBy();
+	}
 }