diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DbRefResolver.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DbRefResolver.java index 8efedbee5..49cd715e3 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DbRefResolver.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DbRefResolver.java @@ -17,6 +17,7 @@ package org.springframework.data.mongodb.core.convert; import java.util.List; +import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; @@ -68,11 +69,13 @@ public interface DbRefResolver { DBObject fetch(DBRef dbRef); /** - * Loads a given {@link List} of {@link DBRef}s from the datasource in one batch.
+ * Loads a given {@link List} of {@link DBRef}s from the datasource in one batch. The resulting {@link List} of + * {@link DBObject} will reflect the ordering of the {@link DBRef} passed in.
* The {@link DBRef} elements in the list must not reference different collections. * * @param dbRefs must not be {@literal null}. * @return never {@literal null}. + * @throws InvalidDataAccessApiUsageException in case not all {@link DBRef} target the same collection. * @since 1.10 */ List bulkFetch(List dbRefs); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DefaultDbRefResolver.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DefaultDbRefResolver.java index 850a8c7ba..346eaea97 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DefaultDbRefResolver.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DefaultDbRefResolver.java @@ -147,7 +147,7 @@ public class DefaultDbRefResolver implements DbRefResolver { DB db = mongoDbFactory.getDb(); List result = db.getCollection(collection) .find(new BasicDBObjectBuilder().add("_id", new BasicDBObject("$in", ids)).get()).toArray(); - Collections.sort(result, new DbRefByReferencePositionComperator(ids)); + Collections.sort(result, new DbRefByReferencePositionComparator(ids)); return result; } @@ -445,11 +445,11 @@ public class DefaultDbRefResolver implements DbRefResolver { * @author Christoph Strobl * @since 1.10 */ - private static class DbRefByReferencePositionComperator implements Comparator { + private static class DbRefByReferencePositionComparator implements Comparator { List reference; - public DbRefByReferencePositionComperator(List referenceIds) { + public DbRefByReferencePositionComparator(List referenceIds) { reference = new ArrayList(referenceIds); } 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 ac220a2fc..dff32f66e 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 @@ -883,6 +883,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App * @param path must not be {@literal null}. * @return the converted {@link Collection} or array, will never be {@literal null}. */ + @SuppressWarnings({ "rawtypes", "unchecked" }) private Object readCollectionOrArray(TypeInformation targetType, BasicDBList sourceValue, ObjectPath path) { Assert.notNull(targetType, "Target type must not be null!"); @@ -901,8 +902,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App Collection items = targetType.getType().isArray() ? new ArrayList() : CollectionFactory.createCollection(collectionType, rawComponentType, sourceValue.size()); - if (isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue) && !DBRef.class.equals(rawComponentType)) { - return bulkReadAndConvertDBRefs((List) (ArrayList) sourceValue, componentType, path, rawComponentType); + if (!DBRef.class.equals(rawComponentType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue)) { + return bulkReadAndConvertDBRefs((List) (List) (sourceValue), componentType, path, rawComponentType); } for (Object dbObjItem : sourceValue) { @@ -921,27 +922,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return getPotentiallyConvertedSimpleRead(items, targetType.getType()); } - private boolean isCollectionOfDbRefWhereBulkFetchIsPossible(Collection source) { - - String collection = null; - - for (Object dbObjItem : source) { - - if (!(dbObjItem instanceof DBRef)) { - return false; - } - - DBRef ref = (DBRef) dbObjItem; - - if (collection != null && !collection.equals(ref.getCollectionName())) { - return false; - } - collection = ref.getCollectionName(); - } - - return true; - } - /** * Reads the given {@link DBObject} into a {@link Map}. will recursively resolve nested {@link Map}s as well. * @@ -967,6 +947,11 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App Map map = CollectionFactory.createMap(mapType, rawKeyType, dbObject.keySet().size()); Map sourceMap = dbObject.toMap(); + if (!DBRef.class.equals(rawValueType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceMap.values())) { + bulkReadAndConvertDBRefMapIntoTarget(valueType, rawValueType, sourceMap, map); + return map; + } + for (Entry entry : sourceMap.entrySet()) { if (typeMapper.isTypeKey(entry.getKey())) { continue; @@ -1247,6 +1232,21 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return CollectionUtils.isEmpty(result) ? null : result.iterator().next(); } + @SuppressWarnings({ "unchecked", "rawtypes" }) + private void bulkReadAndConvertDBRefMapIntoTarget(TypeInformation valueType, Class rawValueType, + Map sourceMap, Map targetMap) { + + LinkedHashMap referenceMap = new LinkedHashMap(sourceMap); + List convertedObjects = bulkReadAndConvertDBRefs((List) new ArrayList(referenceMap.values()), + valueType, ObjectPath.ROOT, rawValueType); + + int index = 0; + for (String key : referenceMap.keySet()) { + targetMap.put(key, convertedObjects.get(index)); + index++; + } + } + @SuppressWarnings("unchecked") private List bulkReadAndConvertDBRefs(List dbrefs, TypeInformation type, ObjectPath path, final Class rawType) { @@ -1278,6 +1278,27 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return targeList; } + private boolean isCollectionOfDbRefWhereBulkFetchIsPossible(Collection source) { + + String collection = null; + + for (Object dbObjItem : source) { + + if (!(dbObjItem instanceof DBRef)) { + return false; + } + + DBRef ref = (DBRef) dbObjItem; + + if (collection != null && !collection.equals(ref.getCollectionName())) { + return false; + } + collection = ref.getCollectionName(); + } + + return true; + } + private void maybeEmitEvent(MongoMappingEvent event) { if (canPublishEvent()) { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index 979f6c45b..e6d2bd049 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -33,6 +33,7 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -3416,6 +3417,31 @@ public class MongoTemplateTests { assertThat(target.getLazyDbRefAnnotatedList(), contains(two, one)); } + /** + * @see DATAMONGO-1194 + */ + @Test + public void shouldFetchMapOfLazyReferencesCorrectly() { + + Sample one = new Sample("1", "jon snow"); + Sample two = new Sample("2", "tyrion lannister"); + + template.save(one); + template.save(two); + + DocumentWithDBRefCollection source = new DocumentWithDBRefCollection(); + source.lazyDbRefAnnotatedMap = new LinkedHashMap(); + source.lazyDbRefAnnotatedMap.put("tyrion", two); + source.lazyDbRefAnnotatedMap.put("jon", one); + template.save(source); + + DocumentWithDBRefCollection target = template.findOne(query(where("id").is(source.id)), + DocumentWithDBRefCollection.class); + + assertThat(target.lazyDbRefAnnotatedMap, instanceOf(LazyLoadingProxy.class)); + assertThat(target.lazyDbRefAnnotatedMap.values(), contains(two, one)); + } + static class TypeWithNumbers { @Id String id; @@ -3490,6 +3516,9 @@ public class MongoTemplateTests { @Field("lazy_db_ref_list") /** @see DATAMONGO-1194 */ @org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) // public List lazyDbRefAnnotatedList; + + @Field("lazy_db_ref_map") /** @see DATAMONGO-1194 */ + @org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) public Map lazyDbRefAnnotatedMap; } static class DocumentWithCollection { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DbRefMappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DbRefMappingMongoConverterUnitTests.java index 9cf3619b1..75a033214 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DbRefMappingMongoConverterUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DbRefMappingMongoConverterUnitTests.java @@ -27,6 +27,7 @@ import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -641,12 +642,83 @@ public class DbRefMappingMongoConverterUnitTests { verify(converterSpy, never()).bulkReadRefs(anyListOf(DBRef.class)); } + /** + * @see DATAMONGO-1194 + */ + @Test + public void shouldBulkFetchMapOfReferences() { + + MapDBRefVal val1 = new MapDBRefVal(); + val1.id = BigInteger.ONE; + + MapDBRefVal val2 = new MapDBRefVal(); + val2.id = BigInteger.ZERO; + + MappingMongoConverter converterSpy = spy(converter); + doReturn(Arrays.asList(new BasicDBObject("_id", val1.id), new BasicDBObject("_id", val2.id))).when(converterSpy) + .bulkReadRefs(anyListOf(DBRef.class)); + + BasicDBObject dbo = new BasicDBObject(); + MapDBRef mapDBRef = new MapDBRef(); + mapDBRef.map = new LinkedHashMap(); + mapDBRef.map.put("one", val1); + mapDBRef.map.put("two", val2); + + converterSpy.write(mapDBRef, dbo); + + MapDBRef result = converterSpy.read(MapDBRef.class, dbo); + + // assertProxyIsResolved(result.map, false); + assertThat(result.map.get("one").id, is(val1.id)); + // assertProxyIsResolved(result.map, true); + assertThat(result.map.get("two").id, is(val2.id)); + + verify(converterSpy, times(1)).bulkReadRefs(anyListOf(DBRef.class)); + verify(converterSpy, never()).readRef(Mockito.any(DBRef.class)); + } + + /** + * @see DATAMONGO-1194 + */ + @Test + public void shouldBulkFetchLazyMapOfReferences() { + + MapDBRefVal val1 = new MapDBRefVal(); + val1.id = BigInteger.ONE; + + MapDBRefVal val2 = new MapDBRefVal(); + val2.id = BigInteger.ZERO; + + MappingMongoConverter converterSpy = spy(converter); + doReturn(Arrays.asList(new BasicDBObject("_id", val1.id), new BasicDBObject("_id", val2.id))).when(converterSpy) + .bulkReadRefs(anyListOf(DBRef.class)); + + BasicDBObject dbo = new BasicDBObject(); + MapDBRef mapDBRef = new MapDBRef(); + mapDBRef.lazyMap = new LinkedHashMap(); + mapDBRef.lazyMap.put("one", val1); + mapDBRef.lazyMap.put("two", val2); + + converterSpy.write(mapDBRef, dbo); + + MapDBRef result = converterSpy.read(MapDBRef.class, dbo); + + assertProxyIsResolved(result.lazyMap, false); + assertThat(result.lazyMap.get("one").id, is(val1.id)); + assertProxyIsResolved(result.lazyMap, true); + assertThat(result.lazyMap.get("two").id, is(val2.id)); + + verify(converterSpy, times(1)).bulkReadRefs(anyListOf(DBRef.class)); + verify(converterSpy, never()).readRef(Mockito.any(DBRef.class)); + } + private Object transport(Object result) { return SerializationUtils.deserialize(SerializationUtils.serialize(result)); } class MapDBRef { @org.springframework.data.mongodb.core.mapping.DBRef Map map; + @org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) Map lazyMap; } class MapDBRefVal {