From 35e8ae12247792dcac6238dc3d29f77b41cd6704 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 31 Jul 2012 15:32:56 +0200 Subject: [PATCH] DATAMONGO-500 - Index creation is only done for the correct MappingContext. Index creation now double checks the MappingContext a MappingContextEvent originates from before actually creating indexes. This avoids invalid indexes being created in a multi-database scenario. --- spring-data-mongodb-parent/pom.xml | 2 +- .../config/MappingMongoConverterParser.java | 45 ++++++++----- .../data/mongodb/core/MongoTemplate.java | 18 ++--- .../core/convert/MappingMongoConverter.java | 11 +--- .../index/MongoMappingEventPublisher.java | 38 +++++++---- .../MongoPersistentEntityIndexCreator.java | 12 +++- ...entEntityIndexCreatorIntegrationTests.java | 66 +++++++++++++++++++ ...PersistentEntityIndexCreatorUnitTests.java | 26 ++++++++ .../data/mongodb/core/index/SampleEntity.java | 14 ++++ ...tyIndexCreatorIntegrationTests-context.xml | 23 +++++++ 10 files changed, 200 insertions(+), 55 deletions(-) create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorIntegrationTests.java create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/SampleEntity.java create mode 100644 spring-data-mongodb/src/test/resources/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorIntegrationTests-context.xml diff --git a/spring-data-mongodb-parent/pom.xml b/spring-data-mongodb-parent/pom.xml index 842ab9e9c..7d08da5ee 100644 --- a/spring-data-mongodb-parent/pom.xml +++ b/spring-data-mongodb-parent/pom.xml @@ -18,7 +18,7 @@ 3.0.7.RELEASE 4.0.0.RELEASE [${org.springframework.version.30}, ${org.springframework.version.40}) - 1.4.0.M1 + 1.4.0.BUILD-SNAPSHOT 1.6.11.RELEASE true diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MappingMongoConverterParser.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MappingMongoConverterParser.java index dfb7e9791..8bfd4aaa6 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MappingMongoConverterParser.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MappingMongoConverterParser.java @@ -48,6 +48,7 @@ import org.springframework.core.type.filter.AnnotationTypeFilter; import org.springframework.core.type.filter.AssignableTypeFilter; import org.springframework.core.type.filter.TypeFilter; import org.springframework.data.annotation.Persistent; +import org.springframework.data.config.BeanComponentDefinitionBuilder; import org.springframework.data.mongodb.core.convert.CustomConversions; import org.springframework.data.mongodb.core.convert.MappingMongoConverter; import org.springframework.data.mongodb.core.index.MongoPersistentEntityIndexCreator; @@ -166,27 +167,35 @@ public class MappingMongoConverterParser extends AbstractBeanDefinitionParser { BeanDefinition conversionsDefinition) { String ctxRef = element.getAttribute("mapping-context-ref"); - if (!StringUtils.hasText(ctxRef)) { - BeanDefinitionBuilder mappingContextBuilder = BeanDefinitionBuilder - .genericBeanDefinition(MongoMappingContext.class); - Set classesToAdd = getInititalEntityClasses(element, mappingContextBuilder); - if (classesToAdd != null) { - mappingContextBuilder.addPropertyValue("initialEntitySet", classesToAdd); - } - - if (conversionsDefinition != null) { - AbstractBeanDefinition simpleTypesDefinition = new GenericBeanDefinition(); - simpleTypesDefinition.setFactoryBeanName("customConversions"); - simpleTypesDefinition.setFactoryMethodName("getSimpleTypeHolder"); - - mappingContextBuilder.addPropertyValue("simpleTypeHolder", simpleTypesDefinition); - } - - parserContext.getRegistry().registerBeanDefinition(MAPPING_CONTEXT, mappingContextBuilder.getBeanDefinition()); - ctxRef = MAPPING_CONTEXT; + if (StringUtils.hasText(ctxRef)) { + return ctxRef; } + BeanComponentDefinitionBuilder componentDefinitionBuilder = new BeanComponentDefinitionBuilder(element, + parserContext); + + BeanDefinitionBuilder mappingContextBuilder = BeanDefinitionBuilder + .genericBeanDefinition(MongoMappingContext.class); + + Set classesToAdd = getInititalEntityClasses(element, mappingContextBuilder); + if (classesToAdd != null) { + mappingContextBuilder.addPropertyValue("initialEntitySet", classesToAdd); + } + + if (conversionsDefinition != null) { + AbstractBeanDefinition simpleTypesDefinition = new GenericBeanDefinition(); + simpleTypesDefinition.setFactoryBeanName("customConversions"); + simpleTypesDefinition.setFactoryMethodName("getSimpleTypeHolder"); + + mappingContextBuilder.addPropertyValue("simpleTypeHolder", simpleTypesDefinition); + } + + parserContext.getRegistry().registerBeanDefinition(MAPPING_CONTEXT, mappingContextBuilder.getBeanDefinition()); + ctxRef = MAPPING_CONTEXT; + + parserContext.registerBeanComponent(componentDefinitionBuilder.getComponent(mappingContextBuilder, ctxRef)); + return ctxRef; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index 4e3a85571..7bf86b206 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java @@ -1356,21 +1356,13 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware { ConversionService conversionService = mongoConverter.getConversionService(); BeanWrapper, Object> wrapper = BeanWrapper.create(savedObject, conversionService); - try { + Object idValue = wrapper.getProperty(idProp, idProp.getType(), true); - Object idValue = wrapper.getProperty(idProp, idProp.getType(), true); - - if (idValue != null) { - return; - } - - wrapper.setProperty(idProp, id); - - } catch (IllegalAccessException e) { - throw new MappingException(e.getMessage(), e); - } catch (InvocationTargetException e) { - throw new MappingException(e.getMessage(), e); + if (idValue != null) { + return; } + + wrapper.setProperty(idProp, id); } private DBCollection getAndPrepareCollection(DB db, String collectionName) { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index 3dcfb9dfe..68bfe67f1 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java @@ -15,7 +15,6 @@ */ package org.springframework.data.mongodb.core.convert; -import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -253,13 +252,9 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App public void doWithAssociation(Association association) { MongoPersistentProperty inverseProp = association.getInverse(); Object obj = getValueInternal(inverseProp, dbo, evaluator, result); - try { - wrapper.setProperty(inverseProp, obj); - } catch (IllegalAccessException e) { - throw new MappingException(e.getMessage(), e); - } catch (InvocationTargetException e) { - throw new MappingException(e.getMessage(), e); - } + + wrapper.setProperty(inverseProp, obj); + } }); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoMappingEventPublisher.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoMappingEventPublisher.java index bf76048d2..f1197deb1 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoMappingEventPublisher.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/index/MongoMappingEventPublisher.java @@ -1,11 +1,11 @@ /* - * Copyright (c) 2011 by the original author(s). + * Copyright 2011-2012 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -13,37 +13,51 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.springframework.data.mongodb.core.index; +import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEventPublisher; import org.springframework.data.mapping.event.MappingContextEvent; +import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; +import org.springframework.data.mongodb.core.mapping.event.AfterLoadEvent; +import org.springframework.data.mongodb.core.mapping.event.AfterSaveEvent; +import org.springframework.util.Assert; /** - * An implementation of ApplicationEventPublisher that will only fire MappingContextEvents for use by the index creator - * when MongoTemplate is used 'stand-alone', that is not declared inside a Spring ApplicationContext. + * An implementation of ApplicationEventPublisher that will only fire {@link MappingContextEvent}s for use by the index + * creator when MongoTemplate is used 'stand-alone', that is not declared inside a Spring {@link ApplicationContext}. + * Declare {@link MongoTemplate} inside an {@link ApplicationContext} to enable the publishing of all persistence events + * such as {@link AfterLoadEvent}, {@link AfterSaveEvent}, etc. * - * Declare MongoTemplate inside an ApplicationContext to enable the publishing of all persistence events such as - * {@link AfterLoadEvent}, {@link AfterSaveEvent}, etc. - * - * @author Jon Brisbin + * @author Jon Brisbin + * @author Oliver Gierke */ public class MongoMappingEventPublisher implements ApplicationEventPublisher { - private MongoPersistentEntityIndexCreator indexCreator; + private final MongoPersistentEntityIndexCreator indexCreator; + /** + * Creates a new {@link MongoMappingEventPublisher} for the given {@link MongoPersistentEntityIndexCreator}. + * + * @param indexCreator must not be {@literal null}. + */ public MongoMappingEventPublisher(MongoPersistentEntityIndexCreator indexCreator) { + + Assert.notNull(indexCreator); this.indexCreator = indexCreator; } + /* + * (non-Javadoc) + * @see org.springframework.context.ApplicationEventPublisher#publishEvent(org.springframework.context.ApplicationEvent) + */ @SuppressWarnings("unchecked") public void publishEvent(ApplicationEvent event) { if (event instanceof MappingContextEvent) { - indexCreator - .onApplicationEvent((MappingContextEvent, MongoPersistentProperty>) event); + indexCreator.onApplicationEvent((MappingContextEvent, MongoPersistentProperty>) event); } } } 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 ed644a7fe..3356745e1 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 @@ -44,12 +44,13 @@ import com.mongodb.util.JSON; * @author Oliver Gierke */ public class MongoPersistentEntityIndexCreator implements - ApplicationListener, MongoPersistentProperty>> { + ApplicationListener, MongoPersistentProperty>> { private static final Logger log = LoggerFactory.getLogger(MongoPersistentEntityIndexCreator.class); private final Map, Boolean> classesSeen = new ConcurrentHashMap, Boolean>(); private final MongoDbFactory mongoDbFactory; + private final MongoMappingContext mappingContext; /** * Creats a new {@link MongoPersistentEntityIndexCreator} for the given {@link MongoMappingContext} and @@ -62,7 +63,9 @@ public class MongoPersistentEntityIndexCreator implements Assert.notNull(mongoDbFactory); Assert.notNull(mappingContext); + this.mongoDbFactory = mongoDbFactory; + this.mappingContext = mappingContext; for (MongoPersistentEntity entity : mappingContext.getPersistentEntities()) { checkForIndexes(entity); @@ -73,8 +76,11 @@ public class MongoPersistentEntityIndexCreator implements * (non-Javadoc) * @see org.springframework.context.ApplicationListener#onApplicationEvent(org.springframework.context.ApplicationEvent) */ - public void onApplicationEvent( - MappingContextEvent, MongoPersistentProperty> event) { + public void onApplicationEvent(MappingContextEvent, MongoPersistentProperty> event) { + + if (!event.wasEmittedBy(mappingContext)) { + return; + } PersistentEntity entity = event.getPersistentEntity(); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorIntegrationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorIntegrationTests.java new file mode 100644 index 000000000..fd468fda5 --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorIntegrationTests.java @@ -0,0 +1,66 @@ +/* + * Copyright 2012 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.mongodb.core.index; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import java.util.List; + +import org.hamcrest.Matchers; +import org.junit.After; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.data.mongodb.core.MongoOperations; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * Integration tests for {@link MongoPersistentEntityIndexCreator}. + * + * @author Oliver Gierke + */ +@RunWith(SpringJUnit4ClassRunner.class) +@ContextConfiguration +public class MongoPersistentEntityIndexCreatorIntegrationTests { + + @Autowired + @Qualifier("mongo1") + MongoOperations templateOne; + + @Autowired + @Qualifier("mongo2") + MongoOperations templateTwo; + + @After + public void cleanUp() { + templateOne.dropCollection(SampleEntity.class); + templateTwo.dropCollection(SampleEntity.class); + } + + @Test + public void foo() { + + List indexInfo = templateOne.indexOps(SampleEntity.class).getIndexInfo(); + assertThat(indexInfo, hasSize(greaterThan(0))); + assertThat(indexInfo, Matchers. hasItem(hasProperty("name", is("prop")))); + + indexInfo = templateTwo.indexOps(SampleEntity.class).getIndexInfo(); + assertThat(indexInfo, hasSize(0)); + } +} 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 f99469b7c..6e46b5d40 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 @@ -24,9 +24,13 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.context.ApplicationContext; +import org.springframework.data.mapping.event.MappingContextEvent; import org.springframework.data.mongodb.MongoDbFactory; import org.springframework.data.mongodb.core.mapping.Field; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; +import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; +import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; import com.mongodb.DBObject; @@ -40,6 +44,8 @@ public class MongoPersistentEntityIndexCreatorUnitTests { @Mock MongoDbFactory factory; + @Mock + ApplicationContext context; @Test public void buildsIndexDefinitionUsingFieldName() { @@ -55,6 +61,26 @@ public class MongoPersistentEntityIndexCreatorUnitTests { assertThat(creator.name, is("indexName")); } + @Test + public void doesNotCreateIndexForEntityComingFromDifferentMappingContext() { + + MongoMappingContext mappingContext = new MongoMappingContext(); + + MongoMappingContext personMappingContext = new MongoMappingContext(); + personMappingContext.setInitialEntitySet(Collections.singleton(Person.class)); + personMappingContext.initialize(); + + DummyMongoPersistentEntityIndexCreator creator = new DummyMongoPersistentEntityIndexCreator(mappingContext, factory); + + MongoPersistentEntity entity = personMappingContext.getPersistentEntity(Person.class); + MappingContextEvent, MongoPersistentProperty> event = new MappingContextEvent, MongoPersistentProperty>( + personMappingContext, entity); + + creator.onApplicationEvent(event); + + assertThat(creator.indexDefinition, is(nullValue())); + } + static class Person { @Indexed(name = "indexName") diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/SampleEntity.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/SampleEntity.java new file mode 100644 index 000000000..a099c4cb0 --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/index/SampleEntity.java @@ -0,0 +1,14 @@ +package org.springframework.data.mongodb.core.index; + +import org.springframework.data.annotation.Id; +import org.springframework.data.mongodb.core.mapping.Document; + +@Document +public class SampleEntity { + + @Id + String id; + + @Indexed + String prop; +} diff --git a/spring-data-mongodb/src/test/resources/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorIntegrationTests-context.xml b/spring-data-mongodb/src/test/resources/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorIntegrationTests-context.xml new file mode 100644 index 000000000..6ed28955f --- /dev/null +++ b/spring-data-mongodb/src/test/resources/org/springframework/data/mongodb/core/index/MongoPersistentEntityIndexCreatorIntegrationTests-context.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + +