From 9a1e6226b122c3a40b0f009bbf9f631aa0cd8619 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 19 Jun 2012 08:59:08 +0200 Subject: [PATCH] DATAMONGO-424 - Don't resolve DBRefs eagerly if property type is DBRef. So far we have resolved DBRef values eagerly without inspecting the actual property type the would have to be assigned eventually. Now we simply skip the recursive resolve process if the property type (or component type, map value type) is DBRef actually. --- .../core/convert/MappingMongoConverter.java | 13 +++- .../MappingMongoConverterUnitTests.java | 78 +++++++++++++++++++ 2 files changed, 88 insertions(+), 3 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 b1ab52916..e5d90c5a3 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 @@ -730,15 +730,19 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App Collection items = targetType.getType().isArray() ? new ArrayList() : CollectionFactory .createCollection(collectionType, sourceValue.size()); TypeInformation componentType = targetType.getComponentType(); + Class rawComponentType = componentType == null ? null : componentType.getType(); for (int i = 0; i < sourceValue.size(); i++) { + Object dbObjItem = sourceValue.get(i); + if (dbObjItem instanceof DBRef) { - items.add(read(componentType, ((DBRef) dbObjItem).fetch(), parent)); + items.add(DBRef.class.equals(rawComponentType) ? dbObjItem : read(componentType, ((DBRef) dbObjItem).fetch(), + parent)); } else if (dbObjItem instanceof DBObject) { items.add(read(componentType, (DBObject) dbObjItem, parent)); } else { - items.add(getPotentiallyConvertedSimpleRead(dbObjItem, componentType == null ? null : componentType.getType())); + items.add(getPotentiallyConvertedSimpleRead(dbObjItem, rawComponentType)); } } @@ -776,9 +780,12 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App Object value = entry.getValue(); TypeInformation valueType = type.getMapValueType(); + Class rawValueType = valueType == null ? null : valueType.getType(); if (value instanceof DBObject) { map.put(key, read(valueType, (DBObject) value, parent)); + } else if (value instanceof DBRef) { + map.put(key, DBRef.class.equals(rawValueType) ? value : read(valueType, ((DBRef) value).fetch())); } else { Class valueClass = valueType == null ? null : valueType.getType(); map.put(key, getPotentiallyConvertedSimpleRead(value, valueClass)); @@ -939,7 +946,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App if (conversions.hasCustomReadTarget(value.getClass(), rawType)) { return (T) conversionService.convert(value, rawType); } else if (value instanceof DBRef) { - return (T) read(type, ((DBRef) value).fetch(), parent); + return (T) (rawType.equals(DBRef.class) ? value : read(type, ((DBRef) value).fetch(), parent)); } else if (value instanceof BasicDBList) { return (T) getPotentiallyConvertedSimpleRead(readCollectionOrArray(type, (BasicDBList) value, parent), rawType); } else if (value instanceof DBObject) { 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 49b96072b..2132bdea5 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 @@ -17,6 +17,7 @@ package org.springframework.data.mongodb.core.convert; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import static org.mockito.Mockito.*; import java.math.BigDecimal; import java.math.BigInteger; @@ -34,6 +35,7 @@ import java.util.Set; import java.util.SortedMap; import org.bson.types.ObjectId; +import org.hamcrest.Matcher; import org.joda.time.LocalDate; import org.junit.Before; import org.junit.Test; @@ -58,6 +60,7 @@ import org.springframework.test.util.ReflectionTestUtils; import com.mongodb.BasicDBList; import com.mongodb.BasicDBObject; +import com.mongodb.DB; import com.mongodb.DBObject; import com.mongodb.DBRef; @@ -1100,6 +1103,73 @@ public class MappingMongoConverterUnitTests { }); } + /** + * @see DATAMONGO-424 + */ + @Test + public void readsPlainDBRefObject() { + + DBRef dbRef = new DBRef(mock(DB.class), "foo", 2); + DBObject dbObject = new BasicDBObject("ref", dbRef); + + DBRefWrapper result = converter.read(DBRefWrapper.class, dbObject); + assertThat(result.ref, is(dbRef)); + } + + /** + * @see DATAMONGO-424 + */ + @Test + public void readsCollectionOfDBRefs() { + + DBRef dbRef = new DBRef(mock(DB.class), "foo", 2); + BasicDBList refs = new BasicDBList(); + refs.add(dbRef); + + DBObject dbObject = new BasicDBObject("refs", refs); + + DBRefWrapper result = converter.read(DBRefWrapper.class, dbObject); + assertThat(result.refs, hasSize(1)); + assertThat(result.refs, hasItem(dbRef)); + } + + /** + * @see DATAMONGO-424 + */ + @Test + public void readsDBRefMap() { + + DBRef dbRef = mock(DBRef.class); + BasicDBObject refMap = new BasicDBObject("foo", dbRef); + DBObject dbObject = new BasicDBObject("refMap", refMap); + + DBRefWrapper result = converter.read(DBRefWrapper.class, dbObject); + + assertThat(result.refMap.entrySet(), hasSize(1)); + assertThat(result.refMap.values(), hasItem(dbRef)); + } + + /** + * @see DATAMONGO-424 + */ + @Test + @SuppressWarnings({ "rawtypes", "unchecked" }) + public void resolvesDBRefMapValue() { + + DBRef dbRef = mock(DBRef.class); + when(dbRef.fetch()).thenReturn(new BasicDBObject()); + + BasicDBObject refMap = new BasicDBObject("foo", dbRef); + DBObject dbObject = new BasicDBObject("personMap", refMap); + + DBRefWrapper result = converter.read(DBRefWrapper.class, dbObject); + + Matcher isPerson = instanceOf(Person.class); + + assertThat(result.personMap.entrySet(), hasSize(1)); + assertThat(result.personMap.values(), hasItem(isPerson)); + } + private static void assertSyntheticFieldValueOf(Object target, Object expected) { for (int i = 0; i < 10; i++) { @@ -1265,6 +1335,14 @@ public class MappingMongoConverterUnitTests { Person person; } + static class DBRefWrapper { + + DBRef ref; + List refs; + Map refMap; + Map personMap; + } + private class LocalDateToDateConverter implements Converter { public Date convert(LocalDate source) {