From e2d901ff299c88c6a96449d0b9f3cb52091cf72e Mon Sep 17 00:00:00 2001 From: Piotr Kubowicz Date: Tue, 15 Jun 2021 09:57:08 +0200 Subject: [PATCH] DATAMONGO-2511 Give context to mapping exceptions When invoking findAll() exception details contained no information which document was malformed and other details (for example which field was wrong) were usually useless for identifying the document. Now the mapping exception will include the document. --- .../core/convert/MappingMongoConverter.java | 19 +++++----- .../convert/MappingMongoConverterTests.java | 37 ++++++++++++++++++- .../MappingMongoConverterUnitTests.java | 10 +++-- 3 files changed, 51 insertions(+), 15 deletions(-) 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 732e8c9a51..c5ee635a45 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 @@ -43,6 +43,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.core.CollectionFactory; +import org.springframework.core.convert.ConversionException; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.data.annotation.Reference; @@ -54,15 +55,7 @@ import org.springframework.data.mapping.PreferredConstructor.Parameter; import org.springframework.data.mapping.callback.EntityCallbacks; import org.springframework.data.mapping.context.MappingContext; -import org.springframework.data.mapping.model.ConvertingPropertyAccessor; -import org.springframework.data.mapping.model.DefaultSpELExpressionEvaluator; -import org.springframework.data.mapping.model.EntityInstantiator; -import org.springframework.data.mapping.model.ParameterValueProvider; -import org.springframework.data.mapping.model.PersistentEntityParameterValueProvider; -import org.springframework.data.mapping.model.PropertyValueProvider; -import org.springframework.data.mapping.model.SpELContext; -import org.springframework.data.mapping.model.SpELExpressionEvaluator; -import org.springframework.data.mapping.model.SpELExpressionParameterValueProvider; +import org.springframework.data.mapping.model.*; import org.springframework.data.mongodb.CodecRegistryProvider; import org.springframework.data.mongodb.MongoDatabaseFactory; import org.springframework.data.mongodb.core.mapping.BasicMongoPersistentProperty; @@ -104,11 +97,13 @@ * @author Roman Puchkovskiy * @author Heesu Jung * @author Divya Srivastava + * @author Piotr Kubowicz */ public class MappingMongoConverter extends AbstractMongoConverter implements ApplicationContextAware { private static final String INCOMPATIBLE_TYPES = "Cannot convert %1$s of type %2$s into an instance of %3$s! Implement a custom Converter<%2$s, %3$s> and register it with the CustomConversions. Parent object was: %4$s"; private static final String INVALID_TYPE_TO_READ = "Expected to read Document %s into type %s but didn't find a PersistentEntity for the latter!"; + private static final String CANNOT_READ = "Failed to read %s"; public static final ClassTypeInformation BSON = ClassTypeInformation.from(Bson.class); @@ -350,7 +345,11 @@ protected S readDocument(ConversionContext context, Bson bson throw new MappingException(String.format(INVALID_TYPE_TO_READ, document, rawType)); } - return read(context, (MongoPersistentEntity) entity, document); + try { + return read(context, (MongoPersistentEntity) entity, document); + } catch (ConversionException | MappingInstantiationException e) { + throw new MappingException(String.format(CANNOT_READ, document), e); + } } private ParameterValueProvider getParameterProvider(ConversionContext context, diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java index 2b17ed4b06..6faadc4024 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterTests.java @@ -23,6 +23,8 @@ import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import lombok.NonNull; +import lombok.Value; import java.time.Instant; import java.time.LocalDate; @@ -55,6 +57,7 @@ * Integration tests for {@link MappingMongoConverter}. * * @author Christoph Strobl + * @author Piotr Kubowicz */ @ExtendWith(MongoClientExtension.class) public class MappingMongoConverterTests { @@ -76,12 +79,13 @@ void setUp() { database.getCollection("samples").deleteMany(new Document()); database.getCollection("java-time-types").deleteMany(new Document()); + database.getCollection("with-nonnull").deleteMany(new Document()); dbRefResolver = spy(new DefaultDbRefResolver(factory)); mappingContext = new MongoMappingContext(); mappingContext.setInitialEntitySet(new HashSet<>( - Arrays.asList(WithLazyDBRefAsConstructorArg.class, WithLazyDBRef.class, WithJavaTimeTypes.class))); + Arrays.asList(WithLazyDBRefAsConstructorArg.class, WithLazyDBRef.class, WithJavaTimeTypes.class, WithNonnullField.class))); mappingContext.setAutoIndexCreation(false); mappingContext.afterPropertiesSet(); @@ -145,6 +149,31 @@ void readJavaTimeValuesWrittenViaCodec() { .isEqualTo(source); } + @Test // DATAMONGO-2511 + public void reportConversionFailedExceptionContext() { + + configureConverterWithNativeJavaTimeCodec(); + MongoCollection mongoCollection = client.getDatabase(DATABASE).getCollection("java-time-types"); + + mongoCollection.insertOne(new Document("_id", "id-of-wrong-document").append("localDate", "not-a-date")); + + assertThatThrownBy(() -> converter.read(WithJavaTimeTypes.class, + mongoCollection.find(new Document("_id", "id-of-wrong-document")).first())) + .hasMessageContaining("id-of-wrong-document"); + } + + @Test // DATAMONGO-2511 + public void reportMappingInstantiationExceptionContext() { + + MongoCollection mongoCollection = client.getDatabase(DATABASE).getCollection("with-nonnull"); + + mongoCollection.insertOne(new Document("_id", "id-of-wrong-document")); + + assertThatThrownBy(() -> converter.read(WithNonnullField.class, + mongoCollection.find(new Document("_id", "id-of-wrong-document")).first())) + .hasMessageContaining("id-of-wrong-document"); + } + void configureConverterWithNativeJavaTimeCodec() { converter = new MappingMongoConverter(dbRefResolver, mappingContext); @@ -212,4 +241,10 @@ Document toDocument() { .append("localDateTime", localDateTime); } } + + @Value + static class WithNonnullField { + @Id String id; + @NonNull String field; + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java index 0361571414..5009a44a55 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java @@ -856,8 +856,9 @@ void rejectsNotFoundConstructorParameterForPrimitiveType() { org.bson.Document document = new org.bson.Document("foo", "bar"); - assertThatThrownBy(() -> converter.read(DefaultedConstructorArgument.class, document)) - .isInstanceOf(MappingInstantiationException.class); + assertThatThrownBy(() -> converter.read(DefaultedConstructorArgument.class, document)) // + .isInstanceOf(MappingException.class) // + .hasCauseInstanceOf(MappingInstantiationException.class); } @Test // DATAMONGO-358 @@ -1855,8 +1856,9 @@ void rejectsConversionFromStringToEnumBackedInterface() { org.bson.Document document = new org.bson.Document("property", InterfacedEnum.INSTANCE.name()); - assertThatExceptionOfType(ConverterNotFoundException.class) // - .isThrownBy(() -> converter.read(DocWithInterfacedEnum.class, document)); + assertThatThrownBy(() -> converter.read(DocWithInterfacedEnum.class, document)) // + .isInstanceOf(MappingException.class) // + .hasCauseInstanceOf(ConverterNotFoundException.class); } @Test // DATAMONGO-1898