DATAMONGO-1194 - Polishing.

Some missing JavaDoc and slight code polish.

Original pull request: #377.

Author: odrotbohm

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

@@ -440,19 +440,31 @@ private synchronized Object resolve() {
 
 	/**
 	 * {@link Comparator} for sorting {@link DBObject} that have been loaded in random order by a predefined list of
-	 * reference ids.
+	 * reference identifiers.
 	 *
 	 * @author Christoph Strobl
+	 * @author Oliver Gierke
 	 * @since 1.10
 	 */
 	private static class DbRefByReferencePositionComparator implements Comparator<DBObject> {
 
-		List<Object> reference;
+		private final List<Object> reference;
 
+		/**
+		 * Creates a new {@link DbRefByReferencePositionComparator} for the given list of reference identifiers.
+		 * 
+		 * @param referenceIds must not be {@literal null}.
+		 */
 		public DbRefByReferencePositionComparator(List<Object> referenceIds) {
-			reference = new ArrayList<Object>(referenceIds);
+
+			Assert.notNull(referenceIds, "Reference identifiers must not be null!");
+			this.reference = new ArrayList<Object>(referenceIds);
 		}
 
+		/*
+		 * (non-Javadoc)
+		 * @see java.util.Comparator#compare(java.lang.Object, java.lang.Object)
+		 */
 		@Override
 		public int compare(DBObject o1, DBObject o2) {
 			return Integer.compare(reference.indexOf(o1.get("_id")), reference.indexOf(o2.get("_id")));

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

@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -1239,8 +1240,8 @@ private void bulkReadAndConvertDBRefMapIntoTarget(TypeInformation<?> valueType,
 		LinkedHashMap<String, Object> referenceMap = new LinkedHashMap<String, Object>(sourceMap);
 		List<Object> convertedObjects = bulkReadAndConvertDBRefs((List<DBRef>) new ArrayList(referenceMap.values()),
 				valueType, ObjectPath.ROOT, rawValueType);
-
 		int index = 0;
+
 		for (String key : referenceMap.keySet()) {
 			targetMap.put(key, convertedObjects.get(index));
 			index++;
@@ -1255,9 +1256,9 @@ private <T> List<T> bulkReadAndConvertDBRefs(List<DBRef> dbrefs, TypeInformation
 			return Collections.emptyList();
 		}
 
-		final List<DBObject> referencedRawDocuments = dbrefs.size() == 1
+		List<DBObject> referencedRawDocuments = dbrefs.size() == 1
 				? Collections.singletonList(readRef(dbrefs.iterator().next())) : bulkReadRefs(dbrefs);
-		final String collectionName = dbrefs.iterator().next().getCollectionName();
+		String collectionName = dbrefs.iterator().next().getCollectionName();
 
 		List<T> targeList = new ArrayList<T>(dbrefs.size());
 
@@ -1278,27 +1279,6 @@ private <T> List<T> bulkReadAndConvertDBRefs(List<DBRef> dbrefs, TypeInformation
 		return targeList;
 	}
 
-	private boolean isCollectionOfDbRefWhereBulkFetchIsPossible(Collection<Object> 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()) {
@@ -1331,6 +1311,34 @@ List<DBObject> bulkReadRefs(List<DBRef> references) {
 		return dbRefResolver.bulkFetch(references);
 	}
 
+	/**
+	 * Returns whether the given {@link Iterable} contains {@link DBRef} instances all pointing to the same collection.
+	 * 
+	 * @param source must not be {@literal null}.
+	 * @return
+	 */
+	private static boolean isCollectionOfDbRefWhereBulkFetchIsPossible(Iterable<Object> source) {
+
+		Assert.notNull(source, "Iterable of DBRefs must not be null!");
+
+		Set<String> collectionsFound = new HashSet<String>();
+
+		for (Object dbObjItem : source) {
+
+			if (!(dbObjItem instanceof DBRef)) {
+				return false;
+			}
+
+			collectionsFound.add(((DBRef) dbObjItem).getCollectionName());
+
+			if (collectionsFound.size() > 1) {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
 	/**
 	 * Marker class used to indicate we have a non root document object here that might be used within an update - so we
 	 * need to preserve type hints for potential nested elements but need to remove it on top level.

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2015 the original author or authors.
+ * Copyright 2013-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -200,7 +200,8 @@ public void lazyLoadingProxyForLazyDbRefOnConcreteCollection() {
 
 		BasicDBObject dbo = new BasicDBObject();
 		ClassWithLazyDbRefs lazyDbRefs = new ClassWithLazyDbRefs();
-		lazyDbRefs.dbRefToConcreteCollection = new ArrayList<LazyDbRefTarget>(Arrays.asList(new LazyDbRefTarget(id, value)));
+		lazyDbRefs.dbRefToConcreteCollection = new ArrayList<LazyDbRefTarget>(
+				Arrays.asList(new LazyDbRefTarget(id, value)));
 		converterSpy.write(lazyDbRefs, dbo);
 
 		ClassWithLazyDbRefs result = converterSpy.read(ClassWithLazyDbRefs.class, dbo);
@@ -248,8 +249,8 @@ public void lazyLoadingProxyForLazyDbRefOnConcreteTypeWithPersistenceConstructor
 
 		BasicDBObject dbo = new BasicDBObject();
 		ClassWithLazyDbRefs lazyDbRefs = new ClassWithLazyDbRefs();
-		lazyDbRefs.dbRefToConcreteTypeWithPersistenceConstructor = new LazyDbRefTargetWithPeristenceConstructor(
-				(Object) id, (Object) value);
+		lazyDbRefs.dbRefToConcreteTypeWithPersistenceConstructor = new LazyDbRefTargetWithPeristenceConstructor((Object) id,
+				(Object) value);
 		converterSpy.write(lazyDbRefs, dbo);
 
 		ClassWithLazyDbRefs result = converterSpy.read(ClassWithLazyDbRefs.class, dbo);
@@ -733,18 +734,23 @@ static class ClassWithLazyDbRefs {
 
 		@Id String id;
 		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) List<LazyDbRefTarget> dbRefToInterface;
-		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) ArrayList<LazyDbRefTarget> dbRefToConcreteCollection;
+		@org.springframework.data.mongodb.core.mapping.DBRef(
+				lazy = true) ArrayList<LazyDbRefTarget> dbRefToConcreteCollection;
 		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) LazyDbRefTarget dbRefToConcreteType;
-		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) LazyDbRefTargetPropertyAccess dbRefToConcreteTypeWithPropertyAccess;
-		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) LazyDbRefTargetWithPeristenceConstructor dbRefToConcreteTypeWithPersistenceConstructor;
-		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) LazyDbRefTargetWithPeristenceConstructorWithoutDefaultConstructor dbRefToConcreteTypeWithPersistenceConstructorWithoutDefaultConstructor;
+		@org.springframework.data.mongodb.core.mapping.DBRef(
+				lazy = true) LazyDbRefTargetPropertyAccess dbRefToConcreteTypeWithPropertyAccess;
+		@org.springframework.data.mongodb.core.mapping.DBRef(
+				lazy = true) LazyDbRefTargetWithPeristenceConstructor dbRefToConcreteTypeWithPersistenceConstructor;
+		@org.springframework.data.mongodb.core.mapping.DBRef(
+				lazy = true) LazyDbRefTargetWithPeristenceConstructorWithoutDefaultConstructor dbRefToConcreteTypeWithPersistenceConstructorWithoutDefaultConstructor;
 	}
 
 	static class SerializableClassWithLazyDbRefs implements Serializable {
 
 		private static final long serialVersionUID = 1L;
 
-		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) SerializableLazyDbRefTarget dbRefToSerializableTarget;
+		@org.springframework.data.mongodb.core.mapping.DBRef(
+				lazy = true) SerializableLazyDbRefTarget dbRefToSerializableTarget;
 	}
 
 	static class LazyDbRefTarget implements Serializable {
@@ -901,9 +907,12 @@ public boolean equals(Object obj) {
 	static class WithObjectMethodOverrideLazyDbRefs {
 
 		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) LazyDbRefTarget dbRefToPlainObject;
-		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) ToStringObjectMethodOverrideLazyDbRefTarget dbRefToToStringObjectMethodOverride;
-		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) EqualsAndHashCodeObjectMethodOverrideLazyDbRefTarget dbRefEqualsAndHashcodeObjectMethodOverride2;
-		@org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) EqualsAndHashCodeObjectMethodOverrideLazyDbRefTarget dbRefEqualsAndHashcodeObjectMethodOverride1;
+		@org.springframework.data.mongodb.core.mapping.DBRef(
+				lazy = true) ToStringObjectMethodOverrideLazyDbRefTarget dbRefToToStringObjectMethodOverride;
+		@org.springframework.data.mongodb.core.mapping.DBRef(
+				lazy = true) EqualsAndHashCodeObjectMethodOverrideLazyDbRefTarget dbRefEqualsAndHashcodeObjectMethodOverride2;
+		@org.springframework.data.mongodb.core.mapping.DBRef(
+				lazy = true) EqualsAndHashCodeObjectMethodOverrideLazyDbRefTarget dbRefEqualsAndHashcodeObjectMethodOverride1;
 	}
 
 	class ClassWithDbRefField {

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

@@ -15,8 +15,8 @@
  */
 package org.springframework.data.mongodb.core.convert;
 
-import static org.hamcrest.collection.IsIterableContainingInOrder.contains;
-import static org.hamcrest.collection.IsIterableWithSize.*;
+import static org.hamcrest.Matchers.*;
+import static org.hamcrest.Matchers.contains;
 import static org.junit.Assert.*;
 import static org.mockito.Matchers.*;
 import static org.mockito.Mockito.*;
@@ -30,6 +30,7 @@
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.mongodb.MongoDbFactory;
@@ -43,7 +44,10 @@
 import com.mongodb.DBRef;
 
 /**
+ * Unit tests for {@link DefaultDbRefResolver}.
+ * 
  * @author Christoph Strobl
+ * @author Oliver Gierke
  */
 @RunWith(MockitoJUnitRunner.class)
 public class DefaultDbRefResolverUnitTests {
@@ -59,8 +63,8 @@ public void setUp() {
 
 		when(factoryMock.getDb()).thenReturn(dbMock);
 		when(dbMock.getCollection(anyString())).thenReturn(collectionMock);
-		when(collectionMock.find(any(DBObject.class))).thenReturn(cursorMock);
-		when(cursorMock.toArray()).thenReturn(Collections.<DBObject> emptyList());
+		when(collectionMock.find(Mockito.any(DBObject.class))).thenReturn(cursorMock);
+		when(cursorMock.toArray()).thenReturn(Collections.<DBObject>emptyList());
 
 		resolver = new DefaultDbRefResolver(factoryMock);
 	}
@@ -105,9 +109,9 @@ public void bulkFetchShouldThrowExceptionWhenUsingDifferntCollectionsWithinSetOf
 	@Test
 	public void bulkFetchShouldReturnEarlyForEmptyLists() {
 
-		resolver.bulkFetch(Collections.<DBRef> emptyList());
+		resolver.bulkFetch(Collections.<DBRef>emptyList());
 
-		verify(collectionMock, never()).find(any(DBObject.class));
+		verify(collectionMock, never()).find(Mockito.any(DBObject.class));
 	}
 
 	/**