diff --git a/build.xml b/build.xml index 7f169ea37db..cfa3043fbde 100644 --- a/build.xml +++ b/build.xml @@ -39,7 +39,7 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant"> - + diff --git a/src/java/main/org/apache/zookeeper/client/HostProvider.java b/src/java/main/org/apache/zookeeper/client/HostProvider.java index ec3ca97f167..caeedcc4e1d 100644 --- a/src/java/main/org/apache/zookeeper/client/HostProvider.java +++ b/src/java/main/org/apache/zookeeper/client/HostProvider.java @@ -34,8 +34,9 @@ * * * The size() of a HostProvider may never be zero. * - * A HostProvider must return resolved InetSocketAddress instances on next(), - * but it's up to the HostProvider, when it wants to do the resolving. + * A HostProvider must return resolved InetSocketAddress instances on next() if the next address is resolvable. + * In that case, it's up to the HostProvider, whether it returns the next resolvable address in the list or return + * the next one as UnResolved. * * Different HostProvider could be imagined: * diff --git a/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java b/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java index cb539361fbc..0602103033f 100644 --- a/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java +++ b/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java @@ -22,6 +22,7 @@ import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -32,11 +33,19 @@ import org.slf4j.LoggerFactory; /** - * Most simple HostProvider, resolves only on instantiation. - * + * Most simple HostProvider, resolves on every next() call. + * + * Please be aware that although this class doesn't do any DNS caching, there're multiple levels of caching already + * present across the stack like in JVM, OS level, hardware, etc. The best we could do here is to get the most recent + * address from the underlying system which is considered up-to-date. + * */ @InterfaceAudience.Public public final class StaticHostProvider implements HostProvider { + public interface Resolver { + InetAddress[] getAllByName(String name) throws UnknownHostException; + } + private static final Logger LOG = LoggerFactory .getLogger(StaticHostProvider.class); @@ -64,6 +73,8 @@ public final class StaticHostProvider implements HostProvider { private float pOld, pNew; + private Resolver resolver; + /** * Constructs a SimpleHostSet. * @@ -73,15 +84,29 @@ public final class StaticHostProvider implements HostProvider { * if serverAddresses is empty or resolves to an empty list */ public StaticHostProvider(Collection serverAddresses) { - sourceOfRandomness = new Random(System.currentTimeMillis() ^ this.hashCode()); + init(serverAddresses, + System.currentTimeMillis() ^ this.hashCode(), + new Resolver() { + @Override + public InetAddress[] getAllByName(String name) throws UnknownHostException { + return InetAddress.getAllByName(name); + } + }); + } - this.serverAddresses = resolveAndShuffle(serverAddresses); - if (this.serverAddresses.isEmpty()) { - throw new IllegalArgumentException( - "A HostProvider may not be empty!"); - } - currentIndex = -1; - lastIndex = -1; + /** + * Constructs a SimpleHostSet. + * + * Introduced for testing purposes. getAllByName() is a static method of InetAddress, therefore cannot be easily mocked. + * By abstraction of Resolver interface we can easily inject a mocked implementation in tests. + * + * @param serverAddresses + * possibly unresolved ZooKeeper server addresses + * @param resolver + * custom resolver implementation + */ + public StaticHostProvider(Collection serverAddresses, Resolver resolver) { + init(serverAddresses, System.currentTimeMillis() ^ this.hashCode(), resolver); } /** @@ -96,36 +121,47 @@ public StaticHostProvider(Collection serverAddresses) { */ public StaticHostProvider(Collection serverAddresses, long randomnessSeed) { - sourceOfRandomness = new Random(randomnessSeed); + init(serverAddresses, randomnessSeed, new Resolver() { + @Override + public InetAddress[] getAllByName(String name) throws UnknownHostException { + return InetAddress.getAllByName(name); + } + }); + } - this.serverAddresses = resolveAndShuffle(serverAddresses); - if (this.serverAddresses.isEmpty()) { + private void init(Collection serverAddresses, long randomnessSeed, Resolver resolver) { + this.sourceOfRandomness = new Random(randomnessSeed); + this.resolver = resolver; + if (serverAddresses.isEmpty()) { throw new IllegalArgumentException( "A HostProvider may not be empty!"); - } + } + this.serverAddresses = shuffle(serverAddresses); currentIndex = -1; - lastIndex = -1; + lastIndex = -1; } - private List resolveAndShuffle(Collection serverAddresses) { - List tmpList = new ArrayList(serverAddresses.size()); - for (InetSocketAddress address : serverAddresses) { - try { - InetAddress ia = address.getAddress(); - String addr = (ia != null) ? ia.getHostAddress() : address.getHostString(); - InetAddress resolvedAddresses[] = InetAddress.getAllByName(addr); - for (InetAddress resolvedAddress : resolvedAddresses) { - InetAddress taddr = InetAddress.getByAddress(address.getHostString(), resolvedAddress.getAddress()); - tmpList.add(new InetSocketAddress(taddr, address.getPort())); - } - } catch (UnknownHostException ex) { - LOG.warn("No IP address found for server: {}", address, ex); + private InetSocketAddress resolve(InetSocketAddress address) { + try { + String curHostString = address.getHostString(); + List resolvedAddresses = new ArrayList<>(Arrays.asList(this.resolver.getAllByName(curHostString))); + if (resolvedAddresses.isEmpty()) { + return address; } + Collections.shuffle(resolvedAddresses); + return new InetSocketAddress(resolvedAddresses.get(0), address.getPort()); + } catch (UnknownHostException e) { + LOG.error("Unable to resolve address: {}", address.toString(), e); + return address; } + } + + private List shuffle(Collection serverAddresses) { + List tmpList = new ArrayList<>(serverAddresses.size()); + tmpList.addAll(serverAddresses); Collections.shuffle(tmpList, sourceOfRandomness); return tmpList; - } - + } /** * Update the list of servers. This returns true if changing connections is necessary for load-balancing, false @@ -149,15 +185,12 @@ private List resolveAndShuffle(Collection * @param currentHost the host to which this client is currently connected * @return true if changing connections is necessary for load-balancing, false otherwise */ - - @Override public synchronized boolean updateServerList( Collection serverAddresses, InetSocketAddress currentHost) { - // Resolve server addresses and shuffle them - List resolvedList = resolveAndShuffle(serverAddresses); - if (resolvedList.isEmpty()) { + List shuffledList = shuffle(serverAddresses); + if (shuffledList.isEmpty()) { throw new IllegalArgumentException( "A HostProvider may not be empty!"); } @@ -183,7 +216,7 @@ public synchronized boolean updateServerList( } } - for (InetSocketAddress addr : resolvedList) { + for (InetSocketAddress addr : shuffledList) { if (addr.getPort() == myServer.getPort() && ((addr.getAddress() != null && myServer.getAddress() != null && addr @@ -200,11 +233,11 @@ public synchronized boolean updateServerList( oldServers.clear(); // Divide the new servers into oldServers that were in the previous list // and newServers that were not in the previous list - for (InetSocketAddress resolvedAddress : resolvedList) { - if (this.serverAddresses.contains(resolvedAddress)) { - oldServers.add(resolvedAddress); + for (InetSocketAddress address : shuffledList) { + if (this.serverAddresses.contains(address)) { + oldServers.add(address); } else { - newServers.add(resolvedAddress); + newServers.add(address); } } @@ -245,11 +278,11 @@ public synchronized boolean updateServerList( } if (!reconfigMode) { - currentIndex = resolvedList.indexOf(getServerAtCurrentIndex()); + currentIndex = shuffledList.indexOf(getServerAtCurrentIndex()); } else { currentIndex = -1; } - this.serverAddresses = resolvedList; + this.serverAddresses = shuffledList; currentIndexOld = -1; currentIndexNew = -1; lastIndex = currentIndex; @@ -314,7 +347,7 @@ public InetSocketAddress next(long spinDelay) { addr = nextHostInReconfigMode(); if (addr != null) { currentIndex = serverAddresses.indexOf(addr); - return addr; + return resolve(addr); } //tried all servers and couldn't connect reconfigMode = false; @@ -339,7 +372,7 @@ public InetSocketAddress next(long spinDelay) { } } - return addr; + return resolve(addr); } public synchronized void onConnected() { diff --git a/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java b/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java index 10c6d1c5f03..998d6e59fc5 100644 --- a/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java +++ b/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java @@ -18,10 +18,6 @@ package org.apache.zookeeper.test; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertTrue; - import org.apache.zookeeper.ZKTestCase; import org.apache.zookeeper.client.HostProvider; import org.apache.zookeeper.client.StaticHostProvider; @@ -32,10 +28,29 @@ import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; +import java.util.Collections; +import java.util.List; import java.util.Random; +import static org.hamcrest.CoreMatchers.anyOf; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.hasItems; +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + public class StaticHostProviderTest extends ZKTestCase { private Random r = new Random(1); @@ -77,23 +92,28 @@ public void testNextDoesNotSleepForZero() { } @Test(expected = IllegalArgumentException.class) - public void testTwoInvalidHostAddresses() { - ArrayList list = new ArrayList(); - list.add(new InetSocketAddress("a...", 2181)); - list.add(new InetSocketAddress("b...", 2181)); - new StaticHostProvider(list); + public void testEmptyServerAddressesList() { + HostProvider hp = new StaticHostProvider(new ArrayList<>()); } @Test - public void testOneInvalidHostAddresses() { - Collection addr = getServerAddresses((byte) 1); - addr.add(new InetSocketAddress("a...", 2181)); + public void testInvalidHostAddresses() { + // Arrange + final List invalidAddresses = new ArrayList<>(); + InetSocketAddress unresolved = InetSocketAddress.createUnresolved("a", 1234); + invalidAddresses.add(unresolved); + StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() { + @Override + public InetAddress[] getAllByName(String name) throws UnknownHostException { + throw new UnknownHostException(); + } + }; + StaticHostProvider sp = new StaticHostProvider(invalidAddresses, resolver); - StaticHostProvider sp = new StaticHostProvider(addr); + // Act & Assert InetSocketAddress n1 = sp.next(0); - InetSocketAddress n2 = sp.next(0); - - assertEquals(n2, n1); + assertTrue("Provider should return unresolved address is host is unresolvable", n1.isUnresolved()); + assertSame("Provider should return original address is host is unresolvable", unresolved, n1); } @Test @@ -111,6 +131,8 @@ public void testOnConnectDoesNotReset() { assertNotSame(first, second); } + /* Reconfig tests with IP addresses */ + private final double slackPercent = 10; private final int numClients = 10000; @@ -123,12 +145,12 @@ public void testUpdateClientMigrateOrNot() throws UnknownHostException { // Number of machines becomes smaller, my server is in the new cluster boolean disconnectRequired = hostProvider.updateServerList(newList, myServer); - assertTrue(!disconnectRequired); + assertFalse(disconnectRequired); hostProvider.onConnected(); // Number of machines stayed the same, my server is in the new cluster disconnectRequired = hostProvider.updateServerList(newList, myServer); - assertTrue(!disconnectRequired); + assertFalse(disconnectRequired); hostProvider.onConnected(); // Number of machines became smaller, my server is not in the new @@ -170,7 +192,7 @@ public void testUpdateClientMigrateOrNot() throws UnknownHostException { } hostProvider.onConnected(); - // should be numClients/10 in expectation, we test that its numClients/10 +- slackPercent + // should be numClients/10 in expectation, we test that its numClients/10 +- slackPercent assertTrue(numDisconnects < upperboundCPS(numClients, 10)); } @@ -178,8 +200,7 @@ public void testUpdateClientMigrateOrNot() throws UnknownHostException { public void testUpdateMigrationGoesRound() throws UnknownHostException { HostProvider hostProvider = getHostProvider((byte) 4); // old list (just the ports): 1238, 1237, 1236, 1235 - Collection newList = new ArrayList( - 10); + Collection newList = new ArrayList(10); for (byte i = 12; i > 2; i--) { // 1246, 1245, 1244, 1243, 1242, 1241, // 1240, 1239, 1238, 1237 newList.add(new InetSocketAddress(InetAddress.getByAddress(new byte[]{10, 10, 10, i}), 1234 + i)); @@ -460,10 +481,7 @@ private StaticHostProvider getHostProvider(byte size) { return new StaticHostProvider(getServerAddresses(size), r.nextLong()); } - private HashMap> precomputedLists = new - HashMap>(); private Collection getServerAddresses(byte size) { - if (precomputedLists.containsKey(size)) return precomputedLists.get(size); ArrayList list = new ArrayList( size); while (size > 0) { @@ -475,10 +493,205 @@ private Collection getServerAddresses(byte size) { } --size; } - precomputedLists.put(size, list); return list; } + /* Reconfig test with unresolved hostnames */ + + /** + * Number of machines becomes smaller, my server is in the new cluster + */ + @Test + public void testUpdateServerList_UnresolvedHostnames_NoDisconnection1() { + // Arrange + // [testhost-4.testdomain.com:1238, testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235] + HostProvider hostProvider = getHostProviderWithUnresolvedHostnames(4); + // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235] + Collection newList = getUnresolvedHostnames(3); + InetSocketAddress myServer = InetSocketAddress.createUnresolved("testhost-3.testdomain.com", 1237); + + // Act + boolean disconnectRequired = hostProvider.updateServerList(newList, myServer); + + // Assert + assertFalse(disconnectRequired); + hostProvider.onConnected(); + } + + /** + * Number of machines stayed the same, my server is in the new cluster + */ + @Test + public void testUpdateServerList_UnresolvedHostnames_NoDisconnection2() { + // Arrange + // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235] + HostProvider hostProvider = getHostProviderWithUnresolvedHostnames(3); + // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235] + Collection newList = getUnresolvedHostnames(3); + InetSocketAddress myServer = InetSocketAddress.createUnresolved("testhost-3.testdomain.com", 1237); + + // Act + boolean disconnectRequired = hostProvider.updateServerList(newList, myServer); + + // Assert + assertFalse(disconnectRequired); + hostProvider.onConnected(); + } + + /** + * Number of machines became smaller, my server is not in the new cluster + */ + @Test + public void testUpdateServerList_UnresolvedHostnames_Disconnection1() { + // Arrange + // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235] + HostProvider hostProvider = getHostProviderWithUnresolvedHostnames(3); + // [testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235] + Collection newList = getUnresolvedHostnames(2); + InetSocketAddress myServer = InetSocketAddress.createUnresolved("testhost-3.testdomain.com", 1237); + + // Act + boolean disconnectRequired = hostProvider.updateServerList(newList, myServer); + + // Assert + assertTrue(disconnectRequired); + hostProvider.onConnected(); + } + + /** + * Number of machines stayed the same, my server is not in the new cluster + */ + @Test + public void testUpdateServerList_UnresolvedHostnames_Disconnection2() { + // Arrange + // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235] + HostProvider hostProvider = getHostProviderWithUnresolvedHostnames(3); + // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235] + Collection newList = getUnresolvedHostnames(3); + InetSocketAddress myServer = InetSocketAddress.createUnresolved("testhost-4.testdomain.com", 1237); + + // Act + boolean disconnectRequired = hostProvider.updateServerList(newList, myServer); + + // Assert + assertTrue(disconnectRequired); + hostProvider.onConnected(); + } + + @Test + public void testUpdateServerList_ResolvedWithUnResolvedAddress_ForceDisconnect() { + // Arrange + // Create a HostProvider with a list of unresolved server address(es) + List addresses = Collections.singletonList( + InetSocketAddress.createUnresolved("testhost-1.resolvable.zk", 1235) + ); + HostProvider hostProvider = new StaticHostProvider(addresses, new TestResolver()); + InetSocketAddress currentHost = hostProvider.next(100); + assertThat("CurrentHost is which the client is currently connecting to, it should be resolved", + currentHost.isUnresolved(), is(false)); + + // Act + InetSocketAddress replaceHost = InetSocketAddress.createUnresolved("testhost-1.resolvable.zk", 1235); + assertThat("Replace host must be unresolved in this test case", replaceHost.isUnresolved(), is(true)); + boolean disconnect = hostProvider.updateServerList(new ArrayList<>( + Collections.singletonList(replaceHost)), + currentHost + ); + + // Assert + assertThat(disconnect, is(false)); + } + + @Test + public void testUpdateServerList_ResolvedWithResolvedAddress_NoDisconnect() throws UnknownHostException { + // Arrange + // Create a HostProvider with a list of unresolved server address(es) + List addresses = Collections.singletonList( + InetSocketAddress.createUnresolved("testhost-1.resolvable.zk", 1235) + ); + HostProvider hostProvider = new StaticHostProvider(addresses, new TestResolver()); + InetSocketAddress currentHost = hostProvider.next(100); + assertThat("CurrentHost is which the client is currently connecting to, it should be resolved", + currentHost.isUnresolved(), is(false)); + + // Act + InetSocketAddress replaceHost = + new InetSocketAddress(InetAddress.getByAddress(currentHost.getHostString(), + currentHost.getAddress().getAddress()), currentHost.getPort()); + assertThat("Replace host must be resolved in this test case", replaceHost.isUnresolved(), is(false)); + boolean disconnect = hostProvider.updateServerList(new ArrayList<>( + Collections.singletonList(replaceHost)), + currentHost + ); + + // Assert + assertThat(disconnect, equalTo(false)); + } + + @Test + public void testUpdateServerList_UnResolvedWithUnResolvedAddress_ForceDisconnect() { + // Arrange + // Create a HostProvider with a list of unresolved server address(es) + List addresses = Collections.singletonList( + InetSocketAddress.createUnresolved("testhost-1.zookeepertest.zk", 1235) + ); + HostProvider hostProvider = new StaticHostProvider(addresses, new TestResolver()); + InetSocketAddress currentHost = hostProvider.next(100); + assertThat("CurrentHost is not resolvable in this test case", + currentHost.isUnresolved(), is(true)); + + // Act + InetSocketAddress replaceHost = InetSocketAddress.createUnresolved("testhost-1.resolvable.zk", 1235); + assertThat("Replace host must be unresolved in this test case", replaceHost.isUnresolved(), is(true)); + boolean disconnect = hostProvider.updateServerList(new ArrayList<>( + Collections.singletonList(replaceHost)), + currentHost + ); + + // Assert + assertThat(disconnect, is(true)); + } + + @Test + public void testUpdateServerList_UnResolvedWithResolvedAddress_ForceDisconnect() throws UnknownHostException { + // Arrange + // Create a HostProvider with a list of unresolved server address(es) + List addresses = Collections.singletonList( + InetSocketAddress.createUnresolved("testhost-1.zookeepertest.zk", 1235) + ); + HostProvider hostProvider = new StaticHostProvider(addresses, new TestResolver()); + InetSocketAddress currentHost = hostProvider.next(100); + assertThat("CurrentHost not resolvable in this test case", + currentHost.isUnresolved(), is(true)); + + // Act + byte[] addr = new byte[] { 10, 0, 0, 1 }; + InetSocketAddress replaceHost = + new InetSocketAddress(InetAddress.getByAddress(currentHost.getHostString(), addr), + currentHost.getPort()); + assertThat("Replace host must be resolved in this test case", replaceHost.isUnresolved(), is(false)); + boolean disconnect = hostProvider.updateServerList(new ArrayList<>( + Collections.singletonList(replaceHost)), + currentHost + ); + + // Assert + assertThat(disconnect, equalTo(false)); + } + + private class TestResolver implements StaticHostProvider.Resolver { + private byte counter = 1; + + @Override + public InetAddress[] getAllByName(String name) throws UnknownHostException { + if (name.contains("resolvable")) { + byte[] addr = new byte[] { 10, 0, 0, (byte)(counter++ % 10) }; + return new InetAddress[] { InetAddress.getByAddress(name, addr) }; + } + throw new UnknownHostException(); + } + } + private double lowerboundCPS(int numClients, int numServers) { return (1 - slackPercent/100.0) * numClients / numServers; } @@ -487,24 +700,210 @@ private double upperboundCPS(int numClients, int numServers) { return (1 + slackPercent/100.0) * numClients / numServers; } + /* DNS resolution tests */ + @Test - public void testLiteralIPNoReverseNS() throws Exception { + public void testLiteralIPNoReverseNS() { byte size = 30; HostProvider hostProvider = getHostProviderUnresolved(size); for (int i = 0; i < size; i++) { InetSocketAddress next = hostProvider.next(0); - assertTrue(next instanceof InetSocketAddress); - assertTrue(!next.isUnresolved()); - assertTrue(!next.toString().startsWith("/")); + assertThat(next, instanceOf(InetSocketAddress.class)); + assertFalse(next.isUnresolved()); + assertTrue(next.toString().startsWith("/")); // Do NOT trigger the reverse name service lookup. String hostname = next.getHostString(); // In this case, the hostname equals literal IP address. - hostname.equals(next.getAddress().getHostAddress()); + assertEquals(next.getAddress().getHostAddress(), hostname); + } + } + + @Test + public void testReResolvingSingle() throws UnknownHostException { + // Arrange + byte size = 1; + ArrayList list = new ArrayList(size); + + // Test a hostname that resolves to a single address + list.add(InetSocketAddress.createUnresolved("issues.apache.org", 1234)); + + final InetAddress issuesApacheOrg = mock(InetAddress.class); + when(issuesApacheOrg.getHostAddress()).thenReturn("192.168.1.1"); + when(issuesApacheOrg.toString()).thenReturn("issues.apache.org"); + when(issuesApacheOrg.getHostName()).thenReturn("issues.apache.org"); + + StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() { + @Override + public InetAddress[] getAllByName(String name) { + return new InetAddress[] { + issuesApacheOrg + }; + } + }; + StaticHostProvider.Resolver spyResolver = spy(resolver); + + // Act + StaticHostProvider hostProvider = new StaticHostProvider(list, spyResolver); + for (int i = 0; i < 10; i++) { + InetSocketAddress next = hostProvider.next(0); + assertEquals(issuesApacheOrg, next.getAddress()); + } + + // Assert + // Resolver called 10 times, because we shouldn't cache the resolved addresses + verify(spyResolver, times(10)).getAllByName("issues.apache.org"); // resolution occurred + } + + @Test + public void testReResolvingMultiple() throws UnknownHostException { + // Arrange + byte size = 1; + ArrayList list = new ArrayList(size); + + // Test a hostname that resolves to multiple addresses + list.add(InetSocketAddress.createUnresolved("www.apache.org", 1234)); + + final InetAddress apacheOrg1 = mock(InetAddress.class); + when(apacheOrg1.getHostAddress()).thenReturn("192.168.1.1"); + when(apacheOrg1.toString()).thenReturn("www.apache.org"); + when(apacheOrg1.getHostName()).thenReturn("www.apache.org"); + + final InetAddress apacheOrg2 = mock(InetAddress.class); + when(apacheOrg2.getHostAddress()).thenReturn("192.168.1.2"); + when(apacheOrg2.toString()).thenReturn("www.apache.org"); + when(apacheOrg2.getHostName()).thenReturn("www.apache.org"); + + final List resolvedAddresses = new ArrayList(); + resolvedAddresses.add(apacheOrg1); + resolvedAddresses.add(apacheOrg2); + StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() { + @Override + public InetAddress[] getAllByName(String name) { + return resolvedAddresses.toArray(new InetAddress[resolvedAddresses.size()]); + } + }; + StaticHostProvider.Resolver spyResolver = spy(resolver); + + // Act & Assert + StaticHostProvider hostProvider = new StaticHostProvider(list, spyResolver); + assertEquals(1, hostProvider.size()); // single address not extracted + + for (int i = 0; i < 10; i++) { + InetSocketAddress next = hostProvider.next(0); + assertThat("Bad IP address returned", next.getAddress().getHostAddress(), anyOf(equalTo(apacheOrg1.getHostAddress()), equalTo(apacheOrg2.getHostAddress()))); + assertEquals(1, hostProvider.size()); // resolve() call keeps the size of provider } + // Resolver called 10 times, because we shouldn't cache the resolved addresses + verify(spyResolver, times(10)).getAllByName("www.apache.org"); // resolution occurred } - private StaticHostProvider getHostProviderUnresolved(byte size) - throws UnknownHostException { + @Test + public void testReResolveMultipleOneFailing() throws UnknownHostException { + // Arrange + final List list = new ArrayList(); + list.add(InetSocketAddress.createUnresolved("www.apache.org", 1234)); + final List ipList = new ArrayList(); + final List resolvedAddresses = new ArrayList(); + for (int i = 0; i < 3; i++) { + ipList.add(String.format("192.168.1.%d", i+1)); + final InetAddress apacheOrg = mock(InetAddress.class); + when(apacheOrg.getHostAddress()).thenReturn(String.format("192.168.1.%d", i+1)); + when(apacheOrg.toString()).thenReturn(String.format("192.168.1.%d", i+1)); + when(apacheOrg.getHostName()).thenReturn("www.apache.org"); + resolvedAddresses.add(apacheOrg); + } + + StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() { + @Override + public InetAddress[] getAllByName(String name) { + return resolvedAddresses.toArray(new InetAddress[resolvedAddresses.size()]); + } + }; + StaticHostProvider.Resolver spyResolver = spy(resolver); + StaticHostProvider hostProvider = new StaticHostProvider(list, spyResolver); + + // Act & Assert + InetSocketAddress resolvedFirst = hostProvider.next(0); + assertFalse("HostProvider should return resolved addresses", resolvedFirst.isUnresolved()); + assertThat("Bad IP address returned", ipList, hasItems(resolvedFirst.getAddress().getHostAddress())); + + hostProvider.onConnected(); // first address worked + + InetSocketAddress resolvedSecond = hostProvider.next(0); + assertFalse("HostProvider should return resolved addresses", resolvedSecond.isUnresolved()); + assertThat("Bad IP address returned", ipList, hasItems(resolvedSecond.getAddress().getHostAddress())); + + // Second address doesn't work, so we don't call onConnected() this time + // StaticHostProvider should try to re-resolve the address in this case + + InetSocketAddress resolvedThird = hostProvider.next(0); + assertFalse("HostProvider should return resolved addresses", resolvedThird.isUnresolved()); + assertThat("Bad IP address returned", ipList, hasItems(resolvedThird.getAddress().getHostAddress())); + + verify(spyResolver, times(3)).getAllByName("www.apache.org"); // resolution occured every time + } + + @Test + public void testEmptyResolution() throws UnknownHostException { + // Arrange + final List list = new ArrayList(); + list.add(InetSocketAddress.createUnresolved("www.apache.org", 1234)); + list.add(InetSocketAddress.createUnresolved("www.google.com", 1234)); + final List resolvedAddresses = new ArrayList(); + + final InetAddress apacheOrg1 = mock(InetAddress.class); + when(apacheOrg1.getHostAddress()).thenReturn("192.168.1.1"); + when(apacheOrg1.toString()).thenReturn("www.apache.org"); + when(apacheOrg1.getHostName()).thenReturn("www.apache.org"); + + resolvedAddresses.add(apacheOrg1); + + StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() { + @Override + public InetAddress[] getAllByName(String name) { + if ("www.apache.org".equalsIgnoreCase(name)) { + return resolvedAddresses.toArray(new InetAddress[resolvedAddresses.size()]); + } else { + return new InetAddress[0]; + } + } + }; + StaticHostProvider.Resolver spyResolver = spy(resolver); + StaticHostProvider hostProvider = new StaticHostProvider(list, spyResolver); + + // Act & Assert + for (int i = 0; i < 10; i++) { + InetSocketAddress resolved = hostProvider.next(0); + hostProvider.onConnected(); + if (resolved.getHostName().equals("www.google.com")) { + assertTrue("HostProvider should return unresolved address if host is unresolvable", resolved.isUnresolved()); + } else { + assertFalse("HostProvider should return resolved addresses", resolved.isUnresolved()); + assertEquals("192.168.1.1", resolved.getAddress().getHostAddress()); + } + } + + verify(spyResolver, times(5)).getAllByName("www.apache.org"); + verify(spyResolver, times(5)).getAllByName("www.google.com"); + } + + @Test + public void testReResolvingLocalhost() { + byte size = 2; + ArrayList list = new ArrayList(size); + + // Test a hostname that resolves to multiple addresses + list.add(InetSocketAddress.createUnresolved("localhost", 1234)); + list.add(InetSocketAddress.createUnresolved("localhost", 1235)); + StaticHostProvider hostProvider = new StaticHostProvider(list); + int sizeBefore = hostProvider.size(); + InetSocketAddress next = hostProvider.next(0); + next = hostProvider.next(0); + assertTrue("Different number of addresses in the list: " + hostProvider.size() + + " (after), " + sizeBefore + " (before)", hostProvider.size() == sizeBefore); + } + + private StaticHostProvider getHostProviderUnresolved(byte size) { return new StaticHostProvider(getUnresolvedServerAddresses(size), r.nextLong()); } @@ -516,4 +915,18 @@ private Collection getUnresolvedServerAddresses(byte size) { } return list; } + + private StaticHostProvider getHostProviderWithUnresolvedHostnames(int size) { + return new StaticHostProvider(getUnresolvedHostnames(size), r.nextLong()); + } + + private Collection getUnresolvedHostnames(int size) { + ArrayList list = new ArrayList<>(size); + while (size > 0) { + list.add(InetSocketAddress.createUnresolved(String.format("testhost-%d.testdomain.com", size), 1234 + size)); + --size; + } + System.out.println(Arrays.toString(list.toArray())); + return list; + } }