DATAMONGO-926 - Avoid StackOverflowError while resolving index structures.

We now guard cyclic non transient, non DBRef property references while inspecting domain types for potentially index structures. To do so we check on the properties path and owning type to determine potential cycles. If found we log a warn message pointing to path, entity and property potentially causing problems and skip processing for this path.

Original pull request: spring-projects#180.

Author: christophstrobl

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolver.java

@@ -17,9 +17,15 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.dao.InvalidDataAccessApiUsageException;
 import org.springframework.data.domain.Sort;
@@ -48,6 +54,8 @@
  */
 public class MongoPersistentEntityIndexResolver implements IndexResolver {
 
+	private static final Logger LOGGER = LoggerFactory.getLogger(MongoPersistentEntityIndexResolver.class);
+
 	private final MongoMappingContext mappingContext;
 
 	/**
@@ -88,14 +96,16 @@ public List<IndexDefinitionHolder> resolveIndexForEntity(final MongoPersistentEn
 		final List<IndexDefinitionHolder> indexInformation = new ArrayList<MongoPersistentEntityIndexResolver.IndexDefinitionHolder>();
 		indexInformation.addAll(potentiallyCreateCompoundIndexDefinitions("", root.getCollection(), root.getType()));
 
+		final CycleGuard guard = new CycleGuard();
+
 		root.doWithProperties(new PropertyHandler<MongoPersistentProperty>() {
 
 			@Override
 			public void doWithPersistentProperty(MongoPersistentProperty persistentProperty) {
 
 				if (persistentProperty.isEntity()) {
 					indexInformation.addAll(resolveIndexForClass(persistentProperty.getActualType(),
-							persistentProperty.getFieldName(), root.getCollection()));
+							persistentProperty.getFieldName(), root.getCollection(), guard));
 				}
 
 				IndexDefinitionHolder indexDefinitionHolder = createIndexDefinitionHolderForProperty(
@@ -118,7 +128,8 @@ public void doWithPersistentProperty(MongoPersistentProperty persistentProperty)
 	 * @return List of {@link IndexDefinitionHolder} representing indexes for given type and its referenced property
 	 *         types. Will never be {@code null}.
 	 */
-	private List<IndexDefinitionHolder> resolveIndexForClass(Class<?> type, final String path, final String collection) {
+	private List<IndexDefinitionHolder> resolveIndexForClass(final Class<?> type, final String path,
+			final String collection, final CycleGuard guard) {
 
 		final List<IndexDefinitionHolder> indexInformation = new ArrayList<MongoPersistentEntityIndexResolver.IndexDefinitionHolder>();
 		indexInformation.addAll(potentiallyCreateCompoundIndexDefinitions(path, collection, type));
@@ -130,9 +141,15 @@ private List<IndexDefinitionHolder> resolveIndexForClass(Class<?> type, final St
 			public void doWithPersistentProperty(MongoPersistentProperty persistentProperty) {
 
 				String propertyDotPath = (StringUtils.hasText(path) ? path + "." : "") + persistentProperty.getFieldName();
+				guard.protect(persistentProperty, path);
 
 				if (persistentProperty.isEntity()) {
-					indexInformation.addAll(resolveIndexForClass(persistentProperty.getActualType(), propertyDotPath, collection));
+					try {
+						indexInformation.addAll(resolveIndexForClass(persistentProperty.getActualType(), propertyDotPath,
+								collection, guard));
+					} catch (CyclicPropertyReferenceException e) {
+						LOGGER.warn(e.getMessage());
+					}
 				}
 
 				IndexDefinitionHolder indexDefinitionHolder = createIndexDefinitionHolderForProperty(propertyDotPath,
@@ -320,6 +337,115 @@ protected IndexDefinitionHolder createGeoSpatialIndexDefinition(String dotPath,
 		return new IndexDefinitionHolder(dotPath, indexDefinition, collection);
 	}
 
+	/**
+	 * {@link CycleGuard} holds information about properties and the paths for accessing those. This information is used
+	 * to detect potential cycles within the references.
+	 * 
+	 * @author Christoph Strobl
+	 */
+	private static class CycleGuard {
+
+		private final Map<String, List<Path>> propertyTypeMap;
+
+		CycleGuard() {
+			this.propertyTypeMap = new LinkedHashMap<String, List<Path>>();
+		}
+
+		/**
+		 * @param property The property to inspect
+		 * @param path The path under which the property can be reached.
+		 * @throws CyclicPropertyReferenceException in case a potential cycle is detected.
+		 */
+		void protect(MongoPersistentProperty property, String path) throws CyclicPropertyReferenceException {
+
+			String propertyTypeKey = createMapKey(property);
+			if (propertyTypeMap.containsKey(propertyTypeKey)) {
+
+				List<Path> paths = propertyTypeMap.get(propertyTypeKey);
+
+				for (Path existingPath : paths) {
+
+					if (existingPath.cycles(property)) {
+						paths.add(new Path(property, path));
+						throw new CyclicPropertyReferenceException(property.getFieldName(), property.getOwner().getType(),
+								existingPath.getPath());
+					}
+				}
+
+				paths.add(new Path(property, path));
+
+			} else {
+
+				ArrayList<Path> paths = new ArrayList<Path>();
+				paths.add(new Path(property, path));
+				propertyTypeMap.put(propertyTypeKey, paths);
+			}
+		}
+
+		private String createMapKey(MongoPersistentProperty property) {
+			return property.getOwner().getType().getSimpleName() + ":" + property.getFieldName();
+		}
+
+		private static class Path {
+
+			private final MongoPersistentProperty property;
+			private final String path;
+
+			Path(MongoPersistentProperty property, String path) {
+
+				this.property = property;
+				this.path = path;
+			}
+
+			public String getPath() {
+				return path;
+			}
+
+			boolean cycles(MongoPersistentProperty property) {
+
+				Pattern pattern = Pattern.compile("\\p{Punct}?" + Pattern.quote(property.getFieldName()) + "(\\p{Punct}|\\w)?");
+				Matcher matcher = pattern.matcher(path);
+
+				int count = 0;
+				while (matcher.find()) {
+					count++;
+				}
+
+				return count >= 1 && property.getOwner().getType().equals(this.property.getOwner().getType());
+			}
+		}
+	}
+
+	/**
+	 * @author Christoph Strobl
+	 * @since 1.5
+	 */
+	public static class CyclicPropertyReferenceException extends RuntimeException {
+
+		private static final long serialVersionUID = -3762979307658772277L;
+
+		private final String propertyName;
+		private final Class<?> type;
+		private final String dotPath;
+
+		public CyclicPropertyReferenceException(String propertyName, Class<?> type, String dotPath) {
+
+			this.propertyName = propertyName;
+			this.type = type;
+			this.dotPath = dotPath;
+		}
+
+		/*
+		 * (non-Javadoc)
+		 * @see java.lang.Throwable#getMessage()
+		 */
+		@Override
+		public String getMessage() {
+			return String.format("Found cycle for field '%s' in type '%s' for path '%s'", propertyName, type.getSimpleName(),
+					dotPath);
+		}
+	}
+
 	/**
 	 * Implementation of {@link IndexDefinition} holding additional (property)path information used for creating the
 	 * index. The path itself is the properties {@literal "dot"} path representation from its root document.
@@ -329,9 +455,9 @@ protected IndexDefinitionHolder createGeoSpatialIndexDefinition(String dotPath,
 	 */
 	public static class IndexDefinitionHolder implements IndexDefinition {
 
-		private String path;
-		private IndexDefinition indexDefinition;
-		private String collection;
+		private final String path;
+		private final IndexDefinition indexDefinition;
+		private final String collection;
 
 		/**
 		 * Create

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolverUnitTests.java

@@ -439,6 +439,55 @@ public void associationsShouldNotBeTraversed() {
 			assertThat(indexDefinitions, empty());
 		}
 
+		/**
+		 * @see DATAMONGO-926
+		 */
+		@Test
+		public void shouldNotRunIntoStackOverflow() {
+
+			List<IndexDefinitionHolder> indexDefinitions = prepareMappingContextAndResolveIndexForType(CycleStartingInBetween.class);
+			assertThat(indexDefinitions, hasSize(1));
+		}
+
+		/**
+		 * @see DATAMONGO-926
+		 */
+		@Test
+		public void indexShouldBeFoundEvenForCyclePropertyReferenceOnLevelZero() {
+
+			List<IndexDefinitionHolder> indexDefinitions = prepareMappingContextAndResolveIndexForType(CycleLevelZero.class);
+			assertIndexPathAndCollection("indexedProperty", "cycleLevelZero", indexDefinitions.get(0));
+			assertIndexPathAndCollection("cyclicReference.indexedProperty", "cycleLevelZero", indexDefinitions.get(1));
+			assertThat(indexDefinitions, hasSize(2));
+		}
+
+		/**
+		 * @see DATAMONGO-926
+		 */
+		@Test
+		public void indexShouldBeFoundEvenForCyclePropertyReferenceOnLevelOne() {
+
+			List<IndexDefinitionHolder> indexDefinitions = prepareMappingContextAndResolveIndexForType(CycleOnLevelOne.class);
+			assertIndexPathAndCollection("reference.indexedProperty", "cycleOnLevelOne", indexDefinitions.get(0));
+			assertIndexPathAndCollection("reference.cyclicReference.reference.indexedProperty", "cycleOnLevelOne",
+					indexDefinitions.get(1));
+			assertThat(indexDefinitions, hasSize(2));
+		}
+
+		/**
+		 * @see DATAMONGO-926
+		 */
+		@Test
+		public void indexBeResolvedCorrectlyWhenPropertiesOfDifferentTypesAreNamedEqually() {
+
+			List<IndexDefinitionHolder> indexDefinitions = prepareMappingContextAndResolveIndexForType(NoCycleButIdenticallyNamedProperties.class);
+			assertIndexPathAndCollection("foo", "noCycleButIdenticallyNamedProperties", indexDefinitions.get(0));
+			assertIndexPathAndCollection("reference.foo", "noCycleButIdenticallyNamedProperties", indexDefinitions.get(1));
+			assertIndexPathAndCollection("reference.deep.foo", "noCycleButIdenticallyNamedProperties",
+					indexDefinitions.get(2));
+			assertThat(indexDefinitions, hasSize(3));
+		}
+
 		@Document
 		static class MixedIndexRoot {
 
@@ -463,6 +512,48 @@ static class Inner {
 			@Indexed Outer outer;
 		}
 
+		@Document
+		static class CycleLevelZero {
+
+			@Indexed String indexedProperty;
+			CycleLevelZero cyclicReference;
+		}
+
+		@Document
+		static class CycleOnLevelOne {
+
+			CycleOnLevelOneReferenced reference;
+		}
+
+		static class CycleOnLevelOneReferenced {
+
+			@Indexed String indexedProperty;
+			CycleOnLevelOne cyclicReference;
+		}
+
+		@Document
+		public static class CycleStartingInBetween {
+
+			CycleOnLevelOne referenceToCycleStart;
+		}
+
+		@Document
+		static class NoCycleButIdenticallyNamedProperties {
+
+			@Indexed String foo;
+			NoCycleButIdenticallyNamedPropertiesNested reference;
+		}
+
+		static class NoCycleButIdenticallyNamedPropertiesNested {
+
+			@Indexed String foo;
+			NoCycleButIndenticallNamedPropertiesDeeplyNested deep;
+		}
+
+		static class NoCycleButIndenticallNamedPropertiesDeeplyNested {
+
+			@Indexed String foo;
+		}
 	}
 
 	private static List<IndexDefinitionHolder> prepareMappingContextAndResolveIndexForType(Class<?> type) {