From ea6cf0f4ef58534216fa1126d739b58169c901a7 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 17 May 2017 14:18:49 -0400 Subject: [PATCH] INT-4245: Improve ReplyProducingMHWrapper JIRA: https://jira.spring.io/browse/INT-4245 * Move `ReplyProducingMessageHandlerWrapper` outside of `ServiceActivatorAnnotationPostProcessor`. * Use it from the `ServiceActivatorFactoryBean` as well to fix the missed `Lifecycle` control * Increase wait timeout in the `ConnectionFactoryTests` * Mark endpoint as `auto-startup="false"` in the SFTP parser tests * Remove redundant `org.gradle.daemon=true` from the `gradle.properties` since it is like that by default for a while already --- gradle.properties | 1 - .../config/ServiceActivatorFactoryBean.java | 22 +++--- ...rviceActivatorAnnotationPostProcessor.java | 42 +---------- .../ReplyProducingMessageHandlerWrapper.java | 71 +++++++++++++++++++ .../connection/ConnectionFactoryTests.java | 2 +- ...annelAdapterParserCachingTests-context.xml | 29 +++----- ...boundChannelAdapterParserTests-context.xml | 3 +- .../config/SftpInboundAutostartup-context.xml | 2 +- 8 files changed, 96 insertions(+), 76 deletions(-) create mode 100644 spring-integration-core/src/main/java/org/springframework/integration/handler/ReplyProducingMessageHandlerWrapper.java diff --git a/gradle.properties b/gradle.properties index 9136e989dd..0114fb7771 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,2 +1 @@ version=5.0.0.BUILD-SNAPSHOT -org.gradle.daemon=true diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/ServiceActivatorFactoryBean.java b/spring-integration-core/src/main/java/org/springframework/integration/config/ServiceActivatorFactoryBean.java index 4def1768ad..ebc0634909 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/ServiceActivatorFactoryBean.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/ServiceActivatorFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -21,8 +21,8 @@ import org.springframework.integration.handler.AbstractMessageProducingHandler; import org.springframework.integration.handler.AbstractReplyProducingMessageHandler; import org.springframework.integration.handler.ExpressionEvaluatingMessageProcessor; import org.springframework.integration.handler.MessageProcessor; +import org.springframework.integration.handler.ReplyProducingMessageHandlerWrapper; import org.springframework.integration.handler.ServiceActivatingHandler; -import org.springframework.messaging.Message; import org.springframework.messaging.MessageHandler; import org.springframework.util.StringUtils; @@ -32,6 +32,8 @@ import org.springframework.util.StringUtils; * @author Mark Fisher * @author Gary Russell * @author David Liu + * @author Artem Bilan + * * @since 2.0 */ public class ServiceActivatorFactoryBean extends AbstractStandardMessageHandlerFactoryBean { @@ -55,8 +57,8 @@ public class ServiceActivatorFactoryBean extends AbstractStandardMessageHandlerF if (handler == null) { handler = configureHandler( StringUtils.hasText(targetMethodName) - ? new ServiceActivatingHandler(targetObject, targetMethodName) - : new ServiceActivatingHandler(targetObject)); + ? new ServiceActivatingHandler(targetObject, targetMethodName) + : new ServiceActivatingHandler(targetObject)); } return handler; } @@ -80,15 +82,7 @@ public class ServiceActivatorFactoryBean extends AbstractStandardMessageHandlerF * Return a reply-producing message handler so that we still get 'produced no reply' messages * and the super class will inject the advice chain to advise the handler method if needed. */ - handler = new AbstractReplyProducingMessageHandler() { - - @Override - protected Object handleRequestMessage(Message requestMessage) { - - ((MessageHandler) targetObject).handleMessage(requestMessage); - return null; - } - }; + handler = new ReplyProducingMessageHandlerWrapper((MessageHandler) targetObject); } return handler; @@ -135,7 +129,7 @@ public class ServiceActivatorFactoryBean extends AbstractStandardMessageHandlerF else { if (this.requiresReply && logger.isDebugEnabled()) { logger.debug("requires-reply can only be set to AbstractReplyProducingMessageHandler or its subclass, " - + handler.getComponentName() + " doesn't support it."); + + handler.getComponentName() + " doesn't support it."); } } } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java index aa2fe1480a..c9fabc7ec8 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -22,14 +22,13 @@ import java.util.Arrays; import java.util.List; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; -import org.springframework.context.Lifecycle; import org.springframework.context.annotation.Bean; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.integration.annotation.ServiceActivator; import org.springframework.integration.handler.AbstractReplyProducingMessageHandler; +import org.springframework.integration.handler.ReplyProducingMessageHandlerWrapper; import org.springframework.integration.handler.ServiceActivatingHandler; import org.springframework.integration.util.MessagingAnnotationUtils; -import org.springframework.messaging.Message; import org.springframework.messaging.MessageHandler; import org.springframework.util.StringUtils; @@ -84,41 +83,4 @@ public class ServiceActivatorAnnotationPostProcessor extends AbstractMethodAnnot return serviceActivator; } - private static final class ReplyProducingMessageHandlerWrapper extends AbstractReplyProducingMessageHandler - implements Lifecycle { - - private final MessageHandler target; - - ReplyProducingMessageHandlerWrapper(MessageHandler target) { - this.target = target; - } - - @Override - protected Object handleRequestMessage(Message requestMessage) { - this.target.handleMessage(requestMessage); - return null; - } - - @Override - public void start() { - if (this.target instanceof Lifecycle) { - ((Lifecycle) this.target).start(); - } - - } - - @Override - public void stop() { - if (this.target instanceof Lifecycle) { - ((Lifecycle) this.target).stop(); - } - } - - @Override - public boolean isRunning() { - return !(this.target instanceof Lifecycle) || ((Lifecycle) this.target).isRunning(); - } - - } - } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/ReplyProducingMessageHandlerWrapper.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/ReplyProducingMessageHandlerWrapper.java new file mode 100644 index 0000000000..b0bdb02668 --- /dev/null +++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/ReplyProducingMessageHandlerWrapper.java @@ -0,0 +1,71 @@ +/* + * Copyright 2017 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.integration.handler; + +import org.springframework.context.Lifecycle; +import org.springframework.messaging.Message; +import org.springframework.messaging.MessageHandler; + +/** + * The {@link AbstractReplyProducingMessageHandler} wrapper around raw {@link MessageHandler} + * for request-reply scenarios, e.g. {@code @ServiceActivator} annotation configuration. + *

+ * This class is used internally by Framework in cased when request-reply is important + * and there is no other way to apply advice chain. + *

+ * The lifecycle control is delegated to the {@code target} {@link MessageHandler}. + * + * @author Artem Bilan + * + * @since 5.0 + */ +public class ReplyProducingMessageHandlerWrapper extends AbstractReplyProducingMessageHandler + implements Lifecycle { + + private final MessageHandler target; + + public ReplyProducingMessageHandlerWrapper(MessageHandler target) { + this.target = target; + } + + @Override + protected Object handleRequestMessage(Message requestMessage) { + this.target.handleMessage(requestMessage); + return null; + } + + @Override + public void start() { + if (this.target instanceof Lifecycle) { + ((Lifecycle) this.target).start(); + } + + } + + @Override + public void stop() { + if (this.target instanceof Lifecycle) { + ((Lifecycle) this.target).stop(); + } + } + + @Override + public boolean isRunning() { + return !(this.target instanceof Lifecycle) || ((Lifecycle) this.target).isRunning(); + } + +} diff --git a/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/ConnectionFactoryTests.java b/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/ConnectionFactoryTests.java index 13b913c5e3..cbc04dbc71 100644 --- a/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/ConnectionFactoryTests.java +++ b/spring-integration-ip/src/test/java/org/springframework/integration/ip/tcp/connection/ConnectionFactoryTests.java @@ -150,7 +150,7 @@ public class ConnectionFactoryTests extends LogAdjustingTestSupport { List clients = clientFactory.getOpenConnectionIds(); assertEquals(1, clients.size()); assertTrue(clients.contains(client.getConnectionId())); - assertTrue("Server connection failed to register", serverConnectionInitLatch.await(1, TimeUnit.SECONDS)); + assertTrue("Server connection failed to register", serverConnectionInitLatch.await(10, TimeUnit.SECONDS)); List servers = serverFactory.getOpenConnectionIds(); assertEquals(1, servers.size()); assertTrue(serverFactory.closeConnection(servers.get(0))); diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/InboundChannelAdapterParserCachingTests-context.xml b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/InboundChannelAdapterParserCachingTests-context.xml index d2e1ab943e..71da736eb6 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/InboundChannelAdapterParserCachingTests-context.xml +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/InboundChannelAdapterParserCachingTests-context.xml @@ -3,18 +3,11 @@ xmlns="http://www.springframework.org/schema/integration" xmlns:beans="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" - xmlns:p="http://www.springframework.org/schema/p" - xmlns:context="http://www.springframework.org/schema/context" xmlns:util="http://www.springframework.org/schema/util" - xmlns:tool="http://www.springframework.org/schema/tool" - xmlns:lang="http://www.springframework.org/schema/lang" xmlns:sftp="http://www.springframework.org/schema/integration/sftp" xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd http://www.springframework.org/schema/integration http://www.springframework.org/schema/integration/spring-integration.xsd - http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context.xsd http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util.xsd - http://www.springframework.org/schema/tool http://www.springframework.org/schema/tool/spring-tool.xsd - http://www.springframework.org/schema/lang http://www.springframework.org/schema/lang/spring-lang.xsd http://www.springframework.org/schema/integration/sftp http://www.springframework.org/schema/integration/sftp/spring-integration-sftp.xsd"> @@ -24,7 +17,7 @@ - + @@ -41,20 +34,20 @@ + session-factory="csf" + channel="requestChannel" + remote-directory="/foo" + local-directory="file:local-test-dir" + auto-startup="false"> + session-factory="sftpSessionFactory" + channel="requestChannel" + remote-directory="/foo" + local-directory="file:local-test-dir" + auto-startup="false"> diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/InboundChannelAdapterParserTests-context.xml b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/InboundChannelAdapterParserTests-context.xml index 31bfa0cc61..e48ab5e0ef 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/InboundChannelAdapterParserTests-context.xml +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/InboundChannelAdapterParserTests-context.xml @@ -23,7 +23,7 @@ - + @@ -44,6 +44,7 @@ local-filter="acceptAllFilter" max-fetch-size="42" delete-remote-files="${delete.remote.files}" + auto-startup="false" preserve-timestamp="true"> diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/SftpInboundAutostartup-context.xml b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/SftpInboundAutostartup-context.xml index 776323d175..d3b3301869 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/SftpInboundAutostartup-context.xml +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/config/SftpInboundAutostartup-context.xml @@ -10,7 +10,7 @@ - +