From 164e94704569973b93349b4d1af76c593c2dca8c Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 5 May 2014 07:47:43 +0200 Subject: [PATCH] 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: #180. --- .../MongoPersistentEntityIndexResolver.java | 138 +++++++++++++++++- ...ersistentEntityIndexResolverUnitTests.java | 91 ++++++++++++ 2 files changed, 223 insertions(+), 6 deletions(-) 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) {