Move collection like check for reactive-/template insert methods to EntityOperations.

Reintroduce the protected ensureNotIterable method in MongoTemplate to keep the API stable. Delegate to the newly introduced method and move the collection like check to EntityOperations.

Original Pull Request: spring-projects#590

Author: christophstrobl

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java

@@ -16,6 +16,7 @@
 package org.springframework.data.mongodb.core;
 
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.Optional;
 
@@ -40,6 +41,7 @@
 import org.springframework.util.ClassUtils;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
+import org.springframework.util.ObjectUtils;
 
 /**
  * Common operations performed on an entity in the context of it's mapping metadata.
@@ -107,6 +109,20 @@ <T> AdaptibleEntity<T> forEntity(T entity, ConversionService conversionService)
 		return AdaptibleMappedEntity.of(entity, context, conversionService);
 	}
 
+	/**
+	 * @param source can be {@literal null}.
+	 * @return {@literal true} if the given value is an {@literal array}, {@link Collection} or {@link Iterator}.
+	 * @since 3.2
+	 */
+	static boolean isCollectionLike(@Nullable Object source) {
+
+		if (source == null) {
+			return false;
+		}
+
+		return ObjectUtils.isArray(source) || source instanceof Collection || source instanceof Iterator;
+	}
+
 	/**
 	 * @param entityClass should not be null.
 	 * @return the {@link MongoPersistentEntity#getCollection() collection name}.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java

@@ -705,7 +705,6 @@ public void dropCollection(String collectionName) {
 		});
 	}
 
-
 	@Override
 	public IndexOperations indexOps(String collectionName) {
 		return indexOps(collectionName, null);
@@ -1154,7 +1153,7 @@ public <T> T insert(T objectToSave) {
 
 		Assert.notNull(objectToSave, "ObjectToSave must not be null!");
 
-		ensureNotAnArrayOrCollection(objectToSave);
+		ensureNotIterable(objectToSave);
 		return insert(objectToSave, getCollectionName(ClassUtils.getUserClass(objectToSave)));
 	}
 
@@ -1169,15 +1168,32 @@ public <T> T insert(T objectToSave, String collectionName) {
 		Assert.notNull(objectToSave, "ObjectToSave must not be null!");
 		Assert.notNull(collectionName, "CollectionName must not be null!");
 
-		ensureNotAnArrayOrCollection(objectToSave);
+		ensureNotIterable(objectToSave);
 		return (T) doInsert(collectionName, objectToSave, this.mongoConverter);
 	}
 
-	protected void ensureNotAnArrayOrCollection(@Nullable Object o) {
-		if (null != o) {
-			if (o.getClass().isArray() || (o instanceof Collection) || (o instanceof Iterator)) {
-				throw new IllegalArgumentException("Cannot use a collection here.");
-			}
+	/**
+	 * Ensure the given {@literal source} is not an {@link java.lang.reflect.Array}, {@link Collection} or
+	 * {@link Iterator}.
+	 *
+	 * @param source can be {@literal null}.
+	 * @deprecated since 3.2. Call {@link #ensureNotCollectionLike(Object)} instead.
+	 */
+	protected void ensureNotIterable(@Nullable Object source) {
+		ensureNotCollectionLike(source);
+	}
+
+	/**
+	 * Ensure the given {@literal source} is not an {@link java.lang.reflect.Array}, {@link Collection} or
+	 * {@link Iterator}.
+	 *
+	 * @param source can be {@literal null}.
+	 * @since 3.2.
+	 */
+	protected void ensureNotCollectionLike(@Nullable Object source) {
+
+		if (EntityOperations.isCollectionLike(source)) {
+			throw new IllegalArgumentException("Cannot use a collection here.");
 		}
 	}
 

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java

@@ -17,8 +17,6 @@
 
 import static org.springframework.data.mongodb.core.query.SerializationUtils.*;
 
-import org.springframework.data.mongodb.core.QueryOperations.AggregationDefinition;
-import org.springframework.data.mongodb.core.aggregation.RelaxedTypeBasedAggregationOperationContext;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 import reactor.util.function.Tuple2;
@@ -38,7 +36,6 @@
 import org.reactivestreams.Subscriber;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import org.springframework.beans.BeansException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
@@ -63,6 +60,7 @@
 import org.springframework.data.mongodb.ReactiveMongoDatabaseUtils;
 import org.springframework.data.mongodb.SessionSynchronization;
 import org.springframework.data.mongodb.core.EntityOperations.AdaptibleEntity;
+import org.springframework.data.mongodb.core.QueryOperations.AggregationDefinition;
 import org.springframework.data.mongodb.core.QueryOperations.CountContext;
 import org.springframework.data.mongodb.core.QueryOperations.DeleteContext;
 import org.springframework.data.mongodb.core.QueryOperations.DistinctQueryContext;
@@ -72,6 +70,7 @@
 import org.springframework.data.mongodb.core.aggregation.AggregationOperationContext;
 import org.springframework.data.mongodb.core.aggregation.AggregationOptions;
 import org.springframework.data.mongodb.core.aggregation.PrefixingDelegatingAggregationOperationContext;
+import org.springframework.data.mongodb.core.aggregation.RelaxedTypeBasedAggregationOperationContext;
 import org.springframework.data.mongodb.core.aggregation.TypeBasedAggregationOperationContext;
 import org.springframework.data.mongodb.core.aggregation.TypedAggregation;
 import org.springframework.data.mongodb.core.convert.DbRefResolver;
@@ -157,18 +156,6 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati
 
 	private static final Logger LOGGER = LoggerFactory.getLogger(ReactiveMongoTemplate.class);
 	private static final WriteResultChecking DEFAULT_WRITE_RESULT_CHECKING = WriteResultChecking.NONE;
-	private static final Collection<Class<?>> ITERABLE_CLASSES;
-
-	static {
-
-		Set<Class<?>> iterableClasses = new HashSet<>();
-		iterableClasses.add(List.class);
-		iterableClasses.add(Collection.class);
-		iterableClasses.add(Iterator.class);
-		iterableClasses.add(Publisher.class);
-
-		ITERABLE_CLASSES = Collections.unmodifiableCollection(iterableClasses);
-	}
 
 	private final MongoConverter mongoConverter;
 	private final MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> mappingContext;
@@ -2668,13 +2655,27 @@ private MongoCollection<Document> getAndPrepareCollection(MongoDatabase db, Stri
 		}
 	}
 
-	protected void ensureNotIterable(Object o) {
+	/**
+	 * Ensure the given {@literal source} is not an {@link java.lang.reflect.Array}, {@link Collection} or
+	 * {@link Iterator}.
+	 *
+	 * @param source can be {@literal null}.
+	 * @deprecated since 3.2. Call {@link #ensureNotCollectionLike(Object)} instead.
+	 */
+	protected void ensureNotIterable(@Nullable Object source) {
+		ensureNotCollectionLike(source);
+	}
 
-		boolean isIterable = o.getClass().isArray()
-				|| ITERABLE_CLASSES.stream().anyMatch(iterableClass -> iterableClass.isAssignableFrom(o.getClass())
-						|| o.getClass().getName().equals(iterableClass.getName()));
+	/**
+	 * Ensure the given {@literal source} is not an {@link java.lang.reflect.Array}, {@link Collection} or
+	 * {@link Iterator}.
+	 *
+	 * @param source can be {@literal null}.
+	 * @since 3.2.
+	 */
+	protected void ensureNotCollectionLike(@Nullable Object source) {
 
-		if (isIterable) {
+		if (EntityOperations.isCollectionLike(source) || source instanceof Publisher) {
 			throw new IllegalArgumentException("Cannot use a collection here.");
 		}
 	}

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java

@@ -2206,6 +2206,13 @@ void esitmatedCountShouldBeDelegatedCorrectly() {
 		verify(collection).estimatedDocumentCount(any());
 	}
 
+	@Test // GH-2911
+	void insertErrorsOnCustomIteratorImplementation() {
+
+		assertThatExceptionOfType(IllegalArgumentException.class)
+				.isThrownBy(() -> template.insert(new TypeImplementingIterator()));
+	}
+
 	class AutogenerateableId {
 
 		@Id BigInteger id;
@@ -2299,6 +2306,19 @@ static class Sith {
 		@Field("firstname") String name;
 	}
 
+	static class TypeImplementingIterator implements Iterator {
+
+		@Override
+		public boolean hasNext() {
+			return false;
+		}
+
+		@Override
+		public Object next() {
+			return null;
+		}
+	}
+
 	/**
 	 * Mocks out the {@link MongoTemplate#getDb()} method to return the {@link DB} mock instead of executing the actual
 	 * behaviour.

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java

@@ -1400,6 +1400,15 @@ void esitmatedCountShouldBeDelegatedCorrectly() {
 		verify(collection).estimatedDocumentCount(any());
 	}
 
+	@Test // GH-2911
+	void insertErrorsOnPublisher() {
+
+		Publisher<String> publisher = Mono.just("data");
+
+		assertThatExceptionOfType(IllegalArgumentException.class)
+				.isThrownBy(() -> template.insert(publisher));
+	}
+
 	private void stubFindSubscribe(Document document) {
 
 		Publisher<Document> realPublisher = Flux.just(document);