From ab666148436985630f9f749b38aefeed27f4a869 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 20 Jun 2012 10:46:22 +0200 Subject: [PATCH] DATAMONGO-454 - Improvements to ServerAddressPropertyEditor. ServerAddressPropertyEditor now only eventually fails if none of the configured addresses can be parsed correctly. Strengthened the parsing implementation to not fail for host-only parsing or accidental double commas. Cleaned up integration tests for replica set configuration. --- .../config/ServerAddressPropertyEditor.java | 54 ++++++++++--- .../config/MongoNamespaceReplicaSetTests.java | 39 +++++---- .../mongodb/config/NamespaceTestSupport.java | 42 ---------- .../ServerAddressPropertyEditorUnitTests.java | 80 +++++++++++++++++++ 4 files changed, 143 insertions(+), 72 deletions(-) delete mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/NamespaceTestSupport.java create mode 100644 spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditorUnitTests.java 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 1cddded9c..c3441fd85 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,7 +17,11 @@ package org.springframework.data.mongodb.config; import java.beans.PropertyEditorSupport; import java.net.UnknownHostException; +import java.util.HashSet; +import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.util.StringUtils; import com.mongodb.ServerAddress; @@ -30,6 +34,8 @@ import com.mongodb.ServerAddress; */ public class ServerAddressPropertyEditor extends PropertyEditorSupport { + private static final Logger LOG = LoggerFactory.getLogger(ServerAddressPropertyEditor.class); + /* * (non-Javadoc) * @see java.beans.PropertyEditorSupport#setAsText(java.lang.String) @@ -38,21 +44,49 @@ public class ServerAddressPropertyEditor extends PropertyEditorSupport { public void setAsText(String replicaSetString) { String[] replicaSetStringArray = StringUtils.commaDelimitedListToStringArray(replicaSetString); - ServerAddress[] serverAddresses = new ServerAddress[replicaSetStringArray.length]; + Set serverAddresses = new HashSet(replicaSetStringArray.length); - for (int i = 0; i < replicaSetStringArray.length; i++) { + for (String element : replicaSetStringArray) { - String[] hostAndPort = StringUtils.delimitedListToStringArray(replicaSetStringArray[i], ":"); + ServerAddress address = parseServerAddress(element); - try { - serverAddresses[i] = new ServerAddress(hostAndPort[0], Integer.parseInt(hostAndPort[1])); - } catch (NumberFormatException e) { - throw new IllegalArgumentException("Could not parse port " + hostAndPort[1], e); - } catch (UnknownHostException e) { - throw new IllegalArgumentException("Could not parse host " + hostAndPort[0], e); + if (address != null) { + serverAddresses.add(address); } } - setValue(serverAddresses); + if (serverAddresses.isEmpty()) { + throw new IllegalArgumentException( + "Could not resolve at least one server of the replica set configuration! Validate your config!"); + } + + setValue(serverAddresses.toArray(new ServerAddress[serverAddresses.size()])); + } + + /** + * Parses the given source into a {@link ServerAddress}. + * + * @param source + * @return the + */ + private ServerAddress parseServerAddress(String source) { + + String[] hostAndPort = StringUtils.delimitedListToStringArray(source.trim(), ":"); + + if (!StringUtils.hasText(source) || hostAndPort.length > 2) { + LOG.warn("Could not parse address source '{}'. Check your replica set configuration!", source); + return null; + } + + try { + return hostAndPort.length == 1 ? new ServerAddress(hostAndPort[0]) : new ServerAddress(hostAndPort[0], + Integer.parseInt(hostAndPort[1])); + } catch (UnknownHostException e) { + LOG.warn("Could not parse host '{}'. Check your replica set configuration!", hostAndPort[0]); + } catch (NumberFormatException e) { + LOG.warn("Could not parse port '{}'. Check your replica set configuration!", hostAndPort[1]); + } + + return null; } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoNamespaceReplicaSetTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoNamespaceReplicaSetTests.java index 0affa2f55..975812f77 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoNamespaceReplicaSetTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/MongoNamespaceReplicaSetTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2010 the original author or authors. + * 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. @@ -16,6 +16,7 @@ package org.springframework.data.mongodb.config; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import java.util.List; @@ -29,6 +30,7 @@ import org.springframework.data.mongodb.core.MongoFactoryBean; import org.springframework.data.mongodb.core.MongoTemplate; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.util.ReflectionTestUtils; import com.mongodb.CommandResult; import com.mongodb.Mongo; @@ -36,47 +38,45 @@ import com.mongodb.ServerAddress; @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration -public class MongoNamespaceReplicaSetTests extends NamespaceTestSupport { +public class MongoNamespaceReplicaSetTests { @Autowired private ApplicationContext ctx; @Test + @SuppressWarnings("unchecked") public void testParsingMongoWithReplicaSets() throws Exception { + assertTrue(ctx.containsBean("replicaSetMongo")); MongoFactoryBean mfb = (MongoFactoryBean) ctx.getBean("&replicaSetMongo"); - List replicaSetSeeds = readField("replicaSetSeeds", mfb); - assertNotNull(replicaSetSeeds); - - assertEquals("127.0.0.1", replicaSetSeeds.get(0).getHost()); - assertEquals(10001, replicaSetSeeds.get(0).getPort()); - - assertEquals("localhost", replicaSetSeeds.get(1).getHost()); - assertEquals(10002, replicaSetSeeds.get(1).getPort()); + List replicaSetSeeds = (List) ReflectionTestUtils.getField(mfb, "replicaSetSeeds"); + assertThat(replicaSetSeeds, is(notNullValue())); + assertThat(replicaSetSeeds, hasItems(new ServerAddress("127.0.0.1", 10001), new ServerAddress("localhost", 10002))); } @Test + @SuppressWarnings("unchecked") public void testParsingWithPropertyPlaceHolder() throws Exception { + assertTrue(ctx.containsBean("manyReplicaSetMongo")); MongoFactoryBean mfb = (MongoFactoryBean) ctx.getBean("&manyReplicaSetMongo"); - List replicaSetSeeds = readField("replicaSetSeeds", mfb); - assertNotNull(replicaSetSeeds); - - assertEquals("192.168.174.130", replicaSetSeeds.get(0).getHost()); - assertEquals(27017, replicaSetSeeds.get(0).getPort()); - assertEquals("192.168.174.130", replicaSetSeeds.get(1).getHost()); - assertEquals(27018, replicaSetSeeds.get(1).getPort()); - assertEquals("192.168.174.130", replicaSetSeeds.get(2).getHost()); - assertEquals(27019, replicaSetSeeds.get(2).getPort()); + List replicaSetSeeds = (List) ReflectionTestUtils.getField(mfb, "replicaSetSeeds"); + assertThat(replicaSetSeeds, is(notNullValue())); + assertThat(replicaSetSeeds, hasSize(3)); + assertThat( + replicaSetSeeds, + hasItems(new ServerAddress("192.168.174.130", 27017), new ServerAddress("192.168.174.130", 27018), + new ServerAddress("192.168.174.130", 27019))); } @Test @Ignore("CI infrastructure does not yet support replica sets") public void testMongoWithReplicaSets() { + Mongo mongo = ctx.getBean(Mongo.class); assertEquals(2, mongo.getAllAddress().size()); List servers = mongo.getAllAddress(); @@ -88,6 +88,5 @@ public class MongoNamespaceReplicaSetTests extends NamespaceTestSupport { MongoTemplate template = new MongoTemplate(mongo, "admin"); CommandResult result = template.executeCommand("{replSetGetStatus : 1}"); assertEquals("blort", result.getString("set")); - } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/NamespaceTestSupport.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/NamespaceTestSupport.java deleted file mode 100644 index bc5faf1fd..000000000 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/NamespaceTestSupport.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2011 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.config; - -import java.lang.reflect.Field; - -public class NamespaceTestSupport { - - @SuppressWarnings({ "unchecked" }) - public static T readField(String name, Object target) throws Exception { - Field field = null; - Class clazz = target.getClass(); - do { - try { - field = clazz.getDeclaredField(name); - } catch (Exception ex) { - } - - clazz = clazz.getSuperclass(); - } while (field == null && !clazz.equals(Object.class)); - - if (field == null) - throw new IllegalArgumentException("Cannot find field '" + name + "' in the class hierarchy of " - + target.getClass()); - field.setAccessible(true); - return (T) field.get(target); - } -} 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 new file mode 100644 index 000000000..3271b1da1 --- /dev/null +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/config/ServerAddressPropertyEditorUnitTests.java @@ -0,0 +1,80 @@ +/* + * 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.config; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import java.net.UnknownHostException; +import java.util.Arrays; +import java.util.Collection; + +import org.junit.Before; +import org.junit.Test; + +import com.mongodb.ServerAddress; + +/** + * Unit tests for {@link ServerAddressPropertyEditor}. + * + * @author Oliver Gierke + */ +public class ServerAddressPropertyEditorUnitTests { + + ServerAddressPropertyEditor editor; + + @Before + public void setUp() { + editor = new ServerAddressPropertyEditor(); + } + + /** + * @see DATAMONGO-454 + */ + @Test(expected = IllegalArgumentException.class) + public void rejectsAddressConfigWithoutASingleParsableServerAddress() { + + editor.setAsText("foo, bar"); + } + + /** + * @see DATAMONGO-454 + */ + @Test + public void skipsUnparsableAddressIfAtLeastOneIsParsable() throws UnknownHostException { + + editor.setAsText("foo, localhost"); + assertSingleAddressOfLocalhost(editor.getValue()); + } + + /** + * @see DATAMONGO-454 + */ + @Test + public void handlesEmptyAddressAsParseError() throws UnknownHostException { + + editor.setAsText(", localhost"); + assertSingleAddressOfLocalhost(editor.getValue()); + } + + private static void assertSingleAddressOfLocalhost(Object result) throws UnknownHostException { + + assertThat(result, is(instanceOf(ServerAddress[].class))); + Collection addresses = Arrays.asList((ServerAddress[]) result); + assertThat(addresses, hasSize(1)); + assertThat(addresses, hasItem(new ServerAddress("localhost"))); + } +}