From 31fbf5c942f7b5f279911635372971f2b67ba7db Mon Sep 17 00:00:00 2001 From: Piotr Kubowicz Date: Tue, 28 Apr 2020 08:47:33 +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 3ca5e2156c..0f4e887c19 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 @@ -41,6 +41,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.convert.TypeMapper; @@ -51,15 +52,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.MongoPersistentEntity; @@ -94,11 +87,13 @@ * @author Mark Paluch * @author Roman Puchkovskiy * @author Heesu Jung + * @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"; protected static final Logger LOGGER = LoggerFactory.getLogger(MappingMongoConverter.class); @@ -314,7 +309,11 @@ private S read(TypeInformation type, Bson bson, ObjectPath throw new MappingException(String.format(INVALID_TYPE_TO_READ, target, typeToUse.getType())); } - return read((MongoPersistentEntity) entity, target, path); + try { + return read((MongoPersistentEntity) entity, target, path); + } catch (ConversionException | MappingInstantiationException e) { + throw new MappingException(String.format(CANNOT_READ, target), e); + } } private ParameterValueProvider getParameterProvider(MongoPersistentEntity entity, 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 aa4e485f0b..4d60dc8337 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 @@ public 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(); @@ -147,6 +151,31 @@ public 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); @@ -214,4 +243,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 82a6840f9c..2068fc952b 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 @@ -851,8 +851,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 @@ -1850,8 +1851,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