From b8e2dfe4e1d6882753dacfac599aa9adc87e8f67 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 27 May 2020 16:10:05 -0400 Subject: [PATCH] GH-3280: NullChannel as reply for void gateways (#3292) * GH-3280: NullChannel as reply for void gateways Fixes https://github.com/spring-projects/spring-integration/issues/3280 To properly support a one-way gateway (`void` return type), it is better to ignore any possible replies from downstream instead of unexpected `no output-channel or replyChannel header available` error * Populate a `nullChannel` into a `replyChannel` header for `void` gateway methods when `replyChannel` is not set explicitly * Remove redundant `GatewayProxyFactoryBean.PARSER` in favor of similar `EXPRESSION_PARSER` in the super class * Test and document the feature * * Fix language in the `whats-new.adoc` Co-authored-by: Gary Russell * * Fix language in gateway.adoc Co-authored-by: Gary Russell Co-authored-by: Gary Russell --- .../gateway/GatewayProxyFactoryBean.java | 29 ++++++++++------- .../gateway/GatewayProxyFactoryBeanTests.java | 31 ++++++++++++++++++- src/reference/asciidoc/gateway.adoc | 3 ++ src/reference/asciidoc/whats-new.adoc | 8 +++++ 4 files changed, 59 insertions(+), 12 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayProxyFactoryBean.java b/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayProxyFactoryBean.java index ef5991fde06..120eec86309 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayProxyFactoryBean.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/gateway/GatewayProxyFactoryBean.java @@ -54,11 +54,11 @@ import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; import org.springframework.expression.common.LiteralExpression; -import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.integration.IntegrationPatternType; import org.springframework.integration.annotation.Gateway; import org.springframework.integration.annotation.GatewayHeader; +import org.springframework.integration.context.IntegrationContextUtils; import org.springframework.integration.endpoint.AbstractEndpoint; import org.springframework.integration.expression.ExpressionUtils; import org.springframework.integration.expression.ValueExpression; @@ -103,8 +103,6 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint implements TrackableComponent, FactoryBean, MethodInterceptor, BeanClassLoaderAware { - private static final SpelExpressionParser PARSER = new SpelExpressionParser(); - private final Object initializationMonitor = new Object(); private final Map gatewayMap = new HashMap<>(); @@ -718,7 +716,7 @@ private Expression extractPayloadExpressionFromAnnotationOrMetadata(@Nullable Ga * no longer work as expected; they will need to use, say, -1 instead. */ if (payloadExpression == null && StringUtils.hasText(gatewayAnnotation.payloadExpression())) { - payloadExpression = PARSER.parseExpression(gatewayAnnotation.payloadExpression()); + payloadExpression = EXPRESSION_PARSER.parseExpression(gatewayAnnotation.payloadExpression()); } } else if (methodMetadata != null && methodMetadata.getPayloadExpression() != null) { @@ -880,11 +878,24 @@ private Map headers(Method method, Map heade Map headers = null; // We don't want to eagerly resolve the error channel here Object errorChannelForVoidReturn = this.errorChannel == null ? this.errorChannelName : this.errorChannel; - if (errorChannelForVoidReturn != null && method.getReturnType().equals(void.class)) { + boolean isVoidReturnType = method.getReturnType().equals(void.class); + if (errorChannelForVoidReturn != null && isVoidReturnType) { headers = new HashMap<>(); headers.put(MessageHeaders.ERROR_CHANNEL, errorChannelForVoidReturn); } + if (isVoidReturnType && + (!headerExpressions.containsKey(MessageHeaders.REPLY_CHANNEL) || + (this.globalMethodMetadata != null && + !this.globalMethodMetadata.getHeaderExpressions() + .containsKey(MessageHeaders.REPLY_CHANNEL)))) { + + if (headers == null) { + headers = new HashMap<>(); + } + headers.put(MessageHeaders.REPLY_CHANNEL, IntegrationContextUtils.NULL_CHANNEL_BEAN_NAME); + } + if (getMessageBuilderFactory() instanceof DefaultMessageBuilderFactory) { Set headerNames = new HashSet<>(headerExpressions.keySet()); @@ -987,16 +998,12 @@ else if (StringUtils.hasText(channelName2)) { @Override // guarded by super#lifecycleLock protected void doStart() { - for (MethodInvocationGateway gateway : this.gatewayMap.values()) { - gateway.start(); - } + this.gatewayMap.values().forEach(MethodInvocationGateway::start); } @Override // guarded by super#lifecycleLock protected void doStop() { - for (MethodInvocationGateway gateway : this.gatewayMap.values()) { - gateway.stop(); - } + this.gatewayMap.values().forEach(MethodInvocationGateway::stop); } @SuppressWarnings("unchecked") diff --git a/spring-integration-core/src/test/java/org/springframework/integration/gateway/GatewayProxyFactoryBeanTests.java b/spring-integration-core/src/test/java/org/springframework/integration/gateway/GatewayProxyFactoryBeanTests.java index 3911646023a..5094de2c788 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/gateway/GatewayProxyFactoryBeanTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/gateway/GatewayProxyFactoryBeanTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.BDDMockito.willReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; @@ -48,7 +49,9 @@ import org.springframework.integration.annotation.GatewayHeader; import org.springframework.integration.channel.DirectChannel; import org.springframework.integration.channel.QueueChannel; +import org.springframework.integration.context.IntegrationContextUtils; import org.springframework.integration.endpoint.EventDrivenConsumer; +import org.springframework.integration.handler.BridgeHandler; import org.springframework.integration.support.utils.IntegrationUtils; import org.springframework.messaging.Message; import org.springframework.messaging.MessageChannel; @@ -129,6 +132,32 @@ public void testOneWay() { assertThat(message.getPayload()).isEqualTo("test"); } + @Test + public void testOneWayIgnoreReply() { + DirectChannel requestChannel = new DirectChannel(); + BeanFactory beanFactory = mock(BeanFactory.class); + QueueChannel nullChannel = new QueueChannel(); + willReturn(nullChannel) + .given(beanFactory) + .getBean(IntegrationContextUtils.NULL_CHANNEL_BEAN_NAME, MessageChannel.class); + BridgeHandler handler = new BridgeHandler(); + handler.setBeanFactory(beanFactory); + handler.afterPropertiesSet(); + requestChannel.subscribe(handler); + GatewayProxyFactoryBean proxyFactory = new GatewayProxyFactoryBean(TestService.class); + proxyFactory.setDefaultRequestChannel(requestChannel); + proxyFactory.setBeanName("testGateway"); + proxyFactory.setBeanFactory(beanFactory); + proxyFactory.afterPropertiesSet(); + TestService service = (TestService) proxyFactory.getObject(); + service.oneWay("test"); + Message message = nullChannel.receive(1000); + assertThat(message) + .isNotNull() + .extracting(Message::getPayload) + .isEqualTo("test"); + } + @Test public void testSolicitResponse() { QueueChannel replyChannel = new QueueChannel(); diff --git a/src/reference/asciidoc/gateway.adoc b/src/reference/asciidoc/gateway.adoc index ac65faffbb0..3b8af83c3de 100644 --- a/src/reference/asciidoc/gateway.adoc +++ b/src/reference/asciidoc/gateway.adoc @@ -82,6 +82,9 @@ The gateway creates a bridge from it to the temporary, anonymous reply channel t You might also want to explicitly provide a reply channel for monitoring or auditing through an interceptor (for example, <<./channel.adoc#channel-wiretap,wiretap>>). To configure a channel interceptor, you need a named channel. +NOTE: Starting with version 5.4, when gateway method return type is `void`, the framework populates a `replyChannel` header as a `nullChannel` bean reference if such a header is not provided explicitly. +This allows any possible reply from the downstream flow to be discarded, meeting the one-way gateway contract. + [[gateway-configuration-annotations]] ==== Gateway Configuration with Annotations and XML diff --git a/src/reference/asciidoc/whats-new.adoc b/src/reference/asciidoc/whats-new.adoc index fc9d295a205..9c685476170 100644 --- a/src/reference/asciidoc/whats-new.adoc +++ b/src/reference/asciidoc/whats-new.adoc @@ -12,3 +12,11 @@ If you are interested in the changes and features that were introduced in earlie If you are interested in more details, see the Issue Tracker tickets that were resolved as part of the 5.4 development process. +[[x5.4-new-components]] +=== New Components + +[[x5.4-general]] +=== General Changes + +The one-way messaging gateway (the `void` method return type) now sets a `nullChannel` explicitly into the `replyChannel` header to ignore any possible downstream replies. +See <<./gateway.adoc#gateway-default-reply-channel,Setting the Default Reply Channel>> for more information.