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 Jun 15, 2021
1 parent c217618 commit e2d901f
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 @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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> BSON = ClassTypeInformation.from(Bson.class);

Expand Down Expand Up @@ -350,7 +345,11 @@ protected <S extends Object> S readDocument(ConversionContext context, Bson bson
throw new MappingException(String.format(INVALID_TYPE_TO_READ, document, rawType));
}

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

private ParameterValueProvider<MongoPersistentProperty> getParameterProvider(ConversionContext context,
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 @@ 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 @@ -145,6 +149,31 @@ 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 @@ -212,4 +241,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 @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e2d901f

Please sign in to comment.