From 6862bd8a450715e4f00a576b6dcaab5787026742 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Mon, 12 Dec 2011 17:53:37 +0100 Subject: [PATCH] DATAMONGO-343 - Fixed registration of ServerAddressPropertyEditor. Changed the setter parameters for ServerAddresses to use arrays instead of List. We now register the ServerAddressPropertyEditor to convert a ServerAddress[] and thus don't register a PropertyEditor for List which caused unwanted side effects before. --- .../mongodb/config/MongoDbFactoryParser.java | 46 +++++++------------ .../data/mongodb/config/MongoParser.java | 33 ++++++------- .../data/mongodb/config/ParsingUtils.java | 27 ++++++++++- .../config/ServerAddressPropertyEditor.java | 29 +++++++----- .../data/mongodb/core/MongoFactoryBean.java | 10 ++-- .../config/MongoParserIntegrationTests.java | 23 ++++++++-- .../test/resources/namespace/mongo-bean.xml | 2 + 7 files changed, 102 insertions(+), 68 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoDbFactoryParser.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoDbFactoryParser.java index d87d64395..4813a4494 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoDbFactoryParser.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoDbFactoryParser.java @@ -15,20 +15,15 @@ */ package org.springframework.data.mongodb.config; -import static org.springframework.data.mongodb.config.BeanNames.DB_FACTORY; -import static org.springframework.data.mongodb.config.ParsingUtils.getSourceBeanDefinition; - -import java.util.Map; +import static org.springframework.data.mongodb.config.BeanNames.*; +import static org.springframework.data.mongodb.config.ParsingUtils.*; import org.springframework.beans.factory.BeanDefinitionStoreException; import org.springframework.beans.factory.config.BeanDefinition; -import org.springframework.beans.factory.config.CustomEditorConfigurer; import org.springframework.beans.factory.config.RuntimeBeanReference; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.BeanDefinitionReaderUtils; -import org.springframework.beans.factory.support.BeanDefinitionRegistry; -import org.springframework.beans.factory.support.ManagedMap; import org.springframework.beans.factory.xml.AbstractBeanDefinitionParser; import org.springframework.beans.factory.xml.BeanDefinitionParser; import org.springframework.beans.factory.xml.ParserContext; @@ -61,39 +56,38 @@ public class MongoDbFactoryParser extends AbstractBeanDefinitionParser { @Override protected AbstractBeanDefinition parseInternal(Element element, ParserContext parserContext) { - + String uri = element.getAttribute("uri"); String mongoRef = element.getAttribute("mongo-ref"); String dbname = element.getAttribute("dbname"); - BeanDefinition userCredentials = getUserCredentialsBeanDefinition(element, parserContext); + BeanDefinition userCredentials = getUserCredentialsBeanDefinition(element, parserContext); // Common setup BeanDefinitionBuilder dbFactoryBuilder = BeanDefinitionBuilder.genericBeanDefinition(SimpleMongoDbFactory.class); ParsingUtils.setPropertyValue(element, dbFactoryBuilder, "write-concern", "writeConcern"); - + if (StringUtils.hasText(uri)) { - if(StringUtils.hasText(mongoRef) || StringUtils.hasText(dbname) || userCredentials != null) { - parserContext.getReaderContext().error("Configure either Mongo URI or details individually!", parserContext.extractSource(element)); + if (StringUtils.hasText(mongoRef) || StringUtils.hasText(dbname) || userCredentials != null) { + parserContext.getReaderContext().error("Configure either Mongo URI or details individually!", + parserContext.extractSource(element)); } - + dbFactoryBuilder.addConstructorArgValue(getMongoUri(uri)); return getSourceBeanDefinition(dbFactoryBuilder, parserContext, element); } - + // Defaulting mongoRef = StringUtils.hasText(mongoRef) ? mongoRef : registerMongoBeanDefinition(element, parserContext); dbname = StringUtils.hasText(dbname) ? dbname : "db"; - + dbFactoryBuilder.addConstructorArgValue(new RuntimeBeanReference(mongoRef)); dbFactoryBuilder.addConstructorArgValue(dbname); if (userCredentials != null) { dbFactoryBuilder.addConstructorArgValue(userCredentials); } - - //Register property editor to parse WriteConcern - - registerWriteConcernPropertyEditor(parserContext.getRegistry()); + + ParsingUtils.registerWriteConcernPropertyEditor(parserContext.getRegistry()); return getSourceBeanDefinition(dbFactoryBuilder, parserContext, element); } @@ -115,14 +109,6 @@ public class MongoDbFactoryParser extends AbstractBeanDefinitionParser { return BeanDefinitionReaderUtils.registerWithGeneratedName(mongoBuilder.getBeanDefinition(), parserContext.getRegistry()); } - - private void registerWriteConcernPropertyEditor(BeanDefinitionRegistry registry) { - BeanDefinitionBuilder customEditorConfigurer = BeanDefinitionBuilder.genericBeanDefinition(CustomEditorConfigurer.class); - Map customEditors = new ManagedMap(); - customEditors.put("com.mongodb.WriteConcern", "org.springframework.data.mongodb.config.WriteConcernPropertyEditor"); - customEditorConfigurer.addPropertyValue("customEditors", customEditors); - BeanDefinitionReaderUtils.registerWithGeneratedName(customEditorConfigurer.getBeanDefinition(), registry); - } /** * Returns a {@link BeanDefinition} for a {@link UserCredentials} object. @@ -145,7 +131,7 @@ public class MongoDbFactoryParser extends AbstractBeanDefinitionParser { return getSourceBeanDefinition(userCredentialsBuilder, context, element); } - + /** * Creates a {@link BeanDefinition} for a {@link MongoURI}. * @@ -153,10 +139,10 @@ public class MongoDbFactoryParser extends AbstractBeanDefinitionParser { * @return */ private BeanDefinition getMongoUri(String uri) { - + BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(MongoURI.class); builder.addConstructorArgValue(uri); - + return builder.getBeanDefinition(); } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoParser.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoParser.java index 1f4958ebb..8c8be401e 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoParser.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/MongoParser.java @@ -52,33 +52,30 @@ public class MongoParser extends AbstractSingleBeanDefinitionParser { ParsingUtils.parseMongoOptions(element, builder); ParsingUtils.parseReplicaSet(element, builder); - + registerServerAddressPropertyEditor(parserContext.getRegistry()); - registerWriteConcernPropertyEditor(parserContext.getRegistry()); + ParsingUtils.registerWriteConcernPropertyEditor(parserContext.getRegistry()); } /** - * One should only register one bean definition but want to have the convenience of using AbstractSingleBeanDefinitionParser but have the side effect of - * registering a 'default' property editor with the container. - * @param parserContext the ParserContext to + * One should only register one bean definition but want to have the convenience of using + * AbstractSingleBeanDefinitionParser but have the side effect of registering a 'default' property editor with the + * container. + * + * @param parserContext the ParserContext to */ private void registerServerAddressPropertyEditor(BeanDefinitionRegistry registry) { - BeanDefinitionBuilder customEditorConfigurer = BeanDefinitionBuilder.genericBeanDefinition(CustomEditorConfigurer.class); - Map customEditors = new ManagedMap(); - customEditors.put("java.util.List", "org.springframework.data.mongodb.config.ServerAddressPropertyEditor"); + + BeanDefinitionBuilder customEditorConfigurer = BeanDefinitionBuilder + .genericBeanDefinition(CustomEditorConfigurer.class); + Map customEditors = new ManagedMap(); + customEditors.put("com.mongodb.ServerAddress[]", + "org.springframework.data.mongodb.config.ServerAddressPropertyEditor"); customEditorConfigurer.addPropertyValue("customEditors", customEditors); - BeanDefinitionReaderUtils.registerWithGeneratedName(customEditorConfigurer.getBeanDefinition(), registry); + BeanDefinitionReaderUtils.registerWithGeneratedName(customEditorConfigurer.getBeanDefinition(), registry); } - - private void registerWriteConcernPropertyEditor(BeanDefinitionRegistry registry) { - BeanDefinitionBuilder customEditorConfigurer = BeanDefinitionBuilder.genericBeanDefinition(CustomEditorConfigurer.class); - Map customEditors = new ManagedMap(); - customEditors.put("com.mongodb.WriteConcern", "org.springframework.data.mongodb.config.WriteConcernPropertyEditor"); - customEditorConfigurer.addPropertyValue("customEditors", customEditors); - BeanDefinitionReaderUtils.registerWithGeneratedName(customEditorConfigurer.getBeanDefinition(), registry); - } - + @Override protected String resolveId(Element element, AbstractBeanDefinition definition, ParserContext parserContext) throws BeanDefinitionStoreException { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/ParsingUtils.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/ParsingUtils.java index 99ced8121..36a2411d3 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/ParsingUtils.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/ParsingUtils.java @@ -16,11 +16,18 @@ package org.springframework.data.mongodb.config; +import java.util.Map; + import org.springframework.beans.factory.config.BeanDefinition; +import org.springframework.beans.factory.config.CustomEditorConfigurer; import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.BeanDefinitionReaderUtils; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.ManagedMap; import org.springframework.beans.factory.xml.ParserContext; import org.springframework.data.mongodb.core.MongoOptionsFactoryBean; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.util.xml.DomUtils; import org.w3c.dom.Element; @@ -108,9 +115,27 @@ abstract class ParsingUtils { * @param element must not be {@literal null}. * @return */ - static AbstractBeanDefinition getSourceBeanDefinition(BeanDefinitionBuilder builder, ParserContext context, Element element) { + static AbstractBeanDefinition getSourceBeanDefinition(BeanDefinitionBuilder builder, ParserContext context, + Element element) { AbstractBeanDefinition definition = builder.getBeanDefinition(); definition.setSource(context.extractSource(element)); return definition; } + + /** + * Registers a {@link WriteConcernPropertyEditor} in the given {@link BeanDefinitionRegistry}. + * + * @param registry must not be {@literal null}. + */ + static void registerWriteConcernPropertyEditor(BeanDefinitionRegistry registry) { + + Assert.notNull(registry); + + BeanDefinitionBuilder customEditorConfigurer = BeanDefinitionBuilder + .genericBeanDefinition(CustomEditorConfigurer.class); + Map> customEditors = new ManagedMap>(); + customEditors.put("com.mongodb.WriteConcern", WriteConcernPropertyEditor.class); + customEditorConfigurer.addPropertyValue("customEditors", customEditors); + BeanDefinitionReaderUtils.registerWithGeneratedName(customEditorConfigurer.getBeanDefinition(), registry); + } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditor.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditor.java index 9b5504fa4..1cddded9c 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditor.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditor.java @@ -17,37 +17,42 @@ package org.springframework.data.mongodb.config; import java.beans.PropertyEditorSupport; import java.net.UnknownHostException; -import java.util.ArrayList; -import java.util.List; import org.springframework.util.StringUtils; import com.mongodb.ServerAddress; /** - * Parse a string to a List. The format is host1:port1,host2:port2,host3:port3. + * Parse a {@link String} to a {@link ServerAddress} array. The format is host1:port1,host2:port2,host3:port3. + * * @author Mark Pollack - * + * @author Oliver Gierke */ public class ServerAddressPropertyEditor extends PropertyEditorSupport { - /** - * Parse a string to a List + /* + * (non-Javadoc) + * @see java.beans.PropertyEditorSupport#setAsText(java.lang.String) */ + @Override public void setAsText(String replicaSetString) { - List serverAddresses = new ArrayList(); String[] replicaSetStringArray = StringUtils.commaDelimitedListToStringArray(replicaSetString); - for (String element2 : replicaSetStringArray) { - String[] hostAndPort = StringUtils.delimitedListToStringArray(element2, ":"); + ServerAddress[] serverAddresses = new ServerAddress[replicaSetStringArray.length]; + + for (int i = 0; i < replicaSetStringArray.length; i++) { + + String[] hostAndPort = StringUtils.delimitedListToStringArray(replicaSetStringArray[i], ":"); + try { - serverAddresses.add(new ServerAddress(hostAndPort[0], Integer.parseInt(hostAndPort[1]))); + serverAddresses[i] = new ServerAddress(hostAndPort[0], Integer.parseInt(hostAndPort[1])); } catch (NumberFormatException e) { - throw new IllegalArgumentException("Could not parse port " + hostAndPort[1], e ); + throw new IllegalArgumentException("Could not parse port " + hostAndPort[1], e); } catch (UnknownHostException e) { - throw new IllegalArgumentException("Could not parse host " + hostAndPort[0], e ); + throw new IllegalArgumentException("Could not parse host " + hostAndPort[0], e); } } + setValue(serverAddresses); } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoFactoryBean.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoFactoryBean.java index 627a67e59..71a7a735f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoFactoryBean.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoFactoryBean.java @@ -16,6 +16,7 @@ package org.springframework.data.mongodb.core; +import java.util.Arrays; import java.util.List; import org.apache.commons.logging.Log; @@ -24,6 +25,7 @@ import org.springframework.beans.factory.FactoryBean; import org.springframework.dao.DataAccessException; import org.springframework.dao.support.PersistenceExceptionTranslator; import org.springframework.data.mongodb.CannotGetMongoDbConnectionException; + import com.mongodb.Mongo; import com.mongodb.MongoOptions; import com.mongodb.ServerAddress; @@ -57,12 +59,12 @@ public class MongoFactoryBean implements FactoryBean, PersistenceExceptio this.mongoOptions = mongoOptions; } - public void setReplicaSetSeeds(List replicaSetSeeds) { - this.replicaSetSeeds = replicaSetSeeds; + public void setReplicaSetSeeds(ServerAddress[] replicaSetSeeds) { + this.replicaSetSeeds = Arrays.asList(replicaSetSeeds); } - public void setReplicaPair(List replicaPair) { - this.replicaPair = replicaPair; + public void setReplicaPair(ServerAddress[] replicaPair) { + this.replicaPair = Arrays.asList(replicaPair); } public void setHost(String host) { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoParserIntegrationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoParserIntegrationTests.java index eb82d6a3c..2d51ab6f3 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoParserIntegrationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoParserIntegrationTests.java @@ -15,8 +15,8 @@ */ package org.springframework.data.mongodb.config; -import static org.junit.Assert.*; import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; import java.util.List; @@ -27,18 +27,21 @@ import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionReader; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; +import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.io.ClassPathResource; +import com.mongodb.Mongo; + /** * Integration tests for {@link MongoParser}. * * @author Oliver Gierke */ public class MongoParserIntegrationTests { - + DefaultListableBeanFactory factory; BeanDefinitionReader reader; - + @Before public void setUp() { factory = new DefaultListableBeanFactory(); @@ -56,4 +59,18 @@ public class MongoParserIntegrationTests { factory.getBean("mongo"); } + + /** + * @see DATAMONGO-343 + */ + @Test + public void readsServerAddressesCorrectly() { + + reader.loadBeanDefinitions(new ClassPathResource("namespace/mongo-bean.xml")); + + GenericApplicationContext context = new GenericApplicationContext(factory); + context.refresh(); + + assertThat(context.getBean("mongo2", Mongo.class), is(notNullValue())); + } } diff --git a/spring-data-mongodb/src/test/resources/namespace/mongo-bean.xml b/spring-data-mongodb/src/test/resources/namespace/mongo-bean.xml index 728428251..5388ec8c3 100644 --- a/spring-data-mongodb/src/test/resources/namespace/mongo-bean.xml +++ b/spring-data-mongodb/src/test/resources/namespace/mongo-bean.xml @@ -6,5 +6,7 @@ http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd"> + +