From 2ef9844219351674e6cbf78bbbd7724195679506 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 19 Feb 2021 13:47:28 +0100 Subject: [PATCH] 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: #590 --- .../data/mongodb/core/EntityOperations.java | 16 ++++++++ .../data/mongodb/core/MongoTemplate.java | 32 +++++++++++---- .../mongodb/core/ReactiveMongoTemplate.java | 41 ++++++++++--------- .../mongodb/core/MongoTemplateUnitTests.java | 20 +++++++++ .../core/ReactiveMongoTemplateUnitTests.java | 9 ++++ 5 files changed, 90 insertions(+), 28 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java index 7effa25ab..f2daf0287 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/EntityOperations.java +++ b/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.Assert; 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 @@ class EntityOperations { 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}. diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index 2aaee97e8..5fbb05149 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java @@ -705,7 +705,6 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, }); } - @Override public IndexOperations indexOps(String collectionName) { return indexOps(collectionName, null); @@ -1154,7 +1153,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, Assert.notNull(objectToSave, "ObjectToSave must not be null!"); - ensureNotAnArrayOrCollection(objectToSave); + ensureNotIterable(objectToSave); return insert(objectToSave, getCollectionName(ClassUtils.getUserClass(objectToSave))); } @@ -1169,15 +1168,32 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, 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."); } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java index cb9bbd5b9..d6743db9d 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java @@ -17,8 +17,6 @@ package org.springframework.data.mongodb.core; 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.Publisher; 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.ReactiveMongoDatabaseFactory; 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.Aggregation; 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> ITERABLE_CLASSES; - - static { - - Set> 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, MongoPersistentProperty> mappingContext; @@ -2668,13 +2655,27 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati } } - 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."); } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java index 11d25bb19..74ee74b1f 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java @@ -2206,6 +2206,13 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { 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 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { @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. diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java index b39cb9797..eb44de349 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateUnitTests.java @@ -1400,6 +1400,15 @@ public class ReactiveMongoTemplateUnitTests { verify(collection).estimatedDocumentCount(any()); } + @Test // GH-2911 + void insertErrorsOnPublisher() { + + Publisher publisher = Mono.just("data"); + + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> template.insert(publisher)); + } + private void stubFindSubscribe(Document document) { Publisher realPublisher = Flux.just(document);