INT-3654: Rework TCP Connection Close

JIRA: https://jira.spring.io/browse/INT-3654

Previously, single-use `TcpConnections` self-closed when their use was complete.
This is unnatural and caused issues such as INT-3722.

Remove the self-closing behavior; connection close is now (properly) the responsibility
of the client using the connection:

- Client Side:
-- `TcpOutboundGateway` after the reply is received
-- `TcpSendingMessageHandler` after the send, when there is no collaborating inbound adapter
-- `TcpReceivingChannelAdapter` when it is collaborating (after receiving the reply)

- Server Side:
-- `TcpInboundGateway` after the reply is sent
-- `TcpReceivingChannelAdapter` after the receive, when there is no collaborating outbound adapter
-- `TcpSendingMessageHandler` when it is colllaborating (after sending the reply)

As before, the `CachingClientConnectionFactory` always sets single use on the target factory
to force it to create new connections on demand. It is always a single-use factory itself
so the clients return the connections to the pool (via `close()`).

__Needs a migration guide entry__

INT-3654: Fix Late Listener Registration

Timing failures in `TcpOutboundGatewayTests.testFailoverCached()` (null listener).

The `FailoverClientConnectionFactory` registers its listener with the actual
connections it retrieves from a delegate.

When nesting failover and cached connection factories, we need to propagate the
`enableManualListenerRegistration` to the delegate factories so the connection
will wait for its listener to be registered.
This commit is contained in:
Gary Russell
2015-06-16 15:51:39 -04:00
committed by Artem Bilan
parent 1dd17ad319
commit ec5230abc7
24 changed files with 281 additions and 378 deletions

View File

@@ -71,7 +71,6 @@ import org.springframework.integration.ip.udp.MulticastReceivingChannelAdapter;
import org.springframework.integration.ip.udp.MulticastSendingMessageHandler;
import org.springframework.integration.ip.udp.UnicastReceivingChannelAdapter;
import org.springframework.integration.ip.udp.UnicastSendingMessageHandler;
import org.springframework.integration.support.MessageBuilderFactory;
import org.springframework.integration.test.util.TestUtils;
import org.springframework.messaging.Message;
import org.springframework.messaging.MessageChannel;

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2013 the original author or authors.
* Copyright 2002-2015 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -32,7 +32,6 @@ import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.context.support.AbstractApplicationContext;
import org.springframework.context.support.ClassPathXmlApplicationContext;
import org.springframework.messaging.Message;
import org.springframework.integration.channel.QueueChannel;
import org.springframework.integration.history.MessageHistory;
import org.springframework.integration.ip.tcp.connection.AbstractClientConnectionFactory;
@@ -46,6 +45,7 @@ import org.springframework.integration.ip.tcp.serializer.ByteArrayRawSerializer;
import org.springframework.integration.ip.util.TestingUtilities;
import org.springframework.integration.support.MessageBuilder;
import org.springframework.integration.test.util.TestUtils;
import org.springframework.messaging.Message;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -169,6 +169,7 @@ public class ConnectionToConnectionTests {
clientNet.start();
TcpConnection connection = clientNet.getConnection();
connection.send(MessageBuilder.withPayload("Test").build());
connection.close();
Message<?> message = serverSideChannel.receive(10000);
assertNotNull(message);
MessageHistory history = MessageHistory.read(message);

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2012 the original author or authors.
* Copyright 2002-2015 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -29,7 +29,7 @@ public class SyslogdTests {
AbstractApplicationContext ctx = new ClassPathXmlApplicationContext("SyslogdTests-context.xml", SyslogdTests.class);
System.out.println("Hit enter to terminate");
System.in.read();
ctx.destroy();
ctx.close();
}
}

View File

@@ -19,6 +19,7 @@ package org.springframework.integration.ip.tcp;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.doThrow;
@@ -51,6 +52,7 @@ import javax.net.ServerSocketFactory;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hamcrest.Matchers;
import org.junit.Test;
import org.mockito.Mockito;
@@ -61,7 +63,6 @@ import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.integration.MessageTimeoutException;
import org.springframework.integration.channel.QueueChannel;
import org.springframework.integration.ip.tcp.connection.AbstractClientConnectionFactory;
import org.springframework.integration.ip.tcp.connection.AbstractConnectionFactory;
import org.springframework.integration.ip.tcp.connection.CachingClientConnectionFactory;
import org.springframework.integration.ip.tcp.connection.FailoverClientConnectionFactory;
import org.springframework.integration.ip.tcp.connection.TcpConnectionSupport;
@@ -85,7 +86,7 @@ public class TcpOutboundGatewayTests {
@Test
public void testGoodNetSingle() throws Exception {
final int port = SocketUtils.findAvailableServerSocket();
AbstractConnectionFactory ccf = new TcpNetClientConnectionFactory("localhost", port);
AbstractClientConnectionFactory ccf = new TcpNetClientConnectionFactory("localhost", port);
final CountDownLatch latch = new CountDownLatch(1);
final AtomicBoolean done = new AtomicBoolean();
final AtomicReference<ServerSocket> serverSocket = new AtomicReference<ServerSocket>();
@@ -175,7 +176,7 @@ public class TcpOutboundGatewayTests {
}
}
});
AbstractConnectionFactory ccf = new TcpNetClientConnectionFactory("localhost", port);
AbstractClientConnectionFactory ccf = new TcpNetClientConnectionFactory("localhost", port);
ccf.setSerializer(new DefaultSerializer());
ccf.setDeserializer(new DefaultDeserializer());
ccf.setSoTimeout(10000);
@@ -230,7 +231,7 @@ public class TcpOutboundGatewayTests {
}
}
});
AbstractConnectionFactory ccf = new TcpNetClientConnectionFactory("localhost", port);
AbstractClientConnectionFactory ccf = new TcpNetClientConnectionFactory("localhost", port);
ccf.setSerializer(new DefaultSerializer());
ccf.setDeserializer(new DefaultDeserializer());
ccf.setSoTimeout(10000);
@@ -316,7 +317,8 @@ public class TcpOutboundGatewayTests {
* own response, not that for the first.
* @throws Exception
*/
private void testGoodNetGWTimeoutGuts(final int port, AbstractConnectionFactory ccf) throws InterruptedException {
private void testGoodNetGWTimeoutGuts(final int port, AbstractClientConnectionFactory ccf)
throws InterruptedException {
final CountDownLatch latch = new CountDownLatch(1);
final AtomicBoolean done = new AtomicBoolean();
/*
@@ -548,6 +550,7 @@ public class TcpOutboundGatewayTests {
AbstractClientConnectionFactory factory1 = mock(AbstractClientConnectionFactory.class);
TcpConnectionSupport mockConn1 = makeMockConnection();
when(factory1.getConnection()).thenReturn(mockConn1);
when(factory1.isSingleUse()).thenReturn(true);
doThrow(new IOException("fail")).when(mockConn1).send(Mockito.any(Message.class));
CachingClientConnectionFactory cachingFactory1 = new CachingClientConnectionFactory(factory1, 1);
@@ -555,7 +558,7 @@ public class TcpOutboundGatewayTests {
factory2.setSerializer(new DefaultSerializer());
factory2.setDeserializer(new DefaultDeserializer());
factory2.setSoTimeout(10000);
factory2.setSingleUse(false);
factory2.setSingleUse(true);
CachingClientConnectionFactory cachingFactory2 = new CachingClientConnectionFactory(factory2, 1);
// Failover
@@ -563,6 +566,8 @@ public class TcpOutboundGatewayTests {
factories.add(cachingFactory1);
factories.add(cachingFactory2);
FailoverClientConnectionFactory failoverFactory = new FailoverClientConnectionFactory(factories);
failoverFactory.setSingleUse(true);
failoverFactory.afterPropertiesSet();
failoverFactory.start();
TcpOutboundGateway gateway = new TcpOutboundGateway();
@@ -693,7 +698,7 @@ public class TcpOutboundGatewayTests {
fail("expected failure");
}
catch (Exception e) {
assertTrue(e.getCause() instanceof EOFException);
assertThat(e.getCause(), Matchers.instanceOf(EOFException.class));
}
assertEquals(0, TestUtils.getPropertyValue(gateway, "pendingReplies", Map.class).size());
Message<?> reply = replyChannel.receive(0);

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2015 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -1162,7 +1162,7 @@ public class TcpSendingMessageHandlerTests extends AbstractTcpChannelAdapterTest
Message<?> m = inbound.receive(1000);
assertNotNull(m);
assertEquals(testPayload, new String((byte[]) m.getPayload()));
ctx.destroy();
ctx.close();
}
@Test

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2012 the original author or authors.
* Copyright 2002-2015 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -15,11 +15,13 @@
*/
package org.springframework.integration.ip.tcp;
import static org.junit.Assert.assertEquals;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.messaging.MessageChannel;
import org.springframework.messaging.MessageHandlingException;
@@ -49,7 +51,7 @@ public class TcpSendingNoSocketTests {
fail("Exception expected");
}
catch (MessageHandlingException e) {
assertEquals("Unable to find outbound socket", e.getMessage());
assertThat(e.getMessage(), startsWith("Unable to find outbound socket"));
}
}

View File

@@ -594,16 +594,20 @@ public class CachingClientConnectionFactoryTests {
TcpNetClientConnectionFactory out = new TcpNetClientConnectionFactory("localhost", port);
CachingClientConnectionFactory cache = new CachingClientConnectionFactory(out, 1);
cache.setSingleUse(false);
cache.setConnectionWaitTimeout(100);
cache.start();
TcpConnectionSupport connection1 = cache.getConnection();
connection1.send(new GenericMessage<String>("foo"));
connection1.close();
TcpConnectionSupport connection2 = cache.getConnection();
connection2.send(new GenericMessage<String>("foo"));
connection2.close();
assertTrue(latch1.await(10, TimeUnit.SECONDS));
assertSame(connectionIds.get(0), connectionIds.get(1));
for (int i = 0; i < 100; i++) {
TcpConnectionSupport connection = cache.getConnection();
connection.send(new GenericMessage<String>("foo"));
connection.close();
}
assertTrue(latch2.await(10, TimeUnit.SECONDS));
assertSame(connectionIds.get(0), connectionIds.get(101));
@@ -665,6 +669,7 @@ public class CachingClientConnectionFactoryTests {
@Override
public Void answer(InvocationOnMock invocation) throws Throwable {
invocation.callRealMethod();
String log = (String) invocation.getArguments()[0];
if (log.startsWith("Response")) {
Executors.newSingleThreadScheduledExecutor().execute(new Runnable() {

View File

@@ -298,13 +298,17 @@ public class FailoverClientConnectionFactoryTests {
private void testRealGuts(AbstractClientConnectionFactory client1, AbstractClientConnectionFactory client2,
AbstractServerConnectionFactory server1, AbstractServerConnectionFactory server2) throws Exception {
int port1;
int port2;
int port1 = 0;
int port2 = 0;
Executor exec = Executors.newCachedThreadPool();
client1.setTaskExecutor(exec);
client2.setTaskExecutor(exec);
server1.setTaskExecutor(exec);
server2.setTaskExecutor(exec);
client1.setBeanName("client1");
client2.setBeanName("client2");
server1.setBeanName("server1");
server2.setBeanName("server2");
ApplicationEventPublisher pub = new ApplicationEventPublisher() {
@Override
@@ -313,9 +317,9 @@ public class FailoverClientConnectionFactoryTests {
@Override
public void publishEvent(Object event) {
}
};
client1.setApplicationEventPublisher(pub);
client2.setApplicationEventPublisher(pub);
@@ -360,16 +364,21 @@ public class FailoverClientConnectionFactoryTests {
Message<String> message = new GenericMessage<String>("foo");
outGateway.setRemoteTimeout(120000);
outGateway.handleMessage(message);
Socket socket = getSocket(client1);
port1 = socket.getLocalPort();
Socket socket = null;
if (!singleUse) {
socket = getSocket(client1);
port1 = socket.getLocalPort();
}
assertTrue(singleUse | connectionId.get().contains(Integer.toString(port1)));
Message<?> replyMessage = replyChannel.receive(10000);
assertNotNull(replyMessage);
server1.stop();
TestingUtilities.waitUntilFactoryHasThisNumberOfConnections(client1, 0);
outGateway.handleMessage(message);
socket = getSocket(client2);
port2 = socket.getLocalPort();
if (!singleUse) {
socket = getSocket(client2);
port2 = socket.getLocalPort();
}
assertTrue(singleUse | connectionId.get().contains(Integer.toString(port2)));
replyMessage = replyChannel.receive(10000);
assertNotNull(replyMessage);

View File

@@ -98,16 +98,16 @@ import org.springframework.util.ReflectionUtils.FieldFilter;
public class TcpNioConnectionTests {
private final ApplicationEventPublisher nullPublisher = new ApplicationEventPublisher() {
@Override
public void publishEvent(ApplicationEvent event) {
}
@Override
public void publishEvent(Object event) {
}
};
@Test
@@ -306,7 +306,7 @@ public class TcpNioConnectionTests {
factory.processNioSelections(1, selector, null, connections);
assertEquals(0, connections.size()); // third is closed
assertEquals(0, TestUtils.getPropertyValue(factory, "connections", List.class).size());
assertEquals(0, TestUtils.getPropertyValue(factory, "connections", Map.class).size());
}
@Test
@@ -723,9 +723,9 @@ public class TcpNioConnectionTests {
@Override
public void publishEvent(Object event) {
}
});
final CountDownLatch assemblerLatch = new CountDownLatch(1);
final AtomicReference<Thread> assembler = new AtomicReference<Thread>();
@@ -752,7 +752,8 @@ public class TcpNioConnectionTests {
Socket socket = SocketFactory.getDefault().createSocket("localhost", port);
assertTrue(connectionLatch.await(10, TimeUnit.SECONDS));
TcpNioConnection connection = (TcpNioConnection) TestUtils.getPropertyValue(factory, "connections", List.class).get(0);
TcpNioConnection connection = (TcpNioConnection) TestUtils.getPropertyValue(factory, "connections", Map.class)
.values().iterator().next();
Log logger = spy(TestUtils.getPropertyValue(connection, "logger", Log.class));
DirectFieldAccessor dfa = new DirectFieldAccessor(connection);
dfa.setPropertyValue("logger", logger);