DATAMONGO-1025 - Fix creation of nested named index.

We new prefix explicitly named indexes on nested types (eg. for embedded properties) with the path pointing to the property. This avoids errors having equally named index definitions on different paths pointing to the same type within one collection.

Along the way we harmonized index naming for geospatial index definitions where only the properties field name was taken into account where it should have been the full property path.

Original pull request: #219.

Author: christophstrobl

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

@@ -73,7 +73,43 @@
 	boolean dropDups() default false;
 
 	/**
-	 * The name of the index to be created.
+	 * The name of the index to be created. <br />
+	 * <br />
+	 * The name will only be applied as is when defined on root level. For usage on nested or embedded structures the
+	 * provided name will be prefixed with the path leading to the entity. <br />
+	 * <br />
+	 * The structure below
+	 * 
+	 * <pre>
+	 * <code>
+	 * &#64;Document
+	 * class Root {
+	 *   Hybrid hybrid;
+	 *   Nested nested;
+	 * }
+	 * 
+	 * &#64;Document
+	 * &#64;CompoundIndex(name = "compound_index", def = "{'h1': 1, 'h2': 1}")
+	 * class Hybrid {
+	 *   String h1, h2;
+	 * }
+	 * 
+	 * &#64;CompoundIndex(name = "compound_index", def = "{'n1': 1, 'n2': 1}")
+	 * class Nested {
+	 *   String n1, n2;
+	 * }
+	 * </code>
+	 * </pre>
+	 * 
+	 * resolves in the following index structures
+	 * 
+	 * <pre>
+	 * <code>
+	 * db.root.ensureIndex( { hybrid.h1: 1, hybrid.h2: 1 } , { name: "hybrid.compound_index" } )
+	 * db.root.ensureIndex( { nested.n1: 1, nested.n2: 1 } , { name: "nested.compound_index" } )
+	 * db.hybrid.ensureIndex( { h1: 1, h2: 1 } , { name: "compound_index" } )
+	 * </code>
+	 * </pre>
 	 * 
 	 * @return
 	 */

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

@@ -32,7 +32,41 @@
 public @interface GeoSpatialIndexed {
 
 	/**
-	 * Name of the property in the document that contains the [x, y] or radial coordinates to index.
+	 * Index name. <br />
+	 * <br />
+	 * The name will only be applied as is when defined on root level. For usage on nested or embedded structures the
+	 * provided name will be prefixed with the path leading to the entity. <br />
+	 * <br />
+	 * The structure below
+	 * 
+	 * <pre>
+	 * <code>
+	 * &#64;Document
+	 * class Root {
+	 *   Hybrid hybrid;
+	 *   Nested nested;
+	 * }
+	 * 
+	 * &#64;Document
+	 * class Hybrid {
+	 *   &#64;GeoSpatialIndexed(name="index") Point h1;
+	 * }
+	 * 
+	 * class Nested {
+	 *   &#64;GeoSpatialIndexed(name="index") Point n1;
+	 * }
+	 * </code>
+	 * </pre>
+	 * 
+	 * resolves in the following index structures
+	 * 
+	 * <pre>
+	 * <code>
+	 * db.root.ensureIndex( { hybrid.h1: "2d" } , { name: "hybrid.index" } )
+	 * db.root.ensureIndex( { nested.n1: "2d" } , { name: "nested.index" } )
+	 * db.hybrid.ensureIndex( { h1: "2d" } , { name: "index" } )
+	 * </code>
+	 * </pre>
 	 * 
 	 * @return
 	 */

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

@@ -58,7 +58,41 @@
 	boolean dropDups() default false;
 
 	/**
-	 * Index name.
+	 * Index name. <br />
+	 * <br />
+	 * The name will only be applied as is when defined on root level. For usage on nested or embedded structures the
+	 * provided name will be prefixed with the path leading to the entity. <br />
+	 * <br />
+	 * The structure below
+	 * 
+	 * <pre>
+	 * <code>
+	 * &#64;Document
+	 * class Root {
+	 *   Hybrid hybrid;
+	 *   Nested nested;
+	 * }
+	 * 
+	 * &#64;Document
+	 * class Hybrid {
+	 *   &#64;Indexed(name="index") String h1;
+	 * }
+	 * 
+	 * class Nested {
+	 *   &#64;Indexed(name="index") String n1;
+	 * }
+	 * </code>
+	 * </pre>
+	 * 
+	 * resolves in the following index structures
+	 * 
+	 * <pre>
+	 * <code>
+	 * db.root.ensureIndex( { hybrid.h1: 1 } , { name: "hybrid.index" } )
+	 * db.root.ensureIndex( { nested.n1: 1 } , { name: "nested.index" } )
+	 * db.hybrid.ensureIndex( { h1: 1} , { name: "index" } )
+	 * </code>
+	 * </pre>
 	 * 
 	 * @return
 	 */

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

@@ -25,7 +25,6 @@
 
 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;
 import org.springframework.data.mapping.PropertyHandler;
@@ -96,7 +95,7 @@ public List<IndexDefinitionHolder> resolveIndexForEntity(final MongoPersistentEn
 		Assert.notNull(document, "Given entity is not collection root.");
 
 		final List<IndexDefinitionHolder> indexInformation = new ArrayList<MongoPersistentEntityIndexResolver.IndexDefinitionHolder>();
-		indexInformation.addAll(potentiallyCreateCompoundIndexDefinitions("", root.getCollection(), root.getType()));
+		indexInformation.addAll(potentiallyCreateCompoundIndexDefinitions("", root.getCollection(), root));
 		indexInformation.addAll(potentiallyCreateTextIndexDefinition(root));
 
 		final CycleGuard guard = new CycleGuard();
@@ -138,10 +137,11 @@ public void doWithPersistentProperty(MongoPersistentProperty persistentProperty)
 	private List<IndexDefinitionHolder> resolveIndexForClass(final Class<?> type, final String path,
 			final String collection, final CycleGuard guard) {
 
+		MongoPersistentEntity<?> entity = mappingContext.getPersistentEntity(type);
+
 		final List<IndexDefinitionHolder> indexInformation = new ArrayList<MongoPersistentEntityIndexResolver.IndexDefinitionHolder>();
-		indexInformation.addAll(potentiallyCreateCompoundIndexDefinitions(path, collection, type));
+		indexInformation.addAll(potentiallyCreateCompoundIndexDefinitions(path, collection, entity));
 
-		MongoPersistentEntity<?> entity = mappingContext.getPersistentEntity(type);
 		entity.doWithProperties(new PropertyHandler<MongoPersistentProperty>() {
 
 			@Override
@@ -183,14 +183,13 @@ private IndexDefinitionHolder createIndexDefinitionHolderForProperty(String dotP
 	}
 
 	private List<IndexDefinitionHolder> potentiallyCreateCompoundIndexDefinitions(String dotPath, String collection,
-			Class<?> type) {
+			MongoPersistentEntity<?> entity) {
 
-		if (AnnotationUtils.findAnnotation(type, CompoundIndexes.class) == null
-				&& AnnotationUtils.findAnnotation(type, CompoundIndex.class) == null) {
+		if (entity.findAnnotation(CompoundIndexes.class) == null && entity.findAnnotation(CompoundIndex.class) == null) {
 			return Collections.emptyList();
 		}
 
-		return createCompoundIndexDefinitions(dotPath, collection, type);
+		return createCompoundIndexDefinitions(dotPath, collection, entity);
 	}
 
 	private Collection<? extends IndexDefinitionHolder> potentiallyCreateTextIndexDefinition(MongoPersistentEntity<?> root) {
@@ -278,35 +277,35 @@ public void doWithPersistentProperty(MongoPersistentProperty persistentProperty)
 	 * @return
 	 */
 	protected List<IndexDefinitionHolder> createCompoundIndexDefinitions(String dotPath, String fallbackCollection,
-			Class<?> type) {
+			MongoPersistentEntity<?> entity) {
 
 		List<IndexDefinitionHolder> indexDefinitions = new ArrayList<MongoPersistentEntityIndexResolver.IndexDefinitionHolder>();
-		CompoundIndexes indexes = AnnotationUtils.findAnnotation(type, CompoundIndexes.class);
+		CompoundIndexes indexes = entity.findAnnotation(CompoundIndexes.class);
 
 		if (indexes != null) {
 			for (CompoundIndex index : indexes.value()) {
-				indexDefinitions.add(createCompoundIndexDefinition(dotPath, fallbackCollection, index));
+				indexDefinitions.add(createCompoundIndexDefinition(dotPath, fallbackCollection, index, entity));
 			}
 		}
 
-		CompoundIndex index = AnnotationUtils.findAnnotation(type, CompoundIndex.class);
+		CompoundIndex index = entity.findAnnotation(CompoundIndex.class);
 
 		if (index != null) {
-			indexDefinitions.add(createCompoundIndexDefinition(dotPath, fallbackCollection, index));
+			indexDefinitions.add(createCompoundIndexDefinition(dotPath, fallbackCollection, index, entity));
 		}
 
 		return indexDefinitions;
 	}
 
 	@SuppressWarnings("deprecation")
 	protected IndexDefinitionHolder createCompoundIndexDefinition(String dotPath, String fallbackCollection,
-			CompoundIndex index) {
+			CompoundIndex index, MongoPersistentEntity<?> entity) {
 
 		CompoundIndexDefinition indexDefinition = new CompoundIndexDefinition(resolveCompoundIndexKeyFromStringDefinition(
 				dotPath, index.def()));
 
 		if (!index.useGeneratedName()) {
-			indexDefinition.named(index.name());
+			indexDefinition.named(pathAwareIndexName(index.name(), dotPath, null));
 		}
 
 		if (index.unique()) {
@@ -377,7 +376,7 @@ protected IndexDefinitionHolder createIndexDefinition(String dotPath, String fal
 				IndexDirection.ASCENDING.equals(index.direction()) ? Sort.Direction.ASC : Sort.Direction.DESC);
 
 		if (!index.useGeneratedName()) {
-			indexDefinition.named(StringUtils.hasText(index.name()) ? index.name() : dotPath);
+			indexDefinition.named(pathAwareIndexName(index.name(), dotPath, persitentProperty));
 		}
 
 		if (index.unique()) {
@@ -419,14 +418,31 @@ protected IndexDefinitionHolder createGeoSpatialIndexDefinition(String dotPath,
 		indexDefinition.withMin(index.min()).withMax(index.max());
 
 		if (!index.useGeneratedName()) {
-			indexDefinition.named(StringUtils.hasText(index.name()) ? index.name() : persistentProperty.getName());
+			indexDefinition.named(pathAwareIndexName(index.name(), dotPath, persistentProperty));
 		}
 
 		indexDefinition.typed(index.type()).withBucketSize(index.bucketSize()).withAdditionalField(index.additionalField());
 
 		return new IndexDefinitionHolder(dotPath, indexDefinition, collection);
 	}
 
+	private String pathAwareIndexName(String indexName, String dotPath, MongoPersistentProperty property) {
+
+		String nameToUse = StringUtils.hasText(indexName) ? indexName : "";
+
+		if (!StringUtils.hasText(dotPath) || (property != null && dotPath.equals(property.getFieldName()))) {
+			return StringUtils.hasText(nameToUse) ? nameToUse : dotPath;
+		}
+
+		if (StringUtils.hasText(dotPath)) {
+
+			nameToUse = StringUtils.hasText(nameToUse) ? (property != null ? dotPath.replace("." + property.getFieldName(),
+					"") : dotPath) + "." + nameToUse : dotPath;
+		}
+		return nameToUse;
+
+	}
+
 	/**
 	 * {@link CycleGuard} holds information about properties and the paths for accessing those. This information is used
 	 * to detect potential cycles within the references.

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

@@ -163,8 +163,8 @@ public void createsNotNestedGeoSpatialIndexCorrectly() {
 		new MongoPersistentEntityIndexCreator(mappingContext, factory);
 
 		assertThat(keysCaptor.getValue(), equalTo(new BasicDBObjectBuilder().add("company.address.location", "2d").get()));
-		assertThat(optionsCaptor.getValue(), equalTo(new BasicDBObjectBuilder().add("name", "location").add("min", -180)
-				.add("max", 180).add("bits", 26).get()));
+		assertThat(optionsCaptor.getValue(), equalTo(new BasicDBObjectBuilder().add("name", "company.address.location")
+				.add("min", -180).add("max", 180).add("bits", 26).get()));
 	}
 
 	/**

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

@@ -744,6 +744,60 @@ public String language() {
 					.resolveIndexForEntity(dummy);
 		}
 
+		/**
+		 * @see DATAMONGO-1025
+		 */
+		@Test
+		public void shouldUsePathIndexAsIndexNameForDocumentsHavingNamedNestedCompoundIndexFixedOnCollection() {
+
+			List<IndexDefinitionHolder> indexDefinitions = prepareMappingContextAndResolveIndexForType(DocumentWithNestedDocumentHavingNamedCompoundIndex.class);
+			assertThat((String) indexDefinitions.get(0).getIndexOptions().get("name"),
+					equalTo("propertyOfTypeHavingNamedCompoundIndex.c_index"));
+		}
+
+		/**
+		 * @see DATAMONGO-1025
+		 */
+		@Test
+		public void shouldUseIndexNameForNestedTypesWithNamedCompoundIndexDefinition() {
+
+			List<IndexDefinitionHolder> indexDefinitions = prepareMappingContextAndResolveIndexForType(DocumentWithNestedTypeHavingNamedCompoundIndex.class);
+			assertThat((String) indexDefinitions.get(0).getIndexOptions().get("name"),
+					equalTo("propertyOfTypeHavingNamedCompoundIndex.c_index"));
+		}
+
+		/**
+		 * @see DATAMONGO-1025
+		 */
+		@Test
+		public void shouldUsePathIndexAsIndexNameForDocumentsHavingNamedNestedIndexFixedOnCollection() {
+
+			List<IndexDefinitionHolder> indexDefinitions = prepareMappingContextAndResolveIndexForType(DocumentWithNestedDocumentHavingNamedIndex.class);
+			assertThat((String) indexDefinitions.get(0).getIndexOptions().get("name"),
+					equalTo("propertyOfTypeHavingNamedIndex.property_index"));
+		}
+
+		/**
+		 * @see DATAMONGO-1025
+		 */
+		@Test
+		public void shouldUseIndexNameForNestedTypesWithNamedIndexDefinition() {
+
+			List<IndexDefinitionHolder> indexDefinitions = prepareMappingContextAndResolveIndexForType(DocumentWithNestedTypeHavingNamedIndex.class);
+			assertThat((String) indexDefinitions.get(0).getIndexOptions().get("name"),
+					equalTo("propertyOfTypeHavingNamedIndex.property_index"));
+		}
+
+		/**
+		 * @see DATAMONGO-1025
+		 */
+		@Test
+		public void shouldUseIndexNameOnRootLevel() {
+
+			List<IndexDefinitionHolder> indexDefinitions = prepareMappingContextAndResolveIndexForType(DocumentWithNamedIndex.class);
+			assertThat((String) indexDefinitions.get(0).getIndexOptions().get("name"), equalTo("property_index"));
+		}
+
 		@Document
 		static class MixedIndexRoot {
 
@@ -836,6 +890,54 @@ static class SelfCyclingViaCollectionType {
 			List<SelfCyclingViaCollectionType> cyclic;
 
 		}
+
+		@Document
+		@CompoundIndex(name = "c_index", def = "{ foo:1, bar:1 }")
+		static class DocumentWithNamedCompoundIndex {
+
+			String property;
+		}
+
+		@Document
+		static class DocumentWithNamedIndex {
+
+			@Indexed(name = "property_index") String property;
+		}
+
+		static class TypeWithNamedIndex {
+
+			@Indexed(name = "property_index") String property;
+		}
+
+		@Document
+		static class DocumentWithNestedDocumentHavingNamedCompoundIndex {
+
+			DocumentWithNamedCompoundIndex propertyOfTypeHavingNamedCompoundIndex;
+		}
+
+		@CompoundIndex(name = "c_index", def = "{ foo:1, bar:1 }")
+		static class TypeWithNamedCompoundIndex {
+			String property;
+		}
+
+		@Document
+		static class DocumentWithNestedTypeHavingNamedCompoundIndex {
+
+			TypeWithNamedCompoundIndex propertyOfTypeHavingNamedCompoundIndex;
+		}
+
+		@Document
+		static class DocumentWithNestedDocumentHavingNamedIndex {
+
+			DocumentWithNamedIndex propertyOfTypeHavingNamedIndex;
+		}
+
+		@Document
+		static class DocumentWithNestedTypeHavingNamedIndex {
+
+			TypeWithNamedIndex propertyOfTypeHavingNamedIndex;
+		}
+
 	}
 
 	private static List<IndexDefinitionHolder> prepareMappingContextAndResolveIndexForType(Class<?> type) {