GEODE-7808: standardize on use of HostAndPort for creating connections (

apache#4765)

* Squashed merge of feature/GEODE-7808

removed HostAddress
renamed LocatorAddress to HostAndPort
modified TcpClient methods to take a HostAndPort argument instead of
InetAddress
modified SocketCreator to take a HostAndPort argument instead of
InetAddress

* GEODE-7808 - standardize on use of HostAndPort for connection formation

This continues a previous PR that passed and was approved for merge.
This commit raises up several methods from SocketCreator into the
TcpSocketCreator interface.  This is an intermediate commit.  A
subsequent commit will refactor TcpSocketCreator to separate the client
and server methods for creating server-sockets and client connections to
server-sockets.

* refactored socket-creators to separate concerns

ServerSocketCreator holds methods for non-client comms
ClientSocketCreator holds methods that clients should use for comms
AdvancedSocketCreator holds methods for people who need to get around
the limitations of the other two interfaces

* adding missing interface

* move code out of inner-classes into first-class classes

* renaming interfaces and methods to be less confusing

* reinstate SocketCreator ip to hostname cache for performance

* changes from review comments

geode-assembly/src/acceptanceTest/java/org/apache/geode/metrics/CacheCommonTagsTest.java

@@ -30,7 +30,6 @@
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.inet.LocalHostUtil;
-import org.apache.geode.internal.net.SocketCreator;
 
 public class CacheCommonTagsTest {
 
@@ -57,7 +56,7 @@ public void metersHaveHostTag() throws UnknownHostException {
 
       assertThat(meter.getId().getTags())
           .as("Tags for meter with name " + meterId.getName())
-          .contains(Tag.of("host", SocketCreator.getHostName(LocalHostUtil.getLocalHost())));
+          .contains(Tag.of("host", LocalHostUtil.getLocalHost().getHostName()));
     }
   }
 

geode-core/src/distributedTest/java/org/apache/geode/cache/client/internal/LocatorLoadBalancingDUnitTest.java

@@ -18,7 +18,6 @@
 
 import java.io.IOException;
 import java.io.Serializable;
-import java.net.InetAddress;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -45,6 +44,7 @@
 import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.distributed.internal.ServerLocation;
 import org.apache.geode.distributed.internal.ServerLocator;
+import org.apache.geode.distributed.internal.tcpserver.HostAndPort;
 import org.apache.geode.distributed.internal.tcpserver.TcpClient;
 import org.apache.geode.internal.InternalDataSerializer;
 import org.apache.geode.internal.cache.CacheServerImpl;
@@ -181,8 +181,8 @@ private Object issueRequest(final String hostName, final int locatorPort,
         .getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
         InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
         InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer())
-            .requestToServer(InetAddress.getByName(hostName),
-                locatorPort,
+            .requestToServer(new HostAndPort(hostName,
+                locatorPort),
                 request,
                 10000, replyExpected);
   }

geode-core/src/distributedTest/java/org/apache/geode/cache30/ClientMembershipDUnitTest.java

@@ -838,9 +838,6 @@ public void memberCrashed(ClientMembershipEvent event) {
     ClientCache clientCache = (ClientCache) getCache();
     Set<InetSocketAddress> servers = clientCache.getCurrentServers();
     assertTrue(!servers.isEmpty());
-    InetSocketAddress serverAddr = servers.iterator().next();
-    InetSocketAddress expectedAddr = new InetSocketAddress(serverMember.getHost(), ports[0]);
-    assertEquals(expectedAddr, serverAddr);
 
     // now check listener results
     assertTrue(fired[JOINED]);

geode-core/src/integrationTest/java/org/apache/geode/cache/client/internal/AutoConnectionSourceImplJUnitTest.java

@@ -68,7 +68,7 @@
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.distributed.internal.ProtocolCheckerImpl;
 import org.apache.geode.distributed.internal.ServerLocation;
-import org.apache.geode.distributed.internal.tcpserver.LocatorAddress;
+import org.apache.geode.distributed.internal.tcpserver.HostAndPort;
 import org.apache.geode.distributed.internal.tcpserver.TcpClient;
 import org.apache.geode.distributed.internal.tcpserver.TcpHandler;
 import org.apache.geode.distributed.internal.tcpserver.TcpServer;
@@ -81,7 +81,6 @@
 import org.apache.geode.internal.security.SecurableCommunicationChannel;
 import org.apache.geode.management.membership.ClientMembershipEvent;
 import org.apache.geode.management.membership.ClientMembershipListener;
-import org.apache.geode.test.dunit.NetworkUtils;
 import org.apache.geode.test.junit.categories.ClientServerTest;
 import org.apache.geode.util.internal.GeodeGlossary;
 
@@ -123,8 +122,8 @@ public void setUp() throws Exception {
     InetAddress ia = InetAddress.getLocalHost();
     InetSocketAddress isa = new InetSocketAddress(ia, port);
     locators.add(isa);
-    List<LocatorAddress> la = new ArrayList<>();
-    la.add(new LocatorAddress(isa, ia.getHostName()));
+    List<HostAndPort> la = new ArrayList<>();
+    la.add(new HostAndPort(ia.getHostName(), port));
     source = new AutoConnectionSourceImpl(la, "", 60 * 1000);
     source.start(pool);
   }
@@ -163,7 +162,7 @@ private void issueStopRequest(final int port)
         .getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
         InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
         InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer())
-            .stop(InetAddress.getLocalHost(), port);
+            .stop(new HostAndPort(InetAddress.getLocalHost().getHostName(), port));
   }
 
   /**
@@ -179,18 +178,18 @@ public void testAddBadLocator() {
     InetSocketAddress floc2 = new InetSocketAddress("fakeLocalHost2", port);
     locators.add(floc1);
     locators.add(floc2);
-    List<LocatorAddress> la = new ArrayList<>();
-    la.add(new LocatorAddress(floc1, floc1.getHostName()));
-    la.add(new LocatorAddress(floc2, floc2.getHostName()));
+    List<HostAndPort> la = new ArrayList<>();
+    la.add(new HostAndPort(floc1.getHostName(), floc1.getPort()));
+    la.add(new HostAndPort(floc2.getHostName(), floc2.getPort()));
     AutoConnectionSourceImpl src = new AutoConnectionSourceImpl(la, "", 60 * 1000);
 
 
     InetSocketAddress b1 = new InetSocketAddress("fakeLocalHost1", port);
     InetSocketAddress b2 = new InetSocketAddress("fakeLocalHost3", port);
 
-    Set<LocatorAddress> bla = new HashSet<>();
-    bla.add(new LocatorAddress(b1, b1.getHostName()));
-    bla.add(new LocatorAddress(b2, b2.getHostName()));
+    Set<HostAndPort> bla = new HashSet<>();
+    bla.add(new HostAndPort(b1.getHostName(), b1.getPort()));
+    bla.add(new HostAndPort(b2.getHostName(), b2.getPort()));
 
 
     src.addbadLocators(la, bla);
@@ -212,13 +211,12 @@ public void testNoRespondingLocators() {
   @Test
   public void testSourceHandlesToDataException() throws IOException, ClassNotFoundException {
     TcpClient mockConnection = mock(TcpClient.class);
-    when(mockConnection.requestToServer(isA(InetSocketAddress.class), any(Object.class),
+    when(mockConnection.requestToServer(isA(HostAndPort.class), any(Object.class),
         isA(Integer.class), isA(Boolean.class))).thenThrow(new ToDataException("testing"));
     try {
-      InetSocketAddress address = new InetSocketAddress(NetworkUtils.getServerHostName(), 1234);
-      source.queryOneLocatorUsingConnection(new LocatorAddress(address, "locator[1234]"), mock(
+      source.queryOneLocatorUsingConnection(new HostAndPort("locator[1234]", 1234), mock(
           ServerLocationRequest.class), mockConnection);
-      verify(mockConnection).requestToServer(isA(InetSocketAddress.class),
+      verify(mockConnection).requestToServer(isA(HostAndPort.class),
           isA(ServerLocationRequest.class), isA(Integer.class), isA(Boolean.class));
     } catch (NoAvailableLocatorsException expected) {
       // do nothing

geode-core/src/integrationTest/java/org/apache/geode/distributed/LauncherIntegrationTestCase.java

@@ -293,7 +293,8 @@ private void givenPortInUse(final int port, InetAddress bindAddress) {
     try {
       socket = SocketCreatorFactory
           .createNonDefaultInstance(false, false, null, null, System.getProperties())
-          .createServerSocket(port, 50, bindAddress, -1);
+          .forCluster()
+          .createServerSocket(port, 50, bindAddress);
       assertThat(socket.isBound()).isTrue();
       assertThat(socket.isClosed()).isFalse();
       assertThat(isPortAvailable(port, SOCKET)).isFalse();

geode-core/src/integrationTest/java/org/apache/geode/distributed/LocatorIntegrationTest.java

@@ -29,7 +29,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.net.InetAddress;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
@@ -51,6 +50,7 @@
 
 import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.distributed.internal.tcpserver.HostAndPort;
 import org.apache.geode.distributed.internal.tcpserver.TcpClient;
 import org.apache.geode.internal.AvailablePortHelper;
 import org.apache.geode.internal.InternalDataSerializer;
@@ -174,7 +174,7 @@ public void testBasicInfo() throws Exception {
         .getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
         InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
         InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer());
-    String[] info = client.getInfo(InetAddress.getLocalHost(), boundPort);
+    String[] info = client.getInfo(new HostAndPort("localhost", boundPort));
 
     assertThat(info).isNotNull();
     assertThat(info.length).isGreaterThanOrEqualTo(1);

geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/tcpserver/TCPClientSSLIntegrationTest.java

@@ -146,7 +146,8 @@ public void clientConnectsIfServerCertificateHasHostname() throws Exception {
 
     startServerAndClient(serverCertificate, clientCertificate, true);
     String response =
-        (String) client.requestToServer(localhost, port, Boolean.valueOf(false), 5 * 1000);
+        (String) client.requestToServer(new HostAndPort(localhost.getHostName(), port),
+            Boolean.valueOf(false), 5 * 1000);
     assertThat(response).isEqualTo("Running!");
   }
 
@@ -165,7 +166,8 @@ public void clientChooseToDisableHasHostnameValidation() throws Exception {
 
     startServerAndClient(serverCertificate, clientCertificate, false);
     String response =
-        (String) client.requestToServer(localhost, port, Boolean.valueOf(false), 5 * 1000);
+        (String) client.requestToServer(new HostAndPort(localhost.getHostName(), port),
+            Boolean.valueOf(false), 5 * 1000);
     assertThat(response).isEqualTo("Running!");
   }
 
@@ -184,7 +186,8 @@ public void clientFailsToConnectIfServerCertificateNoHostname() throws Exception
     startServerAndClient(serverCertificate, clientCertificate, true);
 
     assertThatExceptionOfType(IllegalStateException.class)
-        .isThrownBy(() -> client.requestToServer(localhost, port, Boolean.valueOf(false), 5 * 1000))
+        .isThrownBy(() -> client.requestToServer(new HostAndPort(localhost.getHostName(), port),
+            Boolean.valueOf(false), 5 * 1000))
         .withCauseInstanceOf(SSLHandshakeException.class)
         .withStackTraceContaining("No name matching " + localhost.getHostName() + " found");
   }
@@ -205,7 +208,8 @@ public void clientFailsToConnectIfServerCertificateWrongHostname() throws Except
     startServerAndClient(serverCertificate, clientCertificate, true);
 
     assertThatExceptionOfType(IllegalStateException.class)
-        .isThrownBy(() -> client.requestToServer(localhost, port, Boolean.valueOf(false), 5 * 1000))
+        .isThrownBy(() -> client.requestToServer(new HostAndPort(localhost.getHostName(), port),
+            Boolean.valueOf(false), 5 * 1000))
         .withCauseInstanceOf(SSLHandshakeException.class)
         .withStackTraceContaining("No subject alternative DNS name matching "
             + localhost.getHostName() + " found.");

geode-core/src/integrationTest/java/org/apache/geode/distributed/internal/tcpserver/TCPServerSSLJUnitTest.java

@@ -109,7 +109,7 @@ public void teardown() throws ConnectException, InterruptedException {
      */
     socketCreator.setFailTLSHandshake(false);
 
-    getTcpClient().stop(localhost, port);
+    getTcpClient().stop(new HostAndPort(localhost.getHostAddress(), port));
 
     server.join(60 * 1000);
 
@@ -121,7 +121,8 @@ public void testSSLSocketTimeOut() throws IOException, ClassNotFoundException {
 
     try {
 
-      getTcpClient().requestToServer(localhost, port, Boolean.valueOf(false), 5 * 1000);
+      getTcpClient().requestToServer(new HostAndPort(localhost.getHostAddress(), port),
+          Boolean.valueOf(false), 5 * 1000);
       throw new AssertionError("expected to get an exception but didn't");
 
     } catch (final IllegalStateException | IOException t) {

geode-core/src/integrationTest/java/org/apache/geode/internal/net/ClientSocketFactoryIntegrationTest.java

@@ -35,6 +35,7 @@
 import org.apache.geode.distributed.ClientSocketFactory;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.DistributionConfigImpl;
+import org.apache.geode.distributed.internal.tcpserver.HostAndPort;
 import org.apache.geode.test.junit.categories.ClientServerTest;
 
 /**
@@ -82,7 +83,8 @@ public void tearDown() throws Exception {
   @Test
   public void testClientSocketFactory() throws Exception {
     assertThatThrownBy(() -> this.socket = SocketCreatorFactory
-        .getSocketCreatorForComponent(CLUSTER).connectForClient("localhost", 12345, 0))
+        .getSocketCreatorForComponent(CLUSTER).forClient()
+        .connect(new HostAndPort("localhost", 12345), 0))
             .isExactlyInstanceOf(IOException.class).hasMessage(EXCEPTION_MESSAGE);
 
     assertThat(invokedCreateSocket).isTrue();

geode-core/src/integrationTest/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java

@@ -72,6 +72,7 @@
 import org.apache.geode.distributed.internal.DMStats;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.DistributionConfigImpl;
+import org.apache.geode.distributed.internal.tcpserver.HostAndPort;
 import org.apache.geode.internal.ByteBufferOutputStream;
 import org.apache.geode.internal.inet.LocalHostUtil;
 import org.apache.geode.internal.security.SecurableCommunicationChannel;
@@ -176,11 +177,12 @@ public void socketCreatorShouldUseSsl() throws Exception {
 
   @Test
   public void securedSocketTransmissionShouldWork() throws Exception {
-    this.serverSocket = this.socketCreator.createServerSocket(0, 0, this.localHost);
+    this.serverSocket = this.socketCreator.forCluster().createServerSocket(0, 0, this.localHost);
     this.serverThread = startServer(this.serverSocket, 15000);
 
     int serverPort = this.serverSocket.getLocalPort();
-    this.clientSocket = this.socketCreator.connectForServer(this.localHost, serverPort);
+    this.clientSocket = this.socketCreator.forCluster()
+        .connect(new HostAndPort(this.localHost.getHostAddress(), serverPort), 0, null);
 
     // transmit expected string from Client to Server
     ObjectOutputStream output = new ObjectOutputStream(this.clientSocket.getOutputStream());
@@ -324,7 +326,7 @@ private void readMessageFromNIOSSLClient(Socket socket, ByteBuffer buffer, NioSs
 
   @Test(expected = SocketTimeoutException.class)
   public void handshakeCanTimeoutOnServer() throws Throwable {
-    this.serverSocket = this.socketCreator.createServerSocket(0, 0, this.localHost);
+    this.serverSocket = this.socketCreator.forCluster().createServerSocket(0, 0, this.localHost);
     this.serverThread = startServer(this.serverSocket, 1000);
 
     int serverPort = this.serverSocket.getLocalPort();
@@ -407,8 +409,9 @@ public void run() {
     try {
       await("connect to server socket").until(() -> {
         try {
-          Socket clientSocket = socketCreator.connectForClient(
-              LocalHostUtil.getLocalHost().getHostAddress(), serverSocketPort, 500);
+          Socket clientSocket = socketCreator.forClient().connect(
+              new HostAndPort(LocalHostUtil.getLocalHost().getHostAddress(), serverSocketPort),
+              500);
           clientSocket.close();
           System.err.println(
               "client successfully connected to server but should not have been able to do so");

geode-core/src/integrationTest/java/org/apache/geode/internal/statistics/GemFireStatSamplerIntegrationTest.java

@@ -34,7 +34,6 @@
 import static org.apache.geode.internal.cache.control.HeapMemoryMonitor.getTenuredMemoryPoolMXBean;
 import static org.apache.geode.internal.cache.control.HeapMemoryMonitor.getTenuredPoolStatistics;
 import static org.apache.geode.internal.inet.LocalHostUtil.getLocalHost;
-import static org.apache.geode.internal.net.SocketCreator.getHostName;
 import static org.apache.geode.internal.statistics.HostStatSampler.TEST_FILE_SIZE_LIMIT_IN_KB_PROPERTY;
 import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -136,7 +135,7 @@ public void testInitialization() throws Exception {
         .isLessThanOrEqualTo(currentTimeMillis());
     assertThat(statSampler.getSystemDirectoryPath())
         .as("system directory path")
-        .isEqualTo(getHostName(getLocalHost()));
+        .isEqualTo(getLocalHost().getHostName());
 
     assertThat(statSampler.getVMStats())
         .as("vm stats")

geode-core/src/integrationTest/java/org/apache/geode/internal/statistics/SimpleStatSamplerIntegrationTest.java

@@ -36,7 +36,6 @@
 import org.apache.geode.Statistics;
 import org.apache.geode.StatisticsType;
 import org.apache.geode.internal.inet.LocalHostUtil;
-import org.apache.geode.internal.net.SocketCreator;
 import org.apache.geode.internal.stats50.VMStats50;
 import org.apache.geode.test.junit.categories.StatisticsTest;
 
@@ -100,7 +99,7 @@ public void testBasics() throws Exception {
     assertTrue(statsCount > 0);
 
     assertTrue(statSampler.getSystemStartTime() <= System.currentTimeMillis());
-    assertEquals(SocketCreator.getHostName(LocalHostUtil.getLocalHost()),
+    assertEquals(LocalHostUtil.getLocalHost().getHostName(),
         statSampler.getSystemDirectoryPath());
 
     VMStatsContract vmStats = statSampler.getVMStats();

geode-core/src/integrationTest/java/org/apache/geode/net/SSLSocketParameterExtensionIntegrationTest.java

@@ -58,6 +58,7 @@
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.DistributionConfigImpl;
+import org.apache.geode.distributed.internal.tcpserver.HostAndPort;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.net.SocketCreator;
 import org.apache.geode.internal.net.SocketCreatorFactory;
@@ -141,11 +142,12 @@ public void tearDown() throws Exception {
 
   @Test
   public void securedSocketCheckExtensions() throws Exception {
-    this.serverSocket = this.socketCreator.createServerSocket(0, 0, this.localHost);
+    this.serverSocket = this.socketCreator.forCluster().createServerSocket(0, 0, this.localHost);
     this.serverThread = startServer(this.serverSocket, 15000);
 
     int serverPort = this.serverSocket.getLocalPort();
-    this.clientSocket = this.socketCreator.connectForServer(this.localHost, serverPort);
+    this.clientSocket = this.socketCreator.forCluster()
+        .connect(new HostAndPort(this.localHost.getHostAddress(), serverPort));
 
     SSLSocket sslSocket = (SSLSocket) this.clientSocket;
 
@@ -185,8 +187,9 @@ private Thread startServer(final ServerSocket serverSocket, int timeoutMillis) t
     Thread serverThread = new Thread(new MyThreadGroup(this.testName.getMethodName()), () -> {
       try {
         Socket socket = serverSocket.accept();
-        SocketCreatorFactory.getSocketCreatorForComponent(CLUSTER).handshakeIfSocketIsSSL(socket,
-            timeoutMillis);
+        SocketCreatorFactory.getSocketCreatorForComponent(CLUSTER).forCluster()
+            .handshakeIfSocketIsSSL(socket,
+                timeoutMillis);
         assertThat(socket.getSoTimeout()).isEqualTo(0);
 
         ObjectInputStream ois = new ObjectInputStream(socket.getInputStream());

geode-core/src/main/java/org/apache/geode/admin/internal/DistributionLocatorConfigImpl.java

@@ -15,7 +15,6 @@
 package org.apache.geode.admin.internal;
 
 import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
-import static org.apache.geode.internal.net.InetAddressUtilsWithLogging.toInetAddress;
 import static org.apache.geode.internal.net.InetAddressUtilsWithLogging.validateHost;
 
 import java.net.InetAddress;
@@ -24,6 +23,7 @@
 import org.apache.geode.GemFireConfigException;
 import org.apache.geode.admin.DistributionLocator;
 import org.apache.geode.admin.DistributionLocatorConfig;
+import org.apache.geode.distributed.internal.tcpserver.HostAndPort;
 import org.apache.geode.distributed.internal.tcpserver.TcpClient;
 import org.apache.geode.internal.InternalDataSerializer;
 import org.apache.geode.internal.net.SocketCreatorFactory;
@@ -65,8 +65,6 @@ public class DistributionLocatorConfigImpl extends ManagedEntityConfigImpl
    * Contacts a distribution locator on the given host and port and creates a
    * <code>DistributionLocatorConfig</code> for it.
    *
-   * @see TcpClient#getLocatorInfo
-   *
    * @return <code>null</code> if the locator cannot be contacted
    */
   static DistributionLocatorConfig createConfigFor(String host, int port, InetAddress bindAddress) {
@@ -78,9 +76,9 @@ static DistributionLocatorConfig createConfigFor(String host, int port, InetAddr
           InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
           InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer());
       if (bindAddress != null) {
-        info = client.getInfo(bindAddress, port);
+        info = client.getInfo(new HostAndPort(bindAddress.getHostAddress(), port));
       } else {
-        info = client.getInfo(toInetAddress(host), port);
+        info = client.getInfo(new HostAndPort(host, port));
       }
       if (info == null) {
         return null;