diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolver.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolver.java index e50f14761..87a460343 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolver.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolver.java @@ -17,9 +17,15 @@ package org.springframework.data.mongodb.core.index; 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 @@ import com.mongodb.util.JSON; */ public class MongoPersistentEntityIndexResolver implements IndexResolver { + private static final Logger LOGGER = LoggerFactory.getLogger(MongoPersistentEntityIndexResolver.class); + private final MongoMappingContext mappingContext; /** @@ -88,6 +96,8 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { final List indexInformation = new ArrayList(); indexInformation.addAll(potentiallyCreateCompoundIndexDefinitions("", root.getCollection(), root.getType())); + final CycleGuard guard = new CycleGuard(); + root.doWithProperties(new PropertyHandler() { @Override @@ -95,7 +105,7 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { 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 class MongoPersistentEntityIndexResolver implements IndexResolver { * @return List of {@link IndexDefinitionHolder} representing indexes for given type and its referenced property * types. Will never be {@code null}. */ - private List resolveIndexForClass(Class type, final String path, final String collection) { + private List resolveIndexForClass(final Class type, final String path, + final String collection, final CycleGuard guard) { final List indexInformation = new ArrayList(); indexInformation.addAll(potentiallyCreateCompoundIndexDefinitions(path, collection, type)); @@ -130,9 +141,15 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { 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 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { 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> propertyTypeMap; + + CycleGuard() { + this.propertyTypeMap = new LinkedHashMap>(); + } + + /** + * @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 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 paths = new ArrayList(); + 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 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { */ 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 diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolverUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolverUnitTests.java index b8dcd9bd7..f9e26afd0 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolverUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexResolverUnitTests.java @@ -439,6 +439,55 @@ public class MongoPersistentEntityIndexResolverUnitTests { assertThat(indexDefinitions, empty()); } + /** + * @see DATAMONGO-926 + */ + @Test + public void shouldNotRunIntoStackOverflow() { + + List indexDefinitions = prepareMappingContextAndResolveIndexForType(CycleStartingInBetween.class); + assertThat(indexDefinitions, hasSize(1)); + } + + /** + * @see DATAMONGO-926 + */ + @Test + public void indexShouldBeFoundEvenForCyclePropertyReferenceOnLevelZero() { + + List 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 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 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 @@ public class MongoPersistentEntityIndexResolverUnitTests { @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 prepareMappingContextAndResolveIndexForType(Class type) {