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 d8ac2d0a0..eae4f4270 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 @@ -28,6 +28,7 @@ import org.bson.Document; import org.bson.conversions.Bson; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -46,6 +47,7 @@ import org.springframework.data.geo.Distance; import org.springframework.data.geo.GeoResult; import org.springframework.data.geo.GeoResults; import org.springframework.data.geo.Metric; +import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.callback.EntityCallbacks; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mongodb.MongoDatabaseFactory; @@ -102,7 +104,6 @@ import org.springframework.data.mongodb.util.BsonUtils; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.util.CloseableIterator; import org.springframework.data.util.Optionals; -import org.springframework.jca.cci.core.ConnectionCallback; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -972,7 +973,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, for (Document element : results) { GeoResult geoResult = callback.doWith(element); - aggregate = aggregate.add(new BigDecimal(geoResult.getDistance().getValue())); + aggregate = aggregate.add(BigDecimal.valueOf(geoResult.getDistance().getValue())); result.add(geoResult); } @@ -2751,25 +2752,24 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, * Internal method using callbacks to do queries against the datastore that requires reading a single object from a * collection of objects. It will take the following steps *
    - *
  1. Execute the given {@link ConnectionCallback} for a {@link Document}.
    2. + *
  2. Execute the given {@link CollectionCallback} for a {@link Document}.
    3. *
  3. Apply the given {@link DocumentCallback} to each of the {@link Document}s to obtain the result.
    4. *
      * * @param * @param collectionCallback the callback to retrieve the {@link Document} with - * @param objectCallback the {@link DocumentCallback} to transform {@link Document}s into the actual domain type + * @param documentCallback the {@link DocumentCallback} to transform {@link Document}s into the actual domain type * @param collectionName the collection to be queried * @return */ @Nullable private T executeFindOneInternal(CollectionCallback collectionCallback, - DocumentCallback objectCallback, String collectionName) { + DocumentCallback documentCallback, String collectionName) { try { - T result = objectCallback - .doWith(collectionCallback.doInCollection(getAndPrepareCollection(doGetDatabase(), collectionName))); - return result; + Document document = collectionCallback.doInCollection(getAndPrepareCollection(doGetDatabase(), collectionName)); + return document != null ? documentCallback.doWith(document) : null; } catch (RuntimeException e) { throw potentiallyConvertRuntimeException(e, exceptionTranslator); } @@ -2779,7 +2779,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, * Internal method using callback to do queries against the datastore that requires reading a collection of objects. * It will take the following steps *
        - *
      1. Execute the given {@link ConnectionCallback} for a {@link FindIterable}.
        2. + *
      2. Execute the given {@link CollectionCallback} for a {@link FindIterable}.
        3. *
      3. Prepare that {@link FindIterable} with the given {@link CursorPreparer} (will be skipped if * {@link CursorPreparer} is {@literal null}
        4. *
      4. Iterate over the {@link FindIterable} and applies the given {@link DocumentCallback} to each of the @@ -2789,36 +2789,27 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, * @param * @param collectionCallback the callback to retrieve the {@link FindIterable} with * @param preparer the {@link CursorPreparer} to potentially modify the {@link FindIterable} before iterating over it - * @param objectCallback the {@link DocumentCallback} to transform {@link Document}s into the actual domain type + * @param documentCallback the {@link DocumentCallback} to transform {@link Document}s into the actual domain type * @param collectionName the collection to be queried * @return */ private List executeFindMultiInternal(CollectionCallback> collectionCallback, - CursorPreparer preparer, DocumentCallback objectCallback, String collectionName) { + CursorPreparer preparer, DocumentCallback documentCallback, String collectionName) { try { - MongoCursor cursor = null; - - try { - - cursor = preparer - .initiateFind(getAndPrepareCollection(doGetDatabase(), collectionName), collectionCallback::doInCollection) - .iterator(); + try (MongoCursor cursor = preparer + .initiateFind(getAndPrepareCollection(doGetDatabase(), collectionName), collectionCallback::doInCollection) + .iterator()) { List result = new ArrayList<>(); while (cursor.hasNext()) { Document object = cursor.next(); - result.add(objectCallback.doWith(object)); + result.add(documentCallback.doWith(object)); } return result; - } finally { - - if (cursor != null) { - cursor.close(); - } } } catch (RuntimeException e) { throw potentiallyConvertRuntimeException(e, exceptionTranslator); @@ -2828,24 +2819,13 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, private void executeQueryInternal(CollectionCallback> collectionCallback, CursorPreparer preparer, DocumentCallbackHandler callbackHandler, String collectionName) { - try { - - MongoCursor cursor = null; - - try { - - cursor = preparer - .initiateFind(getAndPrepareCollection(doGetDatabase(), collectionName), collectionCallback::doInCollection) - .iterator(); + try (MongoCursor cursor = preparer + .initiateFind(getAndPrepareCollection(doGetDatabase(), collectionName), collectionCallback::doInCollection) + .iterator()) { while (cursor.hasNext()) { callbackHandler.processDocument(cursor.next()); } - } finally { - if (cursor != null) { - cursor.close(); - } - } } catch (RuntimeException e) { throw potentiallyConvertRuntimeException(e, exceptionTranslator); } @@ -3143,8 +3123,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, interface DocumentCallback { - @Nullable - T doWith(@Nullable Document object); + T doWith(Document object); } /** @@ -3168,22 +3147,19 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, this.collectionName = collectionName; } - @Nullable - public T doWith(@Nullable Document document) { + public T doWith(Document document) { - T source = null; - - if (document != null) { maybeEmitEvent(new AfterLoadEvent<>(document, type, collectionName)); - source = reader.read(type, document); - } + T entity = reader.read(type, document); - if (source != null) { - maybeEmitEvent(new AfterConvertEvent<>(document, source, collectionName)); - source = maybeCallAfterConvert(source, document, collectionName); - } + if (entity == null) { + throw new MappingException(String.format("EntityReader %s returned null", reader)); + } - return source; + maybeEmitEvent(new AfterConvertEvent<>(document, entity, collectionName)); + entity = maybeCallAfterConvert(entity, document, collectionName); + + return entity; } } @@ -3216,8 +3192,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, * @see org.springframework.data.mongodb.core.MongoTemplate.DocumentCallback#doWith(org.bson.Document) */ @SuppressWarnings("unchecked") - @Nullable - public T doWith(@Nullable Document document) { + public T doWith(Document document) { if (document == null) { return null; @@ -3228,15 +3203,16 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, maybeEmitEvent(new AfterLoadEvent<>(document, targetType, collectionName)); - Object source = reader.read(typeToRead, document); - Object result = targetType.isInterface() ? projectionFactory.createProjection(targetType, source) : source; + Object entity = reader.read(typeToRead, document); - if (result != null) { - maybeEmitEvent(new AfterConvertEvent<>(document, result, collectionName)); - result = maybeCallAfterConvert(result, document, collectionName); + if (entity == null) { + throw new MappingException(String.format("EntityReader %s returned null", reader)); } - return (T) result; + Object result = targetType.isInterface() ? projectionFactory.createProjection(targetType, entity) : entity; + + maybeEmitEvent(new AfterConvertEvent<>(document, result, collectionName)); + return (T) maybeCallAfterConvert(result, document, collectionName); } } @@ -3373,8 +3349,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, this.metric = metric; } - @Nullable - public GeoResult doWith(@Nullable Document object) { + public GeoResult doWith(Document object) { double distance = Double.NaN; if (object.containsKey(distanceField)) { @@ -3401,10 +3376,6 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, /** * Creates a new {@link CloseableIterableCursorAdapter} backed by the given {@link MongoCollection}. - * - * @param cursor - * @param exceptionTranslator - * @param objectReadCallback */ CloseableIterableCursorAdapter(MongoIterable cursor, PersistenceExceptionTranslator exceptionTranslator, DocumentCallback objectReadCallback) { @@ -3448,8 +3419,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, try { Document item = cursor.next(); - T converted = objectReadCallback.doWith(item); - return converted; + return objectReadCallback.doWith(item); } catch (RuntimeException ex) { throw potentiallyConvertRuntimeException(ex, exceptionTranslator); } 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 d6743db9d..614894f3b 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 @@ -22,7 +22,15 @@ import reactor.core.publisher.Mono; import reactor.util.function.Tuple2; import reactor.util.function.Tuples; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import java.util.function.Function; @@ -36,6 +44,7 @@ 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; @@ -51,6 +60,7 @@ import org.springframework.data.convert.EntityReader; import org.springframework.data.geo.Distance; import org.springframework.data.geo.GeoResult; import org.springframework.data.geo.Metric; +import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.callback.ReactiveEntityCallbacks; import org.springframework.data.mapping.context.MappingContext; @@ -3152,13 +3162,14 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati maybeEmitEvent(new AfterLoadEvent<>(document, type, collectionName)); - T source = reader.read(type, document); - if (source != null) { - maybeEmitEvent(new AfterConvertEvent<>(document, source, collectionName)); - return maybeCallAfterConvert(source, document, collectionName); + T entity = reader.read(type, document); + + if (entity == null) { + throw new MappingException(String.format("EntityReader %s returned null", reader)); } - return Mono.empty(); + maybeEmitEvent(new AfterConvertEvent<>(document, entity, collectionName)); + return maybeCallAfterConvert(entity, document, collectionName); } } @@ -3196,16 +3207,17 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati maybeEmitEvent(new AfterLoadEvent<>(document, typeToRead, collectionName)); - Object source = reader.read(typeToRead, document); - Object result = targetType.isInterface() ? projectionFactory.createProjection(targetType, source) : source; + Object entity = reader.read(typeToRead, document); - T castEntity = (T) result; - if (castEntity != null) { - maybeEmitEvent(new AfterConvertEvent<>(document, castEntity, collectionName)); - return maybeCallAfterConvert(castEntity, document, collectionName); + if (entity == null) { + throw new MappingException(String.format("EntityReader %s returned null", reader)); } - return Mono.empty(); + Object result = targetType.isInterface() ? projectionFactory.createProjection(targetType, entity) : entity; + + T castEntity = (T) result; + maybeEmitEvent(new AfterConvertEvent<>(document, castEntity, collectionName)); + return maybeCallAfterConvert(castEntity, document, collectionName); } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index 4a2a7fc15..74d189b4c 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java @@ -270,7 +270,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App * (non-Javadoc) * @see org.springframework.data.mongodb.core.core.MongoReader#read(java.lang.Class, com.mongodb.Document) */ - public S read(Class clazz, final Bson bson) { + public S read(Class clazz, Bson bson) { return read(ClassTypeInformation.from(clazz), bson); } 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 150ba5a86..f8170889b 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 @@ -50,6 +50,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; + import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationListener; @@ -64,8 +65,10 @@ import org.springframework.data.annotation.Version; import org.springframework.data.convert.CustomConversions; import org.springframework.data.domain.Sort; import org.springframework.data.geo.Point; +import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.callback.EntityCallbacks; import org.springframework.data.mapping.context.InvalidPersistentPropertyPath; +import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mongodb.MongoDatabaseFactory; import org.springframework.data.mongodb.core.aggregation.*; import org.springframework.data.mongodb.core.aggregation.ComparisonOperators.Gte; @@ -394,11 +397,24 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { verify(collection, times(1)).find(Mockito.eq(query.getQueryObject()), any(Class.class)); } + @Test // GH-3648 + void shouldThrowExceptionIfEntityReaderReturnsNull() { + + when(cursor.hasNext()).thenReturn(true).thenReturn(true).thenReturn(false); + when(cursor.next()).thenReturn(new org.bson.Document("_id", Integer.valueOf(0))); + MappingMongoConverter converter = mock(MappingMongoConverter.class); + when(converter.getMappingContext()).thenReturn((MappingContext) mappingContext); + template = new MongoTemplate(factory, converter); + + assertThatExceptionOfType(MappingException.class).isThrownBy(() -> template.findAll(Person.class)) + .withMessageContaining("returned null"); + } + @Test // DATAMONGO-566 void findAllAndRemoveShouldRemoveDocumentsReturedByFindQuery() { - Mockito.when(cursor.hasNext()).thenReturn(true).thenReturn(true).thenReturn(false); - Mockito.when(cursor.next()).thenReturn(new org.bson.Document("_id", Integer.valueOf(0))) + when(cursor.hasNext()).thenReturn(true).thenReturn(true).thenReturn(false); + when(cursor.next()).thenReturn(new org.bson.Document("_id", Integer.valueOf(0))) .thenReturn(new org.bson.Document("_id", Integer.valueOf(1))); ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(org.bson.Document.class); 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 eb44de349..931ea75ce 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 @@ -57,7 +57,9 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationListener; import org.springframework.context.support.StaticApplicationContext; import org.springframework.data.annotation.Id; +import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.callback.ReactiveEntityCallbacks; +import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mongodb.core.MongoTemplateUnitTests.AutogenerateableId; import org.springframework.data.mongodb.core.aggregation.AggregationOptions; import org.springframework.data.mongodb.core.aggregation.AggregationUpdate; @@ -1137,6 +1139,19 @@ public class ReactiveMongoTemplateUnitTests { verify(findPublisher).projection(new Document("country", 1).append("userid", 1)); } + @Test // GH-3648 + void shouldThrowExceptionIfEntityReaderReturnsNull() { + + MappingMongoConverter converter = mock(MappingMongoConverter.class); + when(converter.getMappingContext()).thenReturn((MappingContext) mappingContext); + template = new ReactiveMongoTemplate(factory, converter); + + when(collection.find(Document.class)).thenReturn(findPublisher); + stubFindSubscribe(new Document()); + + template.find(new Query(), Person.class).as(StepVerifier::create).verifyError(MappingException.class); + } + @Test // DATAMONGO-2479 void findShouldInvokeAfterConvertCallbacks() {