Skip to content

Commit c94b641

Browse files
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 96f6370 commit c94b641

File tree

3 files changed

+143
-22
lines changed

3 files changed

+143
-22
lines changed

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

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,7 @@ private Object readCollectionOrArray(TypeInformation<?> targetType, BasicDBList
901901
Collection<Object> items = targetType.getType().isArray() ? new ArrayList<Object>()
902902
: CollectionFactory.createCollection(collectionType, rawComponentType, sourceValue.size());
903903

904-
if (isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue) && !DBRef.class.equals(rawComponentType)) {
904+
if (!DBRef.class.equals(rawComponentType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceValue)) {
905905
return bulkReadAndConvertDBRefs((List<DBRef>) (ArrayList) sourceValue, componentType, path, rawComponentType);
906906
}
907907

@@ -921,27 +921,6 @@ private Object readCollectionOrArray(TypeInformation<?> targetType, BasicDBList
921921
return getPotentiallyConvertedSimpleRead(items, targetType.getType());
922922
}
923923

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-
945924
/**
946925
* Reads the given {@link DBObject} into a {@link Map}. will recursively resolve nested {@link Map}s as well.
947926
*
@@ -967,6 +946,11 @@ protected Map<Object, Object> readMap(TypeInformation<?> type, DBObject dbObject
967946
Map<Object, Object> map = CollectionFactory.createMap(mapType, rawKeyType, dbObject.keySet().size());
968947
Map<String, Object> sourceMap = dbObject.toMap();
969948

949+
if (!DBRef.class.equals(rawValueType) && isCollectionOfDbRefWhereBulkFetchIsPossible(sourceMap.values())) {
950+
bulkReadAndConvertDBRefMapIntoTarget(valueType, rawValueType, sourceMap, map);
951+
return map;
952+
}
953+
970954
for (Entry<String, Object> entry : sourceMap.entrySet()) {
971955
if (typeMapper.isTypeKey(entry.getKey())) {
972956
continue;
@@ -1247,6 +1231,21 @@ private <T> T readAndConvertDBRef(DBRef dbref, TypeInformation<?> type, ObjectPa
12471231
return CollectionUtils.isEmpty(result) ? null : result.iterator().next();
12481232
}
12491233

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

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

12831303
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)