From 118f007ca66c571041bb64ebb3c07c306e8835a8 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 25 Jun 2014 13:27:13 +0200 Subject: [PATCH] DATAMONGO-963 - @CompoundIndex should treat expireAfterSeconds correctly. We added an additional check on the fields used as key, so that TTL is ignored for CompoundIndex with more than one field (which effectively renders it useless on @CompoundIndex at all). Prior to this change potentially invalid index structures would have been created for e.g. @CompoundIndex(def = "{'foo': 1, 'bar': 1}", expireAfterSeconds=10) leading to MongoDB not being able to clean up the indexes (logs: "ERROR: key for ttl index can only have 1 field") This fix is related to https://jira.mongodb.org/browse/SERVER-10075. Original pull request: #196. --- .../mongodb/core/index/CompoundIndex.java | 3 ++ .../MongoPersistentEntityIndexResolver.java | 11 ++++-- ...ersistentEntityIndexResolverUnitTests.java | 35 +++++++++++++++---- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/CompoundIndex.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/CompoundIndex.java index a3cdf34af..55fee13fa 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/CompoundIndex.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/CompoundIndex.java @@ -107,8 +107,11 @@ public @interface CompoundIndex { /** * Configures the number of seconds after which the collection should expire. Defaults to -1 for no expiry. * + * @deprecated TTL cannot be defined for {@link CompoundIndex} having more than one field as key. Will be removed in + * 1.6. * @see http://docs.mongodb.org/manual/tutorial/expire-data/ * @return */ + @Deprecated int expireAfterSeconds() default -1; } 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 09cf78a41..6da85c086 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 @@ -215,6 +215,7 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { return indexDefinitions; } + @SuppressWarnings("deprecation") protected IndexDefinitionHolder createCompoundIndexDefinition(String dotPath, String fallbackCollection, CompoundIndex index) { @@ -237,8 +238,14 @@ public class MongoPersistentEntityIndexResolver implements IndexResolver { indexDefinition.background(); } - if (index.expireAfterSeconds() >= 0) { - indexDefinition.expire(index.expireAfterSeconds(), TimeUnit.SECONDS); + int ttl = index.expireAfterSeconds(); + + if (ttl >= 0) { + if (indexDefinition.getIndexKeys().keySet().size() > 1) { + LOGGER.warn("TTL is not supported for compound index with more than one key. TTL={} will be ignored.", ttl); + } else { + indexDefinition.expire(ttl, TimeUnit.SECONDS); + } } String collection = StringUtils.hasText(index.collection()) ? index.collection() : fallbackCollection; 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 691c172af..2eb1ff9a1 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 @@ -289,9 +289,8 @@ public class MongoPersistentEntityIndexResolverUnitTests { List indexDefinitions = prepareMappingContextAndResolveIndexForType(CompoundIndexOnLevelZero.class); IndexDefinition indexDefinition = indexDefinitions.get(0).getIndexDefinition(); - assertThat(indexDefinition.getIndexOptions(), - equalTo(new BasicDBObjectBuilder().add("name", "compound_index").add("unique", true).add("dropDups", true) - .add("sparse", true).add("background", true).add("expireAfterSeconds", 10L).get())); + assertThat(indexDefinition.getIndexOptions(), equalTo(new BasicDBObjectBuilder().add("name", "compound_index") + .add("unique", true).add("dropDups", true).add("sparse", true).add("background", true).get())); assertThat(indexDefinition.getIndexKeys(), equalTo(new BasicDBObjectBuilder().add("foo", 1).add("bar", -1).get())); } @@ -304,9 +303,8 @@ public class MongoPersistentEntityIndexResolverUnitTests { List indexDefinitions = prepareMappingContextAndResolveIndexForType(IndexDefinedOnSuperClass.class); IndexDefinition indexDefinition = indexDefinitions.get(0).getIndexDefinition(); - assertThat(indexDefinition.getIndexOptions(), - equalTo(new BasicDBObjectBuilder().add("name", "compound_index").add("unique", true).add("dropDups", true) - .add("sparse", true).add("background", true).add("expireAfterSeconds", 10L).get())); + assertThat(indexDefinition.getIndexOptions(), equalTo(new BasicDBObjectBuilder().add("name", "compound_index") + .add("unique", true).add("dropDups", true).add("sparse", true).add("background", true).get())); assertThat(indexDefinition.getIndexKeys(), equalTo(new BasicDBObjectBuilder().add("foo", 1).add("bar", -1).get())); } @@ -322,7 +320,7 @@ public class MongoPersistentEntityIndexResolverUnitTests { assertThat( indexDefinition.getIndexOptions(), equalTo(new BasicDBObjectBuilder().add("unique", true).add("dropDups", true).add("sparse", true) - .add("background", true).add("expireAfterSeconds", 10L).get())); + .add("background", true).get())); assertThat(indexDefinition.getIndexKeys(), equalTo(new BasicDBObjectBuilder().add("foo", 1).add("bar", -1).get())); } @@ -364,6 +362,22 @@ public class MongoPersistentEntityIndexResolverUnitTests { assertIndexPathAndCollection(new String[] { "foo", "bar" }, "CompoundIndexOnLevelZero", indexDefinitions.get(0)); } + /** + * @see DATAMONGO-963 + */ + @Test + public void compoundIndexShouldIncludeTTLWhenConsistingOfOnlyOneKey() { + + List indexDefinitions = prepareMappingContextAndResolveIndexForType(CompoundIndexWithOnlyOneKeyAndTTL.class); + + IndexDefinition indexDefinition = indexDefinitions.get(0).getIndexDefinition(); + assertThat( + indexDefinition.getIndexOptions(), + equalTo(new BasicDBObjectBuilder().add("unique", true).add("dropDups", true).add("sparse", true) + .add("background", true).add("expireAfterSeconds", 10L).get())); + assertThat(indexDefinition.getIndexKeys(), equalTo(new BasicDBObjectBuilder().add("foo", 1).get())); + } + @Document(collection = "CompoundIndexOnLevelOne") static class CompoundIndexOnLevelOne { @@ -400,6 +414,13 @@ public class MongoPersistentEntityIndexResolverUnitTests { static class ComountIndexWithAutogeneratedName { } + + @Document(collection = "CompoundIndexWithOnlyOneKeyAndTTL") + @CompoundIndex(def = "{'foo': 1}", background = true, dropDups = true, expireAfterSeconds = 10, sparse = true, + unique = true) + static class CompoundIndexWithOnlyOneKeyAndTTL { + + } } public static class MixedIndexResolutionTests {