Skip to content

Commit

Permalink
DATAMONGO-2511 Give context to mapping exceptions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pkubowicz committed Apr 28, 2020
1 parent 26ddf9d commit 31fbf5c
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -314,7 +309,11 @@ private <S extends Object> S read(TypeInformation<S> type, Bson bson, ObjectPath
throw new MappingException(String.format(INVALID_TYPE_TO_READ, target, typeToUse.getType()));
}

return read((MongoPersistentEntity<S>) entity, target, path);
try {
return read((MongoPersistentEntity<S>) entity, target, path);
} catch (ConversionException | MappingInstantiationException e) {
throw new MappingException(String.format(CANNOT_READ, target), e);
}
}

private ParameterValueProvider<MongoPersistentProperty> getParameterProvider(MongoPersistentEntity<?> entity,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -55,6 +57,7 @@
* Integration tests for {@link MappingMongoConverter}.
*
* @author Christoph Strobl
* @author Piotr Kubowicz
*/
@ExtendWith(MongoClientExtension.class)
public class MappingMongoConverterTests {
Expand All @@ -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();

Expand Down Expand Up @@ -147,6 +151,31 @@ public void readJavaTimeValuesWrittenViaCodec() {
.isEqualTo(source);
}

@Test // DATAMONGO-2511
public void reportConversionFailedExceptionContext() {

configureConverterWithNativeJavaTimeCodec();
MongoCollection<Document> 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<Document> 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);
Expand Down Expand Up @@ -214,4 +243,10 @@ Document toDocument() {
.append("localDateTime", localDateTime);
}
}

@Value
static class WithNonnullField {
@Id String id;
@NonNull String field;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 31fbf5c

Please sign in to comment.