From 95cb7e38be78eb7574d69fd9a522cb84f60f2677 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Wed, 7 Dec 2016 15:57:00 -0500 Subject: [PATCH 1/3] when collecting index definitions, look at the parent classes as well as the nested types for definitions. don't create indexes when the mapped class is abstract. fixes #1086 --- .../java/org/mongodb/morphia/IndexHelper.java | 51 +++++++++++-------- .../mongodb/morphia/mapping/MappedClass.java | 21 ++++++++ .../org/mongodb/morphia/IndexHelperTest.java | 38 +++++++++----- 3 files changed, 74 insertions(+), 36 deletions(-) diff --git a/morphia/src/main/java/org/mongodb/morphia/IndexHelper.java b/morphia/src/main/java/org/mongodb/morphia/IndexHelper.java index 6f86962b163..e9efd196ee2 100644 --- a/morphia/src/main/java/org/mongodb/morphia/IndexHelper.java +++ b/morphia/src/main/java/org/mongodb/morphia/IndexHelper.java @@ -167,7 +167,8 @@ private List collectNestedIndexes(final MappedClass mc, final List indexes = collectIndexes(aClass, parents); + for (Index index : indexes) { List fields = new ArrayList(); for (Field field : index.fields()) { fields.add(new FieldBuilder() @@ -189,29 +190,33 @@ private List collectNestedIndexes(final MappedClass mc, final List collectTopLevelIndexes(final MappedClass mc) { List list = new ArrayList(); - final List annotations = mc.getAnnotations(Indexes.class); - if (annotations != null) { - for (final Indexes indexes : annotations) { - for (final Index index : indexes.value()) { - Index updated = index; - if (index.fields().length == 0) { - LOG.warning(format("This index on '%s' is using deprecated configuration options. Please update to use the " - + "fields value on @Index: %s", mc.getClazz().getName(), index.toString())); - updated = new IndexBuilder() - .migrate(index); - } - List fields = new ArrayList(); - for (Field field : updated.fields()) { - fields.add(new FieldBuilder() - .value(findField(mc, index.options(), asList(field.value().split("\\.")))) - .type(field.type()) - .weight(field.weight())); - } + if (mc != null) { + final List annotations = mc.getAnnotations(Indexes.class); + if (annotations != null) { + for (final Indexes indexes : annotations) { + for (final Index index : indexes.value()) { + Index updated = index; + if (index.fields().length == 0) { + LOG.warning(format("This index on '%s' is using deprecated configuration options. Please update to use the " + + "fields value on @Index: %s", mc.getClazz().getName(), index.toString())); + updated = new IndexBuilder() + .migrate(index); + } + List fields = new ArrayList(); + for (Field field : updated.fields()) { + fields.add(new FieldBuilder() + .value(findField(mc, index.options(), asList(field.value().split("\\.")))) + .type(field.type()) + .weight(field.weight())); + } - list.add(replaceFields(updated, fields)); + list.add(replaceFields(updated, fields)); + } } } + list.addAll(collectTopLevelIndexes(mc.getSuperClass())); } + return list; } @@ -359,8 +364,10 @@ String findField(final MappedClass mc, final IndexOptions options, final ListemptyList())) { - createIndex(collection, mc, index, background); + if (!mc.isInterface() && !mc.isAbstract()) { + for (Index index : collectIndexes(mc, Collections.emptyList())) { + createIndex(collection, mc, index, background); + } } } diff --git a/morphia/src/main/java/org/mongodb/morphia/mapping/MappedClass.java b/morphia/src/main/java/org/mongodb/morphia/mapping/MappedClass.java index ae848eeb1e7..3960b63d5dd 100644 --- a/morphia/src/main/java/org/mongodb/morphia/mapping/MappedClass.java +++ b/morphia/src/main/java/org/mongodb/morphia/mapping/MappedClass.java @@ -148,6 +148,17 @@ public MappedClass(final Class clazz, final Mapper mapper) { } } + /** + * This is an internal method subject to change without notice. + * + * @return the parent class of this type if there is one null otherwise + * + * @since 1.3 + */ + public MappedClass getSuperClass() { + return superClass; + } + /** * @return true if the MappedClass is an interface @@ -156,6 +167,16 @@ public boolean isInterface() { return clazz.isInterface(); } + /** + * This is an internal method subject to change without notice. + * + * @return true if the MappedClass is abstract + * @since 1.3 + */ + public boolean isAbstract() { + return Modifier.isAbstract(clazz.getModifiers()); + } + /** * Checks to see if it a Map/Set/List or a property supported by the MongoDB java driver * diff --git a/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java b/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java index 75f85faea88..c2ba723a3df 100644 --- a/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java +++ b/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java @@ -17,6 +17,7 @@ package org.mongodb.morphia; import com.mongodb.BasicDBObject; +import com.mongodb.DBCollection; import com.mongodb.DBObject; import com.mongodb.client.MongoCollection; import org.bson.BsonDocument; @@ -39,6 +40,7 @@ import org.mongodb.morphia.annotations.Property; import org.mongodb.morphia.annotations.Text; import org.mongodb.morphia.mapping.MappedClass; +import org.mongodb.morphia.mapping.Mapper; import org.mongodb.morphia.mapping.MappingException; import org.mongodb.morphia.utils.IndexDirection; import org.mongodb.morphia.utils.IndexType; @@ -64,7 +66,7 @@ public class IndexHelperTest extends TestBase { @Before public void before() { - getMorphia().map(IndexedClass.class, NestedClass.class, NestedClassImpl.class); + getMorphia().map(AbstractParent.class, IndexedClass.class, NestedClass.class, NestedClassImpl.class); } @Test @@ -109,13 +111,13 @@ public void calculateKeys() { @Test public void createIndex() { checkMinServerVersion(3.4); - MongoCollection collection = getDatabase().getCollection("indexes"); - MappedClass mappedClass = getMorphia().getMapper().getMappedClass(IndexedClass.class); + String collectionName = getDs().getCollection(IndexedClass.class).getName(); + MongoCollection collection = getDatabase().getCollection(collectionName); + Mapper mapper = getMorphia().getMapper(); - indexHelper.createIndex(collection, mappedClass, false); + indexHelper.createIndex(collection, mapper.getMappedClass(IndexedClass.class), false); List indexInfo = getDs().getCollection(IndexedClass.class) .getIndexInfo(); - assertEquals("Should have 5 indexes", 5, indexInfo.size()); for (DBObject dbObject : indexInfo) { String name = dbObject.get("name").toString(); if (name.equals("latitude_1")) { @@ -129,22 +131,27 @@ public void createIndex() { assertEquals(parse("{ 'nest.name' : 1} "), dbObject.get("key")); } else if (name.equals("searchme")) { assertEquals(parse("{ 'text' : 10 }"), dbObject.get("weights")); + } else if (name.equals("indexName_1")) { + assertEquals(parse("{'indexName': 1 }"), dbObject.get("key")); } else { if (!"_id_".equals(dbObject.get("name"))) { throw new MappingException("Found an index I wasn't expecting: " + dbObject); } } - } + collection = getDatabase().getCollection(getDs().getCollection(AbstractParent.class).getName()); + indexHelper.createIndex(collection, mapper.getMappedClass(AbstractParent.class), false); + indexInfo = getDs().getCollection(AbstractParent.class).getIndexInfo(); + assertTrue("Shouldn't find any indexes: " + indexInfo, indexInfo.isEmpty()); + } @Test public void findField() { MappedClass mappedClass = getMorphia().getMapper().getMappedClass(IndexedClass.class); - assertEquals("name", indexHelper.findField(mappedClass, new IndexOptionsBuilder(), singletonList("indexName"))); - assertEquals("name", indexHelper.findField(mappedClass, new IndexOptionsBuilder(), singletonList("name"))); + assertEquals("indexName", indexHelper.findField(mappedClass, new IndexOptionsBuilder(), singletonList("indexName"))); assertEquals("nest.name", indexHelper.findField(mappedClass, new IndexOptionsBuilder(), asList("nested", "name"))); assertEquals("nest.name", indexHelper.findField(mappedClass, new IndexOptionsBuilder(), asList("nest", "name"))); @@ -254,7 +261,7 @@ public void oldIndexForm() { List indexInfo = getDs().getCollection(IndexedClass.class) .getIndexInfo(); for (DBObject dbObject : indexInfo) { - if (dbObject.get("name").equals("index_name")) { + if (dbObject.get("name").equals("index_indexName")) { checkIndex(dbObject); } } @@ -454,13 +461,9 @@ private interface NestedClass { @Entity("indexes") @Indexes(@Index(fields = @Field("latitude"))) - private static class IndexedClass { - @Id - private ObjectId id; + private static class IndexedClass extends AbstractParent { @Text(value = 10, options = @IndexOptions(name = "searchme")) private String text; - @Property("name") - private double indexName; private double latitude; @Embedded("nest") private NestedClass nested; @@ -474,4 +477,11 @@ private static class NestedClassImpl implements NestedClass { @Indexed private String name; } + + @Indexes(@Index(fields = @Field("indexName"))) + private static abstract class AbstractParent { + @Id + private ObjectId id; + private double indexName; + } } From c2643d689a94a337f0fe3b595e227d4a704bc35e Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Wed, 7 Dec 2016 16:02:01 -0500 Subject: [PATCH 2/3] re-inline this variable now that I'm done debugging --- morphia/src/main/java/org/mongodb/morphia/IndexHelper.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/morphia/src/main/java/org/mongodb/morphia/IndexHelper.java b/morphia/src/main/java/org/mongodb/morphia/IndexHelper.java index e9efd196ee2..d43dd7fc79c 100644 --- a/morphia/src/main/java/org/mongodb/morphia/IndexHelper.java +++ b/morphia/src/main/java/org/mongodb/morphia/IndexHelper.java @@ -167,8 +167,7 @@ private List collectNestedIndexes(final MappedClass mc, final List indexes = collectIndexes(aClass, parents); - for (Index index : indexes) { + for (Index index : collectIndexes(aClass, parents)) { List fields = new ArrayList(); for (Field field : index.fields()) { fields.add(new FieldBuilder() From ccb2d9e17f7dfa43037efae7e6d7feb2369afed3 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Thu, 8 Dec 2016 08:31:39 -0500 Subject: [PATCH 3/3] restore count check to catch missing (as opposed to extra) index definitions --- .../src/test/java/org/mongodb/morphia/IndexHelperTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java b/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java index c2ba723a3df..8c334120b96 100644 --- a/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java +++ b/morphia/src/test/java/org/mongodb/morphia/IndexHelperTest.java @@ -17,7 +17,6 @@ package org.mongodb.morphia; import com.mongodb.BasicDBObject; -import com.mongodb.DBCollection; import com.mongodb.DBObject; import com.mongodb.client.MongoCollection; import org.bson.BsonDocument; @@ -37,7 +36,6 @@ import org.mongodb.morphia.annotations.IndexOptions; import org.mongodb.morphia.annotations.Indexed; import org.mongodb.morphia.annotations.Indexes; -import org.mongodb.morphia.annotations.Property; import org.mongodb.morphia.annotations.Text; import org.mongodb.morphia.mapping.MappedClass; import org.mongodb.morphia.mapping.Mapper; @@ -118,6 +116,7 @@ public void createIndex() { indexHelper.createIndex(collection, mapper.getMappedClass(IndexedClass.class), false); List indexInfo = getDs().getCollection(IndexedClass.class) .getIndexInfo(); + assertEquals("Should have 6 indexes", 6, indexInfo.size()); for (DBObject dbObject : indexInfo) { String name = dbObject.get("name").toString(); if (name.equals("latitude_1")) { @@ -479,7 +478,7 @@ private static class NestedClassImpl implements NestedClass { } @Indexes(@Index(fields = @Field("indexName"))) - private static abstract class AbstractParent { + private abstract static class AbstractParent { @Id private ObjectId id; private double indexName;