From 063438002beeffee35b2cee6b29ee0e97a089aa5 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 2 Apr 2014 08:05:20 +0200 Subject: [PATCH] DATAMONGO-897 - Fixed potential NullPointerException in QueryMapper. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an association property points to an interface not containing the id property QueryMapper threw a NullPointerException in isAssociationConversionNecessary(…) as the lookup of the id property fails. We now check for the presence of an id property on the target type and check for assignability to indicated the need for conversion (usually in case when developers use raw ids in their update clauses, not the actual target instance. Original pull request: #164. --- .../mongodb/core/convert/QueryMapper.java | 25 +++++- .../core/convert/UpdateMapperUnitTests.java | 78 +++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java index 83abddb31..1237573ae 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java @@ -250,14 +250,31 @@ public class QueryMapper { * type of the given value is compatible with the type of the given document field in order to deal with potential * query field exclusions, since MongoDB uses the {@code int} {@literal 0} as an indicator for an excluded field. * - * @param documentField + * @param documentField must not be {@literal null}. * @param value * @return */ protected boolean isAssociationConversionNecessary(Field documentField, Object value) { - return documentField.isAssociation() && value != null - && (documentField.getProperty().getActualType().isAssignableFrom(value.getClass()) // - || documentField.getPropertyEntity().getIdProperty().getActualType().isAssignableFrom(value.getClass())); + + Assert.notNull(documentField, "Document field must not be null!"); + + if (value == null) { + return false; + } + + if (!documentField.isAssociation()) { + return false; + } + + Class type = value.getClass(); + MongoPersistentProperty property = documentField.getProperty(); + + if (property.getActualType().isAssignableFrom(type)) { + return true; + } + + MongoPersistentEntity entity = documentField.getPropertyEntity(); + return entity.hasIdProperty() && entity.getIdProperty().getActualType().isAssignableFrom(type); } /** diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java index fbc6dd719..6722b82b5 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java @@ -26,6 +26,7 @@ import java.util.List; import org.hamcrest.Matcher; import org.hamcrest.collection.IsIterableContainingInOrder; +import org.hamcrest.core.IsEqual; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -37,6 +38,7 @@ import org.springframework.data.annotation.Id; import org.springframework.data.convert.WritingConverter; import org.springframework.data.mapping.model.MappingException; import org.springframework.data.mongodb.MongoDbFactory; +import org.springframework.data.mongodb.core.DBObjectTestUtils; import org.springframework.data.mongodb.core.mapping.Field; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.query.Update; @@ -412,6 +414,82 @@ public class UpdateMapperUnitTests { assertThat(inClause, IsIterableContainingInOrder. contains(1L, 2L)); } + + /** + * @see DATAMONGO-897 + */ + @Test + public void updateOnDbrefPropertyOfInterfaceTypeWithoutExplicitGetterForIdShouldBeMappedCorrectly() { + + Update update = new Update().set("referencedDocument", new InterfaceDocumentDefinitionImpl("1", "Foo")); + DBObject mappedObject = mapper.getMappedObject(update.getUpdateObject(), + context.getPersistentEntity(DocumentWithReferenceToInterfaceImpl.class)); + + DBObject $set = DBObjectTestUtils.getAsDBObject(mappedObject, "$set"); + Object model = $set.get("referencedDocument"); + + DBRef expectedDBRef = new DBRef(factory.getDb(), "interfaceDocumentDefinitionImpl", "1"); + assertThat(model, allOf(instanceOf(DBRef.class), IsEqual. equalTo(expectedDBRef))); + } + + @org.springframework.data.mongodb.core.mapping.Document(collection = "DocumentWithReferenceToInterface") + static interface DocumentWithReferenceToInterface { + + String getId(); + + InterfaceDocumentDefinitionWithoutId getReferencedDocument(); + + } + + static interface InterfaceDocumentDefinitionWithoutId { + + String getValue(); + } + + static class InterfaceDocumentDefinitionImpl implements InterfaceDocumentDefinitionWithoutId { + + @Id String id; + String value; + + public InterfaceDocumentDefinitionImpl(String id, String value) { + + this.id = id; + this.value = value; + } + + @Override + public String getValue() { + return this.value; + } + + } + + static class DocumentWithReferenceToInterfaceImpl implements DocumentWithReferenceToInterface { + + private @Id String id; + + @org.springframework.data.mongodb.core.mapping.DBRef// + private InterfaceDocumentDefinitionWithoutId referencedDocument; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public void setModel(InterfaceDocumentDefinitionWithoutId referencedDocument) { + this.referencedDocument = referencedDocument; + } + + @Override + public InterfaceDocumentDefinitionWithoutId getReferencedDocument() { + return this.referencedDocument; + } + + } + static interface Model {} static class ModelImpl implements Model {