Skip to content

Commit

Permalink
GEODE-7597: Factor out code needed in Membership from SocketCreator (a…
Browse files Browse the repository at this point in the history
…pache#4637)

* GEODE-7597: Factor out code needed in Membership from SocketCreator

Implemented a new TcpSocketCreatorImpl.  Due to GeodeHttpClientRule
dependencies this and associated interfaces are in geode-common.  I'd
rather have them in geode-tcp-server.

SocketCreator now subclasses TcpSocketCreatorImpl and I've removed the
adapter class, which is no longer needed.

* added missing files

* fixing dunit failure and moving TcpSocketCreator back to the tcp-server project

* fixing bind address and inheritance problems in socket creators

* fixes for unit test failures

* spotless

* relax assertion in test that fails stress-testing
  • Loading branch information
bschuchardt authored Jan 29, 2020
1 parent 11048af commit 434b5b0
Show file tree
Hide file tree
Showing 36 changed files with 427 additions and 471 deletions.
1 change: 1 addition & 0 deletions geode-assembly/geode-assembly-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ dependencies {
compileOnly(project(':geode-pulse'))
implementation(project(':geode-logging'))
implementation(project(':geode-membership'))
compileOnly(project(':geode-tcp-server'))
compileOnly('com.fasterxml.jackson.core:jackson-databind')
compileOnly('commons-io:commons-io')
compileOnly('junit:junit')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
*/
package org.apache.geode.cache.client.internal;

import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;

import java.io.IOException;
Expand Down Expand Up @@ -178,10 +177,8 @@ private QueueConnectionResponse issueQueueConnectionRequest(final String hostNam
private Object issueRequest(final String hostName, final int locatorPort,
final Object request, final boolean replyExpected)
throws IOException, ClassNotFoundException {
return new TcpClient(
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR)),
return new TcpClient(SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer())
.requestToServer(InetAddress.getByName(hostName),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -160,10 +159,8 @@ public void tearDown() {

private void issueStopRequest(final int port)
throws ConnectException, UnknownHostException {
new TcpClient(
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR)),
new TcpClient(SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer())
.stop(InetAddress.getLocalHost(), port);
Expand Down Expand Up @@ -338,9 +335,8 @@ public void test_DiscoverLocators_whenOneLocatorWasShutdown() throws Exception {
"tcp server", new ProtocolCheckerImpl(null, new ClientProtocolServiceLoader()),
DistributionStats::getStatTime,
Executors::newCachedThreadPool,
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR)),
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer(),
GeodeGlossary.GEMFIRE_PREFIX + "TcpServer.READ_TIMEOUT",
Expand Down Expand Up @@ -426,9 +422,8 @@ private void startFakeLocator() throws IOException, InterruptedException {
"Tcp Server", new ProtocolCheckerImpl(null, new ClientProtocolServiceLoader()),
DistributionStats::getStatTime,
Executors::newCachedThreadPool,
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR)),
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer(),
GeodeGlossary.GEMFIRE_PREFIX + "TcpServer.READ_TIMEOUT",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static org.apache.geode.distributed.ConfigurationProperties.LOCATOR_WAIT_TIME;
import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.internal.AvailablePort.SOCKET;
import static org.apache.geode.internal.AvailablePort.getRandomAvailablePort;
import static org.apache.geode.util.internal.GeodeGlossary.GEMFIRE_PREFIX;
Expand Down Expand Up @@ -171,10 +170,8 @@ public void testHandlersAreWaitedOn() throws Exception {
public void testBasicInfo() throws Exception {
locator = Locator.startLocator(port, tmpFile);
int boundPort = port == 0 ? locator.getPort() : port;
TcpClient client = new TcpClient(
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR)),
TcpClient client = new TcpClient(SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer());
String[] info = client.getInfo(InetAddress.getLocalHost(), boundPort);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
import static org.apache.geode.distributed.ConfigurationProperties.MEMBER_TIMEOUT;
import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -279,14 +278,12 @@ public Comparator<InternalDistributedMember> getComparator() {
}
};

final TcpClient locatorClient = new TcpClient(
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR)),
final TcpClient locatorClient = new TcpClient(SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer());
final TcpSocketCreator socketCreator = asTcpSocketCreator(SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.CLUSTER));
final TcpSocketCreator socketCreator = SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.CLUSTER);
final GMSAuthenticator authenticator =
new GMSAuthenticator(config.getSecurityProps(), securityService,
mockSystem.getSecurityLogWriter(), mockSystem.getInternalLogWriter());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
*/
package org.apache.geode.distributed.internal.membership.gms;

import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.doAnswer;
Expand Down Expand Up @@ -44,6 +43,7 @@
import org.apache.geode.distributed.internal.membership.api.MembershipLocatorBuilder;
import org.apache.geode.distributed.internal.tcpserver.TcpClient;
import org.apache.geode.distributed.internal.tcpserver.TcpSocketCreator;
import org.apache.geode.distributed.internal.tcpserver.TcpSocketCreatorImpl;
import org.apache.geode.internal.admin.SSLConfig;
import org.apache.geode.internal.net.SocketCreator;
import org.apache.geode.internal.serialization.DSFIDSerializer;
Expand All @@ -66,12 +66,12 @@ public void before() throws IOException, MembershipConfigurationException {
dsfidSerializer = new DSFIDSerializerFactory().create();

// TODO - using geode-core socket creator
socketCreator = asTcpSocketCreator(new SocketCreator(new SSLConfig.Builder().build()));
socketCreator = new SocketCreator(new SSLConfig.Builder().build());

final Supplier<ExecutorService> executorServiceSupplier =
() -> LoggingExecutors.newCachedThreadPool("membership", false);
membershipLocator = MembershipLocatorBuilder.<MemberIdentifierImpl>newLocatorBuilder(
socketCreator,
new TcpSocketCreatorImpl(),
dsfidSerializer,
temporaryFolder.newFolder("locator").toPath(),
executorServiceSupplier)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_TTL;
import static org.apache.geode.distributed.ConfigurationProperties.MEMBER_TIMEOUT;
import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
import static org.apache.geode.test.awaitility.GeodeAwaitility.getTimeout;
import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -1023,8 +1022,8 @@ public class GMSHealthMonitorTest extends GMSHealthMonitor {
public Set<MemberIdentifier> availabilityCheckedMembers = new HashSet<>();

public GMSHealthMonitorTest() {
super(asTcpSocketCreator(SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.CLUSTER)));
super(SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.CLUSTER));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static org.apache.geode.distributed.ConfigurationProperties.BIND_ADDRESS;
import static org.apache.geode.distributed.ConfigurationProperties.DISABLE_TCP;
import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.distributed.internal.membership.gms.locator.GMSLocator.LOCATOR_FILE_STAMP;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.catchThrowable;
Expand Down Expand Up @@ -95,10 +94,8 @@ public void setUp() throws Exception {
stateFile = new File(temporaryFolder.getRoot(), getClass().getSimpleName() + "_locator.dat");

gmsLocator = new GMSLocator(null, null, false, false, new LocatorStats(), "",
temporaryFolder.getRoot().toPath(), new TcpClient(
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR)),
temporaryFolder.getRoot().toPath(), new TcpClient(SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer()),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
Expand Down Expand Up @@ -189,9 +186,8 @@ public void testRecoverFromOther() throws Exception {
when(mockSystem.getConfig()).thenReturn(config);

final TcpClient locatorClient = new TcpClient(
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR)),
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_TTL;
import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand Down Expand Up @@ -180,8 +179,8 @@ private void initMocks(boolean enableMcast, Properties addProp) throws Exception

when(services.getStatistics()).thenReturn(mock(DistributionStats.class));

socketCreator = asTcpSocketCreator(SocketCreatorFactory.setDistributionConfig(config)
.getSocketCreatorForComponent(SecurableCommunicationChannel.CLUSTER));
socketCreator = SocketCreatorFactory.setDistributionConfig(config)
.getSocketCreatorForComponent(SecurableCommunicationChannel.CLUSTER);
messenger = new JGroupsMessenger<MemberIdentifier>();
messenger.init(services);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
*/
package org.apache.geode.distributed.internal.tcpserver;

import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.security.SecurableCommunicationChannels.LOCATOR;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
Expand Down Expand Up @@ -98,11 +97,9 @@ private void startServerAndClient(CertificateMaterial serverCertificate,

startTcpServer(serverProperties);

client = new TcpClient(
asTcpSocketCreator(
new SocketCreator(
SSLConfigurationFactory.getSSLConfigForComponent(clientProperties,
SecurableCommunicationChannel.LOCATOR))),
client = new TcpClient(new SocketCreator(
SSLConfigurationFactory.getSSLConfigForComponent(clientProperties,
SecurableCommunicationChannel.LOCATOR)),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer());
}
Expand All @@ -122,11 +119,10 @@ private void startTcpServer(Properties sslProperties) throws IOException {
(socket, input, firstByte) -> false,
DistributionStats::getStatTime,
Executors::newCachedThreadPool,
asTcpSocketCreator(
new SocketCreator(
SSLConfigurationFactory.getSSLConfigForComponent(
new DistributionConfigImpl(sslProperties),
SecurableCommunicationChannel.LOCATOR))),
new SocketCreator(
SSLConfigurationFactory.getSSLConfigForComponent(
new DistributionConfigImpl(sslProperties),
SecurableCommunicationChannel.LOCATOR)),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer(),
"not-a-system-property",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import static org.apache.geode.distributed.ConfigurationProperties.SSL_PROTOCOLS;
import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE;
import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTORE_PASSWORD;
import static org.apache.geode.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.test.util.ResourceUtils.createTempFileFromResource;
import static org.assertj.core.api.Assertions.assertThat;

Expand All @@ -44,7 +43,6 @@

import org.apache.geode.distributed.internal.DistributionConfigImpl;
import org.apache.geode.distributed.internal.DistributionStats;
import org.apache.geode.internal.AvailablePort;
import org.apache.geode.internal.InternalDataSerializer;
import org.apache.geode.internal.net.SSLConfigurationFactory;
import org.apache.geode.internal.net.SocketCreatorFactory;
Expand Down Expand Up @@ -78,7 +76,6 @@ public void setup() throws IOException {
expectedSocketTimeout);

localhost = InetAddress.getLocalHost();
port = AvailablePort.getRandomAvailablePort(AvailablePort.SOCKET);

socketCreator = new SocketCreatorFailHandshake(
SSLConfigurationFactory.getSSLConfigForComponent(
Expand All @@ -87,19 +84,20 @@ public void setup() throws IOException {
recordedSocketTimeouts);

server = new TcpServer(
port,
0,
localhost,
Mockito.mock(TcpHandler.class),
"server thread",
(socket, input, firstByte) -> false, DistributionStats::getStatTime,
Executors::newCachedThreadPool,
asTcpSocketCreator(socketCreator),
socketCreator,
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer(),
GeodeGlossary.GEMFIRE_PREFIX + "TcpServer.READ_TIMEOUT",
GeodeGlossary.GEMFIRE_PREFIX + "TcpServer.BACKLOG");

server.start();
port = server.getPort();
}

@After
Expand Down Expand Up @@ -130,7 +128,7 @@ public void testSSLSocketTimeOut() throws IOException, ClassNotFoundException {
// ok!
}

assertThat(recordedSocketTimeouts.size()).isEqualTo(1);
assertThat(recordedSocketTimeouts).hasSizeGreaterThan(0);

for (final Integer socketTimeOut : recordedSocketTimeouts) {
assertThat(Integer.parseInt(expectedSocketTimeout)).isEqualTo(socketTimeOut.intValue());
Expand All @@ -156,12 +154,10 @@ private Properties getSSLConfigurationProperties() {
}

private TcpClient getTcpClient() {
return new TcpClient(
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(
new DistributionConfigImpl(getSSLConfigurationProperties()),
SecurableCommunicationChannel.LOCATOR)),
return new TcpClient(SocketCreatorFactory
.getSocketCreatorForComponent(
new DistributionConfigImpl(getSSLConfigurationProperties()),
SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
* This test class will fail the TLS handshake with an SSLException, by default.
*/
public class SocketCreatorFailHandshake extends SocketCreator {

private final List<Integer> socketSoTimeouts;
private boolean failTLSHandshake;

Expand All @@ -46,9 +45,10 @@ public void setFailTLSHandshake(final boolean failTLSHandshake) {
@Override
public void handshakeIfSocketIsSSL(Socket socket, int timeout) throws IOException {
this.socketSoTimeouts.add(timeout);
if (failTLSHandshake)
if (failTLSHandshake) {
throw new SSLException("This is a test SSLException");
else
} else {
super.handshakeIfSocketIsSSL(socket, timeout);
}
}
}
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.distributed.internal.membership.adapter.TcpSocketCreatorAdapter.asTcpSocketCreator;
import static org.apache.geode.internal.net.InetAddressUtilsWithLogging.toInetAddress;
import static org.apache.geode.internal.net.InetAddressUtilsWithLogging.validateHost;

Expand Down Expand Up @@ -74,10 +73,8 @@ static DistributionLocatorConfig createConfigFor(String host, int port, InetAddr
String[] info = new String[] {"unknown", "unknown"};

try {
TcpClient client = new TcpClient(
asTcpSocketCreator(
SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR)),
TcpClient client = new TcpClient(SocketCreatorFactory
.getSocketCreatorForComponent(SecurableCommunicationChannel.LOCATOR),
InternalDataSerializer.getDSFIDSerializer().getObjectSerializer(),
InternalDataSerializer.getDSFIDSerializer().getObjectDeserializer());
if (bindAddress != null) {
Expand Down
Loading

0 comments on commit 434b5b0

Please sign in to comment.