Skip to content

Commit 9b9a8cc

Browse files
christophstroblchristophstrobl
committed
DATAMONGO-1194 - Improve DBRef resolution for maps.
We bulk load maps of referenced objects as long as they are stored in the same collection. This reduces db roundtrips and network traffic.
1 parent ceecdc4 commit 9b9a8cc

File tree

5 files changed

+152
-27
lines changed

5 files changed

+152
-27
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DbRefResolver.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import java.util.List;
1919

20+
import org.springframework.dao.InvalidDataAccessApiUsageException;
2021
import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity;
2122
import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty;
2223

@@ -68,11 +69,13 @@ DBRef createDbRef(org.springframework.data.mongodb.core.mapping.DBRef annotation
6869
DBObject fetch(DBRef dbRef);
6970

7071
/**
71-
* Loads a given {@link List} of {@link DBRef}s from the datasource in one batch. <br />
72+
* Loads a given {@link List} of {@link DBRef}s from the datasource in one batch. The resulting {@link List} of
73+
* {@link DBObject} will reflect the ordering of the {@link DBRef} passed in.<br />
7274
* The {@link DBRef} elements in the list must not reference different collections.
7375
*
7476
* @param dbRefs must not be {@literal null}.
7577
* @return never {@literal null}.
78+
* @throws InvalidDataAccessApiUsageException in case not all {@link DBRef} target the same collection.
7679
* @since 1.10
7780
*/
7881
List<DBObject> bulkFetch(List<DBRef> dbRefs);

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DefaultDbRefResolver.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public List<DBObject> bulkFetch(List<DBRef> refs) {
147147
DB db = mongoDbFactory.getDb();
148148
List<DBObject> result = db.getCollection(collection)
149149
.find(new BasicDBObjectBuilder().add("_id", new BasicDBObject("$in", ids)).get()).toArray();
150-
Collections.sort(result, new DbRefByReferencePositionComperator(ids));
150+
Collections.sort(result, new DbRefByReferencePositionComparator(ids));
151151
return result;
152152
}
153153

@@ -445,11 +445,11 @@ private synchronized Object resolve() {
445445
* @author Christoph Strobl
446446
* @since 1.10
447447
*/
448-
private static class DbRefByReferencePositionComperator implements Comparator<DBObject> {
448+
private static class DbRefByReferencePositionComparator implements Comparator<DBObject> {
449449

450450
List<Object> reference;
451451

452-
public DbRefByReferencePositionComperator(List<Object> referenceIds) {
452+
public DbRefByReferencePositionComparator(List<Object> referenceIds) {
453453
reference = new ArrayList<Object>(referenceIds);
454454
}
455455

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,7 @@ public Object getValueInternal(MongoPersistentProperty prop, DBObject dbo, SpELE
883883
* @param path must not be {@literal null}.
884884
* @return the converted {@link Collection} or array, will never be {@literal null}.
885885
*/
886+
@SuppressWarnings({ "rawtypes", "unchecked" })
886887
private Object readCollectionOrArray(TypeInformation<?> targetType, BasicDBList sourceValue, ObjectPath path) {
887888

888889
Assert.notNull(targetType, "Target type must not be null!");
@@ -901,8 +902,8 @@ private Object readCollectionOrArray(TypeInformation<?> targetType, BasicDBList
901902
Collection<Object> items = targetType.getType().isArray() ? new ArrayList<Object>()
902903
: CollectionFactory.createCollection(collectionType, rawComponentType, sourceValue.size());
903904

904-
if (isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue) && !DBRef.class.equals(rawComponentType)) {
905-
return bulkReadAndConvertDBRefs((List<DBRef>) (ArrayList) sourceValue, componentType, path, rawComponentType);
905+
if (!DBRef.class.equals(rawComponentType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue)) {
906+
return bulkReadAndConvertDBRefs((List<DBRef>) (List) (sourceValue), componentType, path, rawComponentType);
906907
}
907908

908909
for (Object dbObjItem : sourceValue) {
@@ -921,27 +922,6 @@ private Object readCollectionOrArray(TypeInformation<?> targetType, BasicDBList
921922
return getPotentiallyConvertedSimpleRead(items, targetType.getType());
922923
}
923924

924-
private boolean isCollectionOfDbRefWhereBulkFetchIsPossible(Collection<Object> source) {
925-
926-
String collection = null;
927-
928-
for (Object dbObjItem : source) {
929-
930-
if (!(dbObjItem instanceof DBRef)) {
931-
return false;
932-
}
933-
934-
DBRef ref = (DBRef) dbObjItem;
935-
936-
if (collection != null && !collection.equals(ref.getCollectionName())) {
937-
return false;
938-
}
939-
collection = ref.getCollectionName();
940-
}
941-
942-
return true;
943-
}
944-
945925
/**
946926
* Reads the given {@link DBObject} into a {@link Map}. will recursively resolve nested {@link Map}s as well.
947927
*
@@ -967,6 +947,11 @@ protected Map<Object, Object> readMap(TypeInformation<?> type, DBObject dbObject
967947
Map<Object, Object> map = CollectionFactory.createMap(mapType, rawKeyType, dbObject.keySet().size());
968948
Map<String, Object> sourceMap = dbObject.toMap();
969949

950+
if (!DBRef.class.equals(rawValueType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceMap.values())) {
951+
bulkReadAndConvertDBRefMapIntoTarget(valueType, rawValueType, sourceMap, map);
952+
return map;
953+
}
954+
970955
for (Entry<String, Object> entry : sourceMap.entrySet()) {
971956
if (typeMapper.isTypeKey(entry.getKey())) {
972957
continue;
@@ -1247,6 +1232,21 @@ private <T> T readAndConvertDBRef(DBRef dbref, TypeInformation<?> type, ObjectPa
12471232
return CollectionUtils.isEmpty(result) ? null : result.iterator().next();
12481233
}
12491234

1235+
@SuppressWarnings({ "unchecked", "rawtypes" })
1236+
private void bulkReadAndConvertDBRefMapIntoTarget(TypeInformation<?> valueType, Class<?> rawValueType,
1237+
Map<String, Object> sourceMap, Map<Object, Object> targetMap) {
1238+
1239+
LinkedHashMap<String, Object> referenceMap = new LinkedHashMap<String, Object>(sourceMap);
1240+
List<Object> convertedObjects = bulkReadAndConvertDBRefs((List<DBRef>) new ArrayList(referenceMap.values()),
1241+
valueType, ObjectPath.ROOT, rawValueType);
1242+
1243+
int index = 0;
1244+
for (String key : referenceMap.keySet()) {
1245+
targetMap.put(key, convertedObjects.get(index));
1246+
index++;
1247+
}
1248+
}
1249+
12501250
@SuppressWarnings("unchecked")
12511251
private <T> List<T> bulkReadAndConvertDBRefs(List<DBRef> dbrefs, TypeInformation<?> type, ObjectPath path,
12521252
final Class<?> rawType) {
@@ -1278,6 +1278,27 @@ private <T> List<T> bulkReadAndConvertDBRefs(List<DBRef> dbrefs, TypeInformation
12781278
return targeList;
12791279
}
12801280

1281+
private boolean isCollectionOfDbRefWhereBulkFetchIsPossible(Collection<Object> source) {
1282+
1283+
String collection = null;
1284+
1285+
for (Object dbObjItem : source) {
1286+
1287+
if (!(dbObjItem instanceof DBRef)) {
1288+
return false;
1289+
}
1290+
1291+
DBRef ref = (DBRef) dbObjItem;
1292+
1293+
if (collection != null && !collection.equals(ref.getCollectionName())) {
1294+
return false;
1295+
}
1296+
collection = ref.getCollectionName();
1297+
}
1298+
1299+
return true;
1300+
}
1301+
12811302
private void maybeEmitEvent(MongoMappingEvent<?> event) {
12821303

12831304
if (canPublishEvent()) {

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.Date;
3434
import java.util.HashMap;
3535
import java.util.HashSet;
36+
import java.util.LinkedHashMap;
3637
import java.util.List;
3738
import java.util.Locale;
3839
import java.util.Map;
@@ -3392,6 +3393,31 @@ public void shouldFetchListOfLazyReferencesCorrectly() {
33923393
assertThat(target.getLazyDbRefAnnotatedList(), contains(two, one));
33933394
}
33943395

3396+
/**
3397+
* @see DATAMONGO-1194
3398+
*/
3399+
@Test
3400+
public void shouldFetchMapOfLazyReferencesCorrectly() {
3401+
3402+
Sample one = new Sample("1", "jon snow");
3403+
Sample two = new Sample("2", "tyrion lannister");
3404+
3405+
template.save(one);
3406+
template.save(two);
3407+
3408+
DocumentWithDBRefCollection source = new DocumentWithDBRefCollection();
3409+
source.lazyDbRefAnnotatedMap = new LinkedHashMap<String, Sample>();
3410+
source.lazyDbRefAnnotatedMap.put("tyrion", two);
3411+
source.lazyDbRefAnnotatedMap.put("jon", one);
3412+
template.save(source);
3413+
3414+
DocumentWithDBRefCollection target = template.findOne(query(where("id").is(source.id)),
3415+
DocumentWithDBRefCollection.class);
3416+
3417+
assertThat(target.lazyDbRefAnnotatedMap, instanceOf(LazyLoadingProxy.class));
3418+
assertThat(target.lazyDbRefAnnotatedMap.values(), contains(two, one));
3419+
}
3420+
33953421
static class TypeWithNumbers {
33963422

33973423
@Id String id;
@@ -3466,6 +3492,9 @@ static class DocumentWithDBRefCollection {
34663492
@Field("lazy_db_ref_list") /** @see DATAMONGO-1194 */
34673493
@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) //
34683494
public List<Sample> lazyDbRefAnnotatedList;
3495+
3496+
@Field("lazy_db_ref_map") /** @see DATAMONGO-1194 */
3497+
@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) public Map<String, Sample> lazyDbRefAnnotatedMap;
34693498
}
34703499

34713500
static class DocumentWithCollection {

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DbRefMappingMongoConverterUnitTests.java

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.ArrayList;
2828
import java.util.Arrays;
2929
import java.util.HashMap;
30+
import java.util.LinkedHashMap;
3031
import java.util.LinkedList;
3132
import java.util.List;
3233
import java.util.Map;
@@ -641,12 +642,83 @@ public void shouldFallbackToOneByOneFetchingWhenElementsInListOfReferencesPointT
641642
verify(converterSpy, never()).bulkReadRefs(anyListOf(DBRef.class));
642643
}
643644

645+
/**
646+
* @see DATAMONGO-1194
647+
*/
648+
@Test
649+
public void shouldBulkFetchMapOfReferences() {
650+
651+
MapDBRefVal val1 = new MapDBRefVal();
652+
val1.id = BigInteger.ONE;
653+
654+
MapDBRefVal val2 = new MapDBRefVal();
655+
val2.id = BigInteger.ZERO;
656+
657+
MappingMongoConverter converterSpy = spy(converter);
658+
doReturn(Arrays.asList(new BasicDBObject("_id", val1.id), new BasicDBObject("_id", val2.id))).when(converterSpy)
659+
.bulkReadRefs(anyListOf(DBRef.class));
660+
661+
BasicDBObject dbo = new BasicDBObject();
662+
MapDBRef mapDBRef = new MapDBRef();
663+
mapDBRef.map = new LinkedHashMap<String, MapDBRefVal>();
664+
mapDBRef.map.put("one", val1);
665+
mapDBRef.map.put("two", val2);
666+
667+
converterSpy.write(mapDBRef, dbo);
668+
669+
MapDBRef result = converterSpy.read(MapDBRef.class, dbo);
670+
671+
// assertProxyIsResolved(result.map, false);
672+
assertThat(result.map.get("one").id, is(val1.id));
673+
// assertProxyIsResolved(result.map, true);
674+
assertThat(result.map.get("two").id, is(val2.id));
675+
676+
verify(converterSpy, times(1)).bulkReadRefs(anyListOf(DBRef.class));
677+
verify(converterSpy, never()).readRef(Mockito.any(DBRef.class));
678+
}
679+
680+
/**
681+
* @see DATAMONGO-1194
682+
*/
683+
@Test
684+
public void shouldBulkFetchLazyMapOfReferences() {
685+
686+
MapDBRefVal val1 = new MapDBRefVal();
687+
val1.id = BigInteger.ONE;
688+
689+
MapDBRefVal val2 = new MapDBRefVal();
690+
val2.id = BigInteger.ZERO;
691+
692+
MappingMongoConverter converterSpy = spy(converter);
693+
doReturn(Arrays.asList(new BasicDBObject("_id", val1.id), new BasicDBObject("_id", val2.id))).when(converterSpy)
694+
.bulkReadRefs(anyListOf(DBRef.class));
695+
696+
BasicDBObject dbo = new BasicDBObject();
697+
MapDBRef mapDBRef = new MapDBRef();
698+
mapDBRef.lazyMap = new LinkedHashMap<String, MapDBRefVal>();
699+
mapDBRef.lazyMap.put("one", val1);
700+
mapDBRef.lazyMap.put("two", val2);
701+
702+
converterSpy.write(mapDBRef, dbo);
703+
704+
MapDBRef result = converterSpy.read(MapDBRef.class, dbo);
705+
706+
assertProxyIsResolved(result.lazyMap, false);
707+
assertThat(result.lazyMap.get("one").id, is(val1.id));
708+
assertProxyIsResolved(result.lazyMap, true);
709+
assertThat(result.lazyMap.get("two").id, is(val2.id));
710+
711+
verify(converterSpy, times(1)).bulkReadRefs(anyListOf(DBRef.class));
712+
verify(converterSpy, never()).readRef(Mockito.any(DBRef.class));
713+
}
714+
644715
private Object transport(Object result) {
645716
return SerializationUtils.deserialize(SerializationUtils.serialize(result));
646717
}
647718

648719
class MapDBRef {
649720
@org.springframework.data.mongodb.core.mapping.DBRef Map<String, MapDBRefVal> map;
721+
@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) Map<String, MapDBRefVal> lazyMap;
650722
}
651723

652724
class MapDBRefVal {

0 commit comments

Comments
 (0)