Skip to content

Commit

Permalink
GEODE-7808: standardize on use of HostAndPort to form client-side con…
Browse files Browse the repository at this point in the history
…nections (apache#4743)

* 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
  • Loading branch information
bschuchardt authored Mar 2, 2020
1 parent cceaa66 commit 0af6264
Show file tree
Hide file tree
Showing 91 changed files with 1,431 additions and 1,002 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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()));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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));
}

/**
Expand All @@ -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);
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!");
}

Expand All @@ -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!");
}

Expand All @@ -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");
}
Expand All @@ -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.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand Down
Loading

0 comments on commit 0af6264

Please sign in to comment.