From 2493de5f918e39a1b874e3e468652113aea5988d Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Tue, 2 Jul 2013 15:36:14 +0200 Subject: [PATCH] DATAMONGO-693 - More robust handling of host and replica set config in MongoFactoryBean. MongoFactoryBean now considers empty strings for the replicaPair property as not set at all. The ServerAdressPropertyEditor also returns null as value for empty text strings. Deprecated setter for replica pair on MongoFactoryBean. --- .../config/ServerAddressPropertyEditor.java | 8 +++- .../data/mongodb/core/MongoFactoryBean.java | 47 ++++++++++++++++--- .../ServerAddressPropertyEditorUnitTests.java | 13 ++++- .../core/MongoFactoryBeanIntegrationTest.java | 24 +++++++++- 4 files changed, 83 insertions(+), 9 deletions(-) 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 c3441fd85..8e3dddbda 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 @@ -1,5 +1,5 @@ /* - * Copyright 2011 the original author or authors. + * Copyright 2011-2013 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. @@ -31,6 +31,7 @@ import com.mongodb.ServerAddress; * * @author Mark Pollack * @author Oliver Gierke + * @author Thomas Darimont */ public class ServerAddressPropertyEditor extends PropertyEditorSupport { @@ -43,6 +44,11 @@ public class ServerAddressPropertyEditor extends PropertyEditorSupport { @Override public void setAsText(String replicaSetString) { + if (!StringUtils.hasText(replicaSetString)) { + setValue(null); + return; + } + String[] replicaSetStringArray = StringUtils.commaDelimitedListToStringArray(replicaSetString); Set serverAddresses = new HashSet(replicaSetStringArray.length); 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 7a001bd12..343d3b93d 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 @@ -15,7 +15,9 @@ */ package org.springframework.data.mongodb.core; -import java.util.Arrays; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.List; import org.springframework.beans.factory.DisposableBean; @@ -24,6 +26,7 @@ import org.springframework.beans.factory.InitializingBean; import org.springframework.dao.DataAccessException; import org.springframework.dao.support.PersistenceExceptionTranslator; import org.springframework.data.mongodb.CannotGetMongoDbConnectionException; +import org.springframework.util.StringUtils; import com.mongodb.Mongo; import com.mongodb.MongoOptions; @@ -36,6 +39,7 @@ import com.mongodb.WriteConcern; * @author Thomas Risberg * @author Graeme Rocher * @author Oliver Gierke + * @author Thomas Darimont * @since 1.0 */ public class MongoFactoryBean implements FactoryBean, InitializingBean, DisposableBean, @@ -57,11 +61,38 @@ public class MongoFactoryBean implements FactoryBean, InitializingBean, D } public void setReplicaSetSeeds(ServerAddress[] replicaSetSeeds) { - this.replicaSetSeeds = Arrays.asList(replicaSetSeeds); + this.replicaSetSeeds = filterNonNullElementsAsList(replicaSetSeeds); } + /** + * @deprecated use {@link #setReplicaSetSeeds(ServerAddress[])} instead + * + * @param replicaPair + */ + @Deprecated public void setReplicaPair(ServerAddress[] replicaPair) { - this.replicaPair = Arrays.asList(replicaPair); + this.replicaPair = filterNonNullElementsAsList(replicaPair); + } + + /** + * @param elements the elements to filter + * @return a new unmodifiable {@link List#} from the given elements without nulls + */ + private List filterNonNullElementsAsList(T[] elements) { + + if (elements == null) { + return Collections.emptyList(); + } + + List candidateElements = new ArrayList(); + + for (T element : elements) { + if (element != null) { + candidateElements.add(element); + } + } + + return Collections.unmodifiableList(candidateElements); } public void setHost(String host) { @@ -126,15 +157,15 @@ public class MongoFactoryBean implements FactoryBean, InitializingBean, D mongoOptions = new MongoOptions(); } - if (replicaPair != null) { + if (!isNullOrEmpty(replicaPair)) { if (replicaPair.size() < 2) { throw new CannotGetMongoDbConnectionException("A replica pair must have two server entries"); } mongo = new Mongo(replicaPair.get(0), replicaPair.get(1), mongoOptions); - } else if (replicaSetSeeds != null) { + } else if (!isNullOrEmpty(replicaSetSeeds)) { mongo = new Mongo(replicaSetSeeds, mongoOptions); } else { - String mongoHost = host != null ? host : defaultOptions.getHost(); + String mongoHost = StringUtils.hasText(host) ? host : defaultOptions.getHost(); mongo = port != null ? new Mongo(new ServerAddress(mongoHost, port), mongoOptions) : new Mongo(mongoHost, mongoOptions); } @@ -146,6 +177,10 @@ public class MongoFactoryBean implements FactoryBean, InitializingBean, D this.mongo = mongo; } + private boolean isNullOrEmpty(Collection elements) { + return elements == null || elements.isEmpty(); + } + /* * (non-Javadoc) * @see org.springframework.beans.factory.DisposableBean#destroy() diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditorUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditorUnitTests.java index 3271b1da1..baf688f86 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditorUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditorUnitTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2013 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. @@ -31,6 +31,7 @@ import com.mongodb.ServerAddress; * Unit tests for {@link ServerAddressPropertyEditor}. * * @author Oliver Gierke + * @author Thomas Darimont */ public class ServerAddressPropertyEditorUnitTests { @@ -70,6 +71,16 @@ public class ServerAddressPropertyEditorUnitTests { assertSingleAddressOfLocalhost(editor.getValue()); } + /** + * @see DATAMONGO-693 + */ + @Test + public void interpretEmptyStringAsNull() { + + editor.setAsText(""); + assertNull(editor.getValue()); + } + private static void assertSingleAddressOfLocalhost(Object result) throws UnknownHostException { assertThat(result, is(instanceOf(ServerAddress[].class))); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoFactoryBeanIntegrationTest.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoFactoryBeanIntegrationTest.java index 48c6e0cb1..5abe37dbd 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoFactoryBeanIntegrationTest.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoFactoryBeanIntegrationTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2012 the original author or authors. + * Copyright 2012-2013 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. @@ -21,15 +21,19 @@ import static org.junit.Assert.*; import org.junit.Test; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.data.mongodb.config.ServerAddressPropertyEditor; import org.springframework.data.mongodb.config.WriteConcernPropertyEditor; import org.springframework.test.util.ReflectionTestUtils; +import com.mongodb.Mongo; +import com.mongodb.ServerAddress; import com.mongodb.WriteConcern; /** * Integration tests for {@link MongoFactoryBean}. * * @author Oliver Gierke + * @author Thomas Darimont */ public class MongoFactoryBeanIntegrationTest { @@ -49,4 +53,22 @@ public class MongoFactoryBeanIntegrationTest { MongoFactoryBean bean = factory.getBean("&factory", MongoFactoryBean.class); assertThat(ReflectionTestUtils.getField(bean, "writeConcern"), is((Object) WriteConcern.SAFE)); } + + /** + * @see DATAMONGO-693 + */ + @Test + public void createMongoInstanceWithHostAndEmptyReplicaSets() { + + RootBeanDefinition definition = new RootBeanDefinition(MongoFactoryBean.class); + definition.getPropertyValues().addPropertyValue("host", "localhost"); + definition.getPropertyValues().addPropertyValue("replicaPair", ""); + + DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); + factory.registerCustomEditor(ServerAddress.class, ServerAddressPropertyEditor.class); + factory.registerBeanDefinition("factory", definition); + + Mongo mongo = factory.getBean(Mongo.class); + assertNotNull(mongo); + } }