From dc15910f2cc32ebcb85856f8c5d592029315a93a Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Tue, 28 Apr 2020 12:22:10 -0400 Subject: [PATCH] GH-3266: Don't override props in AnnGWProxyFB Fixes https://github.com/spring-projects/spring-integration/issues/3266 It turns out that `AnnotationGatewayProxyFactoryBean.onInit()` implementation parses a `@MessagingGateway` attributes ignoring possible properties population by setters. This way a Java DSL `GatewayProxySpec` becomes useless since all its options are overridden by default values from a synthesized `@MessagingGateway`. Also it is inconsistency when we declare an `AnnotationGatewayProxyFactoryBean` as regular bean, but then called setters are ignored * Add `protected` getters into `GatewayProxyFactoryBean` for all the properties which can be overridden by annotation attributes * Fix `AnnotationGatewayProxyFactoryBean` to consult with those getters before populating a property with value from the annotation **Cherry-pick to 5.2.x** --- .../AnnotationGatewayProxyFactoryBean.java | 74 ++++++++++++------- .../gateway/GatewayProxyFactoryBean.java | 66 ++++++++++++++++- .../dsl/gateway/GatewayDslTests.java | 15 +++- 3 files changed, 122 insertions(+), 33 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/gateway/AnnotationGatewayProxyFactoryBean.java b/spring-integration-core/src/main/java/org/springframework/integration/gateway/AnnotationGatewayProxyFactoryBean.java index 83bcc8e622..efd6533b9a 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/gateway/AnnotationGatewayProxyFactoryBean.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/gateway/AnnotationGatewayProxyFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2017-2019 the original author or authors. + * Copyright 2017-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. @@ -72,23 +72,38 @@ public class AnnotationGatewayProxyFactoryBean extends GatewayProxyFactoryBean { protected void onInit() { ConfigurableListableBeanFactory beanFactory = (ConfigurableListableBeanFactory) getBeanFactory(); - populateGatewayMethodMetadata(); + if (getGlobalMethodMetadata() == null) { + populateGatewayMethodMetadata(); + } + + String defaultRequestTimeout = resolveAttribute("defaultRequestTimeout"); + String defaultReplyTimeout = resolveAttribute("defaultReplyTimeout"); JavaUtils.INSTANCE - .acceptIfHasText(resolveAttribute("defaultRequestChannel"), this::setDefaultRequestChannelName) - .acceptIfHasText(resolveAttribute("defaultReplyChannel"), this::setDefaultReplyChannelName) - .acceptIfHasText(resolveAttribute("errorChannel"), this::setErrorChannelName) - .acceptIfHasText(resolveAttribute("defaultRequestTimeout"), + .acceptIfCondition(getDefaultRequestChannel() == null && getDefaultRequestChannelName() == null, + resolveAttribute("defaultRequestChannel"), + this::setDefaultRequestChannelName) + .acceptIfCondition(getDefaultReplyChannel() == null && getDefaultReplyChannelName() == null, + resolveAttribute("defaultReplyChannel"), + this::setDefaultReplyChannelName) + .acceptIfCondition(getErrorChannel() == null && getErrorChannelName() == null, + resolveAttribute("errorChannel"), + this::setErrorChannelName) + .acceptIfCondition(getDefaultRequestTimeout() == null && StringUtils.hasText(defaultRequestTimeout), + defaultRequestTimeout, value -> setDefaultRequestTimeout(Long.parseLong(value))) - .acceptIfHasText(resolveAttribute("defaultReplyTimeout"), + .acceptIfCondition(getDefaultReplyTimeout() == null && StringUtils.hasText(defaultReplyTimeout), + defaultReplyTimeout, value -> setDefaultReplyTimeout(Long.parseLong(value))); - String asyncExecutor = beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString("asyncExecutor")); - if (asyncExecutor == null || AnnotationConstants.NULL.equals(asyncExecutor)) { - setAsyncExecutor(null); - } - else if (StringUtils.hasText(asyncExecutor)) { - setAsyncExecutor(beanFactory.getBean(asyncExecutor, Executor.class)); + if (!isAsyncExecutorExplicitlySet()) { + String asyncExecutor = resolveAttribute("asyncExecutor"); + if (asyncExecutor == null || AnnotationConstants.NULL.equals(asyncExecutor)) { + setAsyncExecutor(null); + } + else if (StringUtils.hasText(asyncExecutor)) { + setAsyncExecutor(beanFactory.getBean(asyncExecutor, Executor.class)); + } } boolean proxyDefaultMethods = this.gatewayAttributes.getBoolean("proxyDefaultMethods"); if (proxyDefaultMethods) { @@ -117,7 +132,7 @@ public class AnnotationGatewayProxyFactoryBean extends GatewayProxyFactoryBean { "'defaultHeaders' are not allowed when a 'mapper' is provided"); JavaUtils.INSTANCE - .acceptIfHasText(mapper, + .acceptIfCondition(hasMapper && getMapper() == null, mapper, value -> setMapper(beanFactory.getBean(value, MethodArgsMessageMapper.class))); if (hasDefaultHeaders || hasDefaultPayloadExpression) { @@ -127,23 +142,26 @@ public class AnnotationGatewayProxyFactoryBean extends GatewayProxyFactoryBean { gatewayMethodMetadata.setPayloadExpression(EXPRESSION_PARSER.parseExpression(defaultPayloadExpression)); } - Map headerExpressions = Arrays.stream(defaultHeaders) - .collect(Collectors.toMap( - header -> beanFactory.resolveEmbeddedValue((String) header.get("name")), - header -> { - String headerValue = beanFactory.resolveEmbeddedValue((String) header.get("value")); - boolean hasValue = StringUtils.hasText(headerValue); + Map headerExpressions = + Arrays.stream(defaultHeaders) + .collect(Collectors.toMap( + header -> beanFactory.resolveEmbeddedValue((String) header.get("name")), + header -> { + String headerValue = + beanFactory.resolveEmbeddedValue((String) header.get("value")); + boolean hasValue = StringUtils.hasText(headerValue); - String headerExpression = - beanFactory.resolveEmbeddedValue((String) header.get("expression")); + String headerExpression = + beanFactory.resolveEmbeddedValue((String) header.get("expression")); - Assert.state(!(hasValue == StringUtils.hasText(headerExpression)), - "exactly one of 'value' or 'expression' is required on a gateway's header."); + Assert.state(!(hasValue == StringUtils.hasText(headerExpression)), + "exactly one of 'value' or 'expression' is required on a gateway's " + + "header."); - return hasValue ? - new LiteralExpression(headerValue) : - EXPRESSION_PARSER.parseExpression(headerExpression); - })); + return hasValue ? + new LiteralExpression(headerValue) : + EXPRESSION_PARSER.parseExpression(headerExpression); + })); gatewayMethodMetadata.setHeaderExpressions(headerExpressions); 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 88c846fc67..ef5991fde0 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 @@ -139,6 +139,8 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint private AsyncTaskExecutor asyncExecutor = new SimpleAsyncTaskExecutor(); + private boolean asyncExecutorExplicitlySet; + private Class asyncSubmitType; private Class asyncSubmitListenableType; @@ -189,6 +191,16 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint this.defaultRequestChannelName = defaultRequestChannelName; } + @Nullable + protected MessageChannel getDefaultRequestChannel() { + return this.defaultRequestChannel; + } + + @Nullable + protected String getDefaultRequestChannelName() { + return this.defaultRequestChannelName; + } + /** * Set the default reply channel. If no default reply channel is provided, * and no reply channel is configured with annotations, an anonymous, @@ -212,6 +224,16 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint this.defaultReplyChannelName = defaultReplyChannelName; } + @Nullable + protected MessageChannel getDefaultReplyChannel() { + return this.defaultReplyChannel; + } + + @Nullable + protected String getDefaultReplyChannelName() { + return this.defaultReplyChannelName; + } + /** * Set the error channel. If no error channel is provided, this gateway will * propagate Exceptions to the caller. To completely suppress Exceptions, provide @@ -233,6 +255,16 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint this.errorChannelName = errorChannelName; } + @Nullable + protected MessageChannel getErrorChannel() { + return this.errorChannel; + } + + @Nullable + protected String getErrorChannelName() { + return this.errorChannelName; + } + /** * Set the default timeout value for sending request messages. If not explicitly * configured with an annotation, or on a method element, this value will be used. @@ -266,6 +298,11 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint } } + @Nullable + protected Expression getDefaultRequestTimeout() { + return this.defaultRequestTimeout; + } + /** * Set the default timeout value for receiving reply messages. If not explicitly * configured with an annotation, or on a method element, this value will be used. @@ -299,6 +336,11 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint } } + @Nullable + protected Expression getDefaultReplyTimeout() { + return this.defaultReplyTimeout; + } + @Override public void setShouldTrack(boolean shouldTrack) { this.shouldTrack = shouldTrack; @@ -317,12 +359,15 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint * @param executor The executor. */ public void setAsyncExecutor(@Nullable Executor executor) { - if (executor == null && logger.isInfoEnabled()) { + if (executor == null) { logger.info("A null executor disables the async gateway; " + "methods returning Future will run on the calling thread"); } - this.asyncExecutor = (executor instanceof AsyncTaskExecutor || executor == null) ? (AsyncTaskExecutor) executor - : new TaskExecutorAdapter(executor); + this.asyncExecutor = + (executor instanceof AsyncTaskExecutor || executor == null) + ? (AsyncTaskExecutor) executor + : new TaskExecutorAdapter(executor); + this.asyncExecutorExplicitlySet = true; } public void setTypeConverter(TypeConverter typeConverter) { @@ -338,6 +383,11 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint this.globalMethodMetadata = globalMethodMetadata; } + @Nullable + protected GatewayMethodMetadata getGlobalMethodMetadata() { + return this.globalMethodMetadata; + } + @Override public void setBeanClassLoader(ClassLoader beanClassLoader) { this.beanClassLoader = beanClassLoader; @@ -352,6 +402,11 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint this.argsMapper = mapper; } + @Nullable + protected MethodArgsMessageMapper getMapper() { + return this.argsMapper; + } + /** * Indicate if {@code default} methods on the interface should be proxied as well. * If an explicit {@link Gateway} annotation is present on method it is proxied @@ -365,10 +420,15 @@ public class GatewayProxyFactoryBean extends AbstractEndpoint this.proxyDefaultMethods = proxyDefaultMethods; } + @Nullable protected AsyncTaskExecutor getAsyncExecutor() { return this.asyncExecutor; } + protected boolean isAsyncExecutorExplicitlySet() { + return this.asyncExecutorExplicitlySet; + } + /** * Return the Map of {@link Method} to {@link MessagingGatewaySupport} * generated by this factory bean. diff --git a/spring-integration-core/src/test/java/org/springframework/integration/dsl/gateway/GatewayDslTests.java b/spring-integration-core/src/test/java/org/springframework/integration/dsl/gateway/GatewayDslTests.java index e55ad8c742..4ae4919732 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/dsl/gateway/GatewayDslTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/dsl/gateway/GatewayDslTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 the original author or authors. + * Copyright 2019-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. @@ -37,6 +37,7 @@ import org.springframework.integration.MessageRejectedException; import org.springframework.integration.annotation.Gateway; import org.springframework.integration.channel.QueueChannel; import org.springframework.integration.config.EnableIntegration; +import org.springframework.integration.core.MessagingTemplate; import org.springframework.integration.dsl.IntegrationFlow; import org.springframework.integration.dsl.IntegrationFlows; import org.springframework.integration.dsl.MessageChannels; @@ -44,6 +45,7 @@ import org.springframework.integration.gateway.GatewayProxyFactoryBean; import org.springframework.integration.gateway.MessagingGatewaySupport; import org.springframework.integration.gateway.MethodArgsHolder; import org.springframework.integration.support.MessageBuilder; +import org.springframework.integration.test.util.TestUtils; import org.springframework.messaging.Message; import org.springframework.messaging.MessageChannel; import org.springframework.messaging.PollableChannel; @@ -139,6 +141,13 @@ public class GatewayDslTests { assertThat(receive).isNotNull() .extracting(Message::getPayload) .isEqualTo(defaultMethodPayload); + + MessagingGatewaySupport methodGateway = gateways.values().iterator().next(); + MessagingTemplate messagingTemplate = + TestUtils.getPropertyValue(methodGateway, "messagingTemplate", MessagingTemplate.class); + + assertThat(messagingTemplate.getReceiveTimeout()).isEqualTo(10); + assertThat(messagingTemplate.getSendTimeout()).isEqualTo(20); } @Autowired @@ -194,7 +203,9 @@ public class GatewayDslTests { return IntegrationFlows.from(MessageFunction.class, (gateway) -> gateway .header("gatewayMethod", MethodArgsHolder::getMethod) - .header("gatewayArgs", MethodArgsHolder::getArgs)) + .header("gatewayArgs", MethodArgsHolder::getArgs) + .replyTimeout(10) + .requestTimeout(20)) .bridge() .get(); }