From 916b856e972f99be449802d106505234b90bebc1 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 28 Apr 2014 17:36:45 +0200 Subject: [PATCH] DATAMONGO-899 - Polished API of new index creation abstractions. Removed the introduction of the IndexDefinition being collection aware again. The collection an index is created in is now held in the IndexDefinitionHolder. This is mostly due to the fact that the IndexDefinition implementations can be used with MongoTemplate and the index opoerations take a collection alongside the index definition. Made the IndexResolver API package protected so that we can further change it going forward. We should think about deprecating the collectionName attributes on index annotations as it doesn't make too much sense to manually configure the collection name for the indexes as the collection is predefined through the domain type setting here. This would allow us to remove the entire collection handling code inside the IndexResolver implementation. Turned IndexDefinitionHolder into a value object. Original pull request: #168. --- .../core/index/CompoundIndexDefinition.java | 16 +++++- .../mongodb/core/index/GeospatialIndex.java | 18 ------ .../data/mongodb/core/index/Index.java | 19 ------- .../mongodb/core/index/IndexDefinition.java | 8 --- .../mongodb/core/index/IndexResolver.java | 6 +- .../MongoPersistentEntityIndexCreator.java | 14 +++-- .../MongoPersistentEntityIndexResolver.java | 55 +++++++------------ ...PersistentEntityIndexCreatorUnitTests.java | 2 +- ...ersistentEntityIndexResolverUnitTests.java | 1 - 9 files changed, 47 insertions(+), 92 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/CompoundIndexDefinition.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/CompoundIndexDefinition.java index 890d3cf64..c5d76be3f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/CompoundIndexDefinition.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/CompoundIndexDefinition.java @@ -15,10 +15,14 @@ */ package org.springframework.data.mongodb.core.index; +import org.springframework.util.Assert; + import com.mongodb.BasicDBObject; import com.mongodb.DBObject; /** + * Index definition to span multiple keys. + * * @author Christoph Strobl * @since 1.5 */ @@ -26,10 +30,21 @@ public class CompoundIndexDefinition extends Index { private DBObject keys; + /** + * Creates a new {@link CompoundIndexDefinition} for the given keys. + * + * @param keys must not be {@literal null}. + */ public CompoundIndexDefinition(DBObject keys) { + + Assert.notNull(keys, "Keys must not be null!"); this.keys = keys; } + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.core.index.Index#getIndexKeys() + */ @Override public DBObject getIndexKeys() { @@ -38,5 +53,4 @@ public class CompoundIndexDefinition extends Index { dbo.putAll(super.getIndexKeys()); return dbo; } - } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/GeospatialIndex.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/GeospatialIndex.java index 3a14f6551..40b548334 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/GeospatialIndex.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/GeospatialIndex.java @@ -39,7 +39,6 @@ public class GeospatialIndex implements IndexDefinition { private GeoSpatialIndexType type = GeoSpatialIndexType.GEO_2D; private Double bucketSize = 1.0; private String additionalField; - private String collection; /** * Creates a new {@link GeospatialIndex} for the given field. @@ -122,23 +121,6 @@ public class GeospatialIndex implements IndexDefinition { return this; } - /* - * (non-Javadoc) - * @see org.springframework.data.mongodb.core.index.IndexDefinition#getCollection() - */ - @Override - public String getCollection() { - return collection; - } - - /** - * @param collection - * @since 1.5 - */ - public void setCollection(String collection) { - this.collection = collection; - } - public DBObject getIndexKeys() { DBObject dbo = new BasicDBObject(); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/Index.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/Index.java index b12fed883..73c7b454b 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/Index.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/Index.java @@ -51,8 +51,6 @@ public class Index implements IndexDefinition { private long expire = -1; - private String collection; - public Index() {} public Index(String key, Direction direction) { @@ -154,23 +152,6 @@ public class Index implements IndexDefinition { return this; } - /* - * (non-Javadoc) - * @see org.springframework.data.mongodb.core.index.IndexDefinition#getCollection() - */ - @Override - public String getCollection() { - return collection; - } - - /** - * @param collection - * @since 1.5 - */ - public void setCollection(String collection) { - this.collection = collection; - } - /** * @see http://docs.mongodb.org/manual/core/index-creation/#index-creation-duplicate-dropping * @param duplicates diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexDefinition.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexDefinition.java index ea28c3b68..42c0152b9 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexDefinition.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexDefinition.java @@ -27,12 +27,4 @@ public interface IndexDefinition { DBObject getIndexKeys(); DBObject getIndexOptions(); - - /** - * Get the collection name for the index. - * - * @return - * @since 1.5 - */ - String getCollection(); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexResolver.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexResolver.java index 2f829fe54..b444d1da8 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexResolver.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/IndexResolver.java @@ -15,13 +15,15 @@ */ package org.springframework.data.mongodb.core.index; +import org.springframework.data.mongodb.core.index.MongoPersistentEntityIndexResolver.IndexDefinitionHolder; + /** * {@link IndexResolver} finds those {@link IndexDefinition}s to be created for a given class. * * @author Christoph Strobl * @since 1.5 */ -public interface IndexResolver { +interface IndexResolver { /** * Find and create {@link IndexDefinition}s for properties of given {@code type}. {@link IndexDefinition}s are created @@ -30,6 +32,6 @@ public interface IndexResolver { * @param type * @return Empty {@link Iterable} in case no {@link IndexDefinition} could be resolved for type. */ - Iterable resolveIndexForClass(Class type); + Iterable resolveIndexForClass(Class type); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreator.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreator.java index d0760eb0c..d6f73b2b1 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreator.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreator.java @@ -25,6 +25,7 @@ import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.context.MappingContextEvent; import org.springframework.data.mongodb.MongoDbFactory; +import org.springframework.data.mongodb.core.index.MongoPersistentEntityIndexResolver.IndexDefinitionHolder; import org.springframework.data.mongodb.core.mapping.Document; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; @@ -106,7 +107,9 @@ public class MongoPersistentEntityIndexCreator implements } private void checkForIndexes(final MongoPersistentEntity entity) { - final Class type = entity.getType(); + + Class type = entity.getType(); + if (!classesSeen.containsKey(type)) { this.classesSeen.put(type, Boolean.TRUE); @@ -119,18 +122,18 @@ public class MongoPersistentEntityIndexCreator implements } } - protected void checkForAndCreateIndexes(MongoPersistentEntity entity) { + private void checkForAndCreateIndexes(MongoPersistentEntity entity) { if (entity.findAnnotation(Document.class) != null) { - for (IndexDefinition indexToCreate : indexResolver.resolveIndexForClass(entity.getType())) { + for (IndexDefinitionHolder indexToCreate : indexResolver.resolveIndexForClass(entity.getType())) { createIndex(indexToCreate); } } } - protected void createIndex(IndexDefinition indexDefinition) { + private void createIndex(IndexDefinitionHolder indexDefinition) { mongoDbFactory.getDb().getCollection(indexDefinition.getCollection()) - .ensureIndex(indexDefinition.getIndexKeys(), indexDefinition.getIndexOptions()); + .createIndex(indexDefinition.getIndexKeys(), indexDefinition.getIndexOptions()); } /** @@ -142,5 +145,4 @@ public class MongoPersistentEntityIndexCreator implements public boolean isIndexCreatorFor(MappingContext context) { return this.mappingContext.equals(context); } - } 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 78a94b294..29b3ed8e9 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 @@ -126,7 +126,7 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { @Override public void doWithPersistentProperty(MongoPersistentProperty persistentProperty) { - String propertyDotPath = (StringUtils.hasText(path) ? (path + ".") : "") + persistentProperty.getFieldName(); + String propertyDotPath = (StringUtils.hasText(path) ? path + "." : "") + persistentProperty.getFieldName(); if (persistentProperty.isEntity()) { indexInformation.addAll(resolveIndexForClass(persistentProperty.getActualType(), propertyDotPath, collection)); @@ -169,11 +169,12 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { * Create {@link IndexDefinition} wrapped in {@link IndexDefinitionHolder} for {@link CompoundIndexes} of given type. * * @param dotPath The properties {@literal "dot"} path representation from its document root. - * @param collection + * @param fallbackCollection * @param type * @return */ - protected List createCompoundIndexDefinitions(String dotPath, String collection, Class type) { + protected List createCompoundIndexDefinitions(String dotPath, String fallbackCollection, + Class type) { CompoundIndexes indexes = AnnotationUtils.findAnnotation(type, CompoundIndexes.class); List indexDefinitions = new ArrayList( @@ -181,12 +182,11 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { for (CompoundIndex index : indexes.value()) { - IndexDefinitionHolder holder = new IndexDefinitionHolder(StringUtils.hasText(index.name()) ? index.name() - : dotPath); + String path = StringUtils.hasText(index.name()) ? index.name() : dotPath; + String collection = StringUtils.hasText(index.collection()) ? index.collection() : fallbackCollection; CompoundIndexDefinition indexDefinition = new CompoundIndexDefinition((DBObject) JSON.parse(index.def())); indexDefinition.named(index.name()); - indexDefinition.setCollection(StringUtils.hasText(index.collection()) ? index.collection() : collection); if (index.unique()) { indexDefinition.unique(index.dropDups() ? Duplicates.DROP : Duplicates.RETAIN); } @@ -200,8 +200,7 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { indexDefinition.expire(index.expireAfterSeconds(), TimeUnit.SECONDS); } - holder.setIndexDefinition(indexDefinition); - indexDefinitions.add(holder); + indexDefinitions.add(new IndexDefinitionHolder(path, indexDefinition, collection)); } return indexDefinitions; @@ -216,15 +215,13 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { * @param persitentProperty * @return */ - protected IndexDefinitionHolder createIndexDefinition(String dotPath, String collection, + protected IndexDefinitionHolder createIndexDefinition(String dotPath, String fallbackCollection, MongoPersistentProperty persitentProperty) { Indexed index = persitentProperty.findAnnotation(Indexed.class); - - IndexDefinitionHolder holder = new IndexDefinitionHolder(dotPath); + String collection = StringUtils.hasText(index.collection()) ? index.collection() : fallbackCollection; Index indexDefinition = new Index(); - indexDefinition.setCollection(StringUtils.hasText(index.collection()) ? index.collection() : collection); indexDefinition.named(StringUtils.hasText(index.name()) ? index.name() : persitentProperty.getFieldName()); indexDefinition.on(persitentProperty.getFieldName(), IndexDirection.ASCENDING.equals(index.direction()) ? Sort.Direction.ASC : Sort.Direction.DESC); @@ -242,8 +239,7 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { indexDefinition.expire(index.expireAfterSeconds(), TimeUnit.SECONDS); } - holder.setIndexDefinition(indexDefinition); - return holder; + return new IndexDefinitionHolder(dotPath, indexDefinition, collection); } /** @@ -255,22 +251,19 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { * @param persistentProperty * @return */ - protected IndexDefinitionHolder createGeoSpatialIndexDefinition(String dotPath, String collection, + protected IndexDefinitionHolder createGeoSpatialIndexDefinition(String dotPath, String fallbackCollection, MongoPersistentProperty persistentProperty) { GeoSpatialIndexed index = persistentProperty.findAnnotation(GeoSpatialIndexed.class); - - IndexDefinitionHolder holder = new IndexDefinitionHolder(dotPath); + String collection = StringUtils.hasText(index.collection()) ? index.collection() : fallbackCollection; GeospatialIndex indexDefinition = new GeospatialIndex(dotPath); - indexDefinition.setCollection(StringUtils.hasText(index.collection()) ? index.collection() : collection); indexDefinition.withBits(index.bits()); indexDefinition.withMin(index.min()).withMax(index.max()); indexDefinition.named(StringUtils.hasText(index.name()) ? index.name() : persistentProperty.getName()); indexDefinition.typed(index.type()).withBucketSize(index.bucketSize()).withAdditionalField(index.additionalField()); - holder.setIndexDefinition(indexDefinition); - return holder; + return new IndexDefinitionHolder(dotPath, indexDefinition, collection); } /** @@ -284,23 +277,22 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { private String path; private IndexDefinition indexDefinition; + private String collection; /** * Create * * @param path */ - public IndexDefinitionHolder(String path) { + public IndexDefinitionHolder(String path, IndexDefinition definition, String collection) { + this.path = path; + this.indexDefinition = definition; + this.collection = collection; } - /* - * (non-Javadoc) - * @see org.springframework.data.mongodb.core.index.IndexDefinition#getCollection() - */ - @Override public String getCollection() { - return indexDefinition != null ? indexDefinition.getCollection() : null; + return collection; } /** @@ -321,15 +313,6 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { return indexDefinition; } - /** - * Set the {@literal raw} {@link IndexDefinition}. - * - * @param indexDefinition - */ - public void setIndexDefinition(IndexDefinition indexDefinition) { - this.indexDefinition = indexDefinition; - } - /* * (non-Javadoc) * @see org.springframework.data.mongodb.core.index.IndexDefinition#getIndexKeys() diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorUnitTests.java index a5591f73d..bc8b33645 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorUnitTests.java @@ -74,7 +74,7 @@ public class MongoPersistentEntityIndexCreatorUnitTests { Mockito.when(factory.getDb()).thenReturn(db); Mockito.when(db.getCollection(collectionCaptor.capture())).thenReturn(collection); - Mockito.doNothing().when(collection).ensureIndex(keysCaptor.capture(), optionsCaptor.capture()); + Mockito.doNothing().when(collection).createIndex(keysCaptor.capture(), optionsCaptor.capture()); } @Test 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 162beadac..91f673bea 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 @@ -324,7 +324,6 @@ public class MongoPersistentEntityIndexResolverUnitTests { List indexDefinitions = prepareMappingContextAndResolveIndexForType(Inner.class); assertThat(indexDefinitions, hasSize(1)); - assertThat(indexDefinitions.get(0).getIndexDefinition().getCollection(), equalTo("inner")); assertThat(indexDefinitions.get(0).getIndexDefinition().getIndexKeys(), equalTo(new BasicDBObjectBuilder().add("outer", 1).get())); }