From c12a27a8f8711dd9a0a04b0108a245baa67d618b Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Tue, 10 Dec 2013 09:35:40 +0100 Subject: [PATCH] DATAMONGO-805 - Excluding DBRef field in a query causes a MappingException. Previously we tried to convert all DBRef associations into appropriate DBRef structures even if they were to be ignored. We now ignore excluded properties in DBRef associations correctly. Original pull request: #102. --- .../mongodb/core/convert/QueryMapper.java | 22 +++++- .../core/convert/QueryMapperUnitTests.java | 18 +++++ .../mongodb/core/mapping/MappingTests.java | 74 +++++++++++++++++++ 3 files changed, 111 insertions(+), 3 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 2ba9b8862..6f10a9a6f 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 @@ -200,13 +200,28 @@ public class QueryMapper { return getMappedKeyword(new Keyword((DBObject) value), null); } - if (documentField.isAssociation()) { + if (isAssociationConversionNecessary(documentField, value)) { return convertAssociation(value, documentField.getProperty()); } return convertSimpleOrDBObject(value, documentField.getPropertyEntity()); } + /** + * Returns whether the given {@link Field} represents an association reference that together with the given value + * requires conversion to a {@link org.springframework.data.mongodb.core.mapping.DBRef} object. We check whether the + * 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 value + * @return + */ + private boolean isAssociationConversionNecessary(Field documentField, Object value) { + return documentField.isAssociation() && value != null + && documentField.getProperty().getActualType().isAssignableFrom(value.getClass()); + } + /** * Retriggers mapping if the given source is a {@link DBObject} or simply invokes the * @@ -248,7 +263,8 @@ public class QueryMapper { */ private Object convertAssociation(Object source, MongoPersistentProperty property) { - if (property == null || !property.isAssociation()) { + if (property == null || !property.isAssociation() || source == null || source instanceof DBRef + || !property.isEntity()) { return source; } @@ -270,7 +286,7 @@ public class QueryMapper { return result; } - return source == null || source instanceof DBRef ? source : converter.toDBRef(source, property); + return converter.toDBRef(source, property); } /** diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java index 34cd8716d..3b69748c9 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java @@ -56,6 +56,7 @@ import com.mongodb.QueryBuilder; * * @author Oliver Gierke * @author Patryk Wasik + * @author Thomas Darimont */ @RunWith(MockitoJUnitRunner.class) public class QueryMapperUnitTests { @@ -466,6 +467,23 @@ public class QueryMapperUnitTests { assertThat(result.get("myvalue"), is((Object) "$center")); } + /** + * @DATAMONGO-805 + */ + @Test + public void shouldExcludeDBRefAssociation() { + + Query query = query(where("someString").is("foo")); + query.fields().exclude("reference"); + + BasicMongoPersistentEntity entity = context.getPersistentEntity(WithDBRef.class); + DBObject queryResult = mapper.getMappedObject(query.getQueryObject(), entity); + DBObject fieldsResult = mapper.getMappedObject(query.getFieldsObject(), entity); + + assertThat(queryResult.get("someString"), is((Object) "foo")); + assertThat(fieldsResult.get("reference"), is((Object) 0)); + } + class IdWrapper { Object id; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/MappingTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/MappingTests.java index bfd453517..b101ae6ba 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/MappingTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapping/MappingTests.java @@ -24,6 +24,7 @@ import static org.springframework.data.mongodb.core.query.Update.*; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -49,6 +50,7 @@ import com.mongodb.MongoException; /** * @author Jon Brisbin * @author Oliver Gierke + * @author Thomas Darimont */ public class MappingTests extends AbstractIntegrationTests { @@ -465,6 +467,57 @@ public class MappingTests extends AbstractIntegrationTests { assertThat(result.items.get(0).id, is(items.id)); } + /** + * @see DATAMONGO-805 + */ + @Test + public void supportExcludeDbRefAssociation() { + + template.dropCollection(Item.class); + template.dropCollection(Container.class); + + Item item = new Item(); + template.insert(item); + + Container container = new Container("foo"); + container.item = item; + + template.insert(container); + + Query query = new Query(Criteria.where("id").is("foo")); + query.fields().exclude("item"); + Container result = template.findOne(query, Container.class); + + assertThat(result, is(notNullValue())); + assertThat(result.item, is(nullValue())); + } + + /** + * @see DATAMONGO-805 + */ + @Test + public void shouldMapFieldsOfIterableEntity() { + + template.dropCollection(IterableItem.class); + template.dropCollection(Container.class); + + Item item = new IterableItem(); + item.value = "bar"; + template.insert(item); + + Container container = new Container("foo"); + container.item = item; + + template.insert(container); + + Query query = new Query(Criteria.where("id").is("foo")); + Container result = template.findOne(query, Container.class); + + assertThat(result, is(notNullValue())); + assertThat(result.item, is(notNullValue())); + assertThat(result.item.value, is("bar")); + } + static class Container { @Id final String id; @@ -473,6 +526,10 @@ public class MappingTests extends AbstractIntegrationTests { id = new ObjectId().toString(); } + public Container(String id) { + this.id = id; + } + @DBRef Item item; @DBRef List items; } @@ -480,9 +537,26 @@ public class MappingTests extends AbstractIntegrationTests { static class Item { @Id final String id; + String value; public Item() { this.id = new ObjectId().toString(); } } + + static class IterableItem extends Item implements Iterable { + + List data = new ArrayList(); + + @Override + public Iterator iterator() { + return data.iterator(); + } + } + + static class ItemData { + + String id; + String value; + } }