Skip to content

Commit e13aa28

Browse files
artembilangaryrussell
artembilan
authored and
garyrussell
committed
Fix smells for AnnGatePFB & MessagePubErrHandler
* Move `Throwable` handling in the `MessagePublishingErrorHandler` to the `handleDeliveryError()` method
1 parent 626ba7a commit e13aa28

File tree

2 files changed

+102
-105
lines changed

2 files changed

+102
-105
lines changed

‎spring-integration-core/src/main/java/org/springframework/integration/channel/MessagePublishingErrorHandler.java

Lines changed: 48 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -28,6 +28,7 @@
2828
import org.springframework.messaging.support.ErrorMessage;
2929
import org.springframework.util.Assert;
3030
import org.springframework.util.ErrorHandler;
31+
import org.springframework.util.StringUtils;
3132

3233
/**
3334
* {@link ErrorHandler} implementation that sends an {@link ErrorMessage} to a
@@ -43,12 +44,12 @@ public class MessagePublishingErrorHandler extends ErrorMessagePublisher impleme
4344

4445
private static final int DEFAULT_SEND_TIMEOUT = 1000;
4546

46-
private static final ErrorMessageStrategy DEFAULT_ERROR_MESSAGE_STRATEGY = (t, a) -> {
47-
if (t instanceof MessagingExceptionWrapper) {
48-
return new ErrorMessage(t.getCause(), ((MessagingExceptionWrapper) t).getFailedMessage());
47+
private static final ErrorMessageStrategy DEFAULT_ERROR_MESSAGE_STRATEGY = (ex, attrs) -> {
48+
if (ex instanceof MessagingExceptionWrapper) {
49+
return new ErrorMessage(ex.getCause(), ((MessagingExceptionWrapper) ex).getFailedMessage());
4950
}
5051
else {
51-
return new ErrorMessage(t);
52+
return new ErrorMessage(ex);
5253
}
5354
};
5455

@@ -87,65 +88,68 @@ public void setDefaultErrorChannelName(String defaultErrorChannelName) {
8788
}
8889

8990
@Override
90-
public final void handleError(Throwable t) {
91-
MessageChannel errorChannel = resolveErrorChannel(t);
91+
public final void handleError(Throwable ex) {
92+
MessageChannel errorChannel = resolveErrorChannel(ex);
9293
boolean sent = false;
9394
if (errorChannel != null) {
9495
try {
95-
getMessagingTemplate().send(errorChannel, getErrorMessageStrategy().buildErrorMessage(t, null));
96+
getMessagingTemplate().send(errorChannel, getErrorMessageStrategy().buildErrorMessage(ex, null));
9697
sent = true;
9798
}
98-
catch (Throwable errorDeliveryError) { //NOSONAR
99-
// message will be logged only
100-
if (this.logger.isWarnEnabled()) {
101-
this.logger.warn("Error message was not delivered.", errorDeliveryError);
102-
}
103-
if (errorDeliveryError instanceof Error) {
104-
throw ((Error) errorDeliveryError);
105-
}
99+
catch (Throwable errorDeliveryError) {
100+
handleDeliveryError(errorDeliveryError);
106101
}
107102
}
108103
if (!sent && this.logger.isErrorEnabled()) {
109-
Message<?> failedMessage = (t instanceof MessagingException) ?
110-
((MessagingException) t).getFailedMessage() : null;
111-
if (failedMessage != null) {
112-
this.logger.error("failure occurred in messaging task with message: " + failedMessage, t);
113-
}
114-
else {
115-
this.logger.error("failure occurred in messaging task", t);
116-
}
104+
Message<?> failedMessage =
105+
ex instanceof MessagingException
106+
? ((MessagingException) ex).getFailedMessage()
107+
: null;
108+
109+
this.logger.error("failure occurred in messaging task" +
110+
(failedMessage != null ? " with message: " + failedMessage : ""), ex);
111+
}
112+
}
113+
114+
private void handleDeliveryError(Throwable errorDeliveryError) {
115+
// message will be logged only
116+
if (this.logger.isWarnEnabled()) {
117+
this.logger.warn("Error message was not delivered.", errorDeliveryError);
118+
}
119+
if (errorDeliveryError instanceof Error) {
120+
throw ((Error) errorDeliveryError);
117121
}
118122
}
119123

120124
@Nullable
121125
private MessageChannel resolveErrorChannel(Throwable t) {
126+
DestinationResolver<MessageChannel> channelResolver = getChannelResolver();
122127
Throwable actualThrowable = t;
123128
if (t instanceof MessagingExceptionWrapper) {
124129
actualThrowable = t.getCause();
125130
}
126-
Message<?> failedMessage = (actualThrowable instanceof MessagingException) ?
127-
((MessagingException) actualThrowable).getFailedMessage() : null;
128-
if (getDefaultErrorChannel() == null && getChannelResolver() != null) {
129-
setChannel(getChannelResolver().resolveDestination(// NOSONAR not null
130-
IntegrationContextUtils.ERROR_CHANNEL_BEAN_NAME));
131+
Message<?> failedMessage =
132+
actualThrowable instanceof MessagingException
133+
? ((MessagingException) actualThrowable).getFailedMessage()
134+
: null;
135+
if (getDefaultErrorChannel() == null && channelResolver != null) {
136+
setChannel(channelResolver.resolveDestination(IntegrationContextUtils.ERROR_CHANNEL_BEAN_NAME));
131137
}
132138

133-
if (failedMessage == null || failedMessage.getHeaders().getErrorChannel() == null) {
134-
return getDefaultErrorChannel();
135-
}
136-
Object errorChannelHeader = failedMessage.getHeaders().getErrorChannel();
137-
if (errorChannelHeader instanceof MessageChannel) {
138-
return (MessageChannel) errorChannelHeader;
139-
}
140-
Assert.isInstanceOf(String.class, errorChannelHeader, () ->
141-
"Unsupported error channel header type. Expected MessageChannel or String, but actual type is [" +
142-
errorChannelHeader.getClass() + "]"); // NOSONAR never null here
143-
if (getChannelResolver() != null) {
144-
return getChannelResolver().resolveDestination((String) errorChannelHeader); // NOSONAR not null
145-
}
146-
else {
147-
return null;
139+
if (failedMessage != null && failedMessage.getHeaders().getErrorChannel() != null) {
140+
Object errorChannelHeader = failedMessage.getHeaders().getErrorChannel();
141+
if (errorChannelHeader instanceof MessageChannel) {
142+
return (MessageChannel) errorChannelHeader;
143+
}
144+
Assert.isInstanceOf(String.class, errorChannelHeader, () ->
145+
"Unsupported error channel header type. Expected MessageChannel or String, but actual type is [" +
146+
errorChannelHeader.getClass() + "]");
147+
if (channelResolver != null && StringUtils.hasText((String) errorChannelHeader)) {
148+
return channelResolver.resolveDestination((String) errorChannelHeader);
149+
}
148150
}
151+
152+
return getDefaultErrorChannel();
149153
}
150154

151155
}

‎spring-integration-core/src/main/java/org/springframework/integration/gateway/AnnotationGatewayProxyFactoryBean.java

Lines changed: 54 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@
1616

1717
package org.springframework.integration.gateway;
1818

19-
import java.util.HashMap;
19+
import java.util.Arrays;
2020
import java.util.Map;
2121
import java.util.concurrent.Executor;
22+
import java.util.stream.Collectors;
2223

2324
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
2425
import org.springframework.core.annotation.AnnotatedElementUtils;
@@ -28,6 +29,8 @@
2829
import org.springframework.expression.common.LiteralExpression;
2930
import org.springframework.integration.annotation.AnnotationConstants;
3031
import org.springframework.integration.annotation.MessagingGateway;
32+
import org.springframework.integration.util.JavaUtils;
33+
import org.springframework.lang.Nullable;
3134
import org.springframework.util.Assert;
3235
import org.springframework.util.ObjectUtils;
3336
import org.springframework.util.StringUtils;
@@ -48,6 +51,7 @@ public class AnnotationGatewayProxyFactoryBean extends GatewayProxyFactoryBean {
4851

4952
public AnnotationGatewayProxyFactoryBean(Class<?> serviceInterface) {
5053
super(serviceInterface);
54+
5155
AnnotationAttributes annotationAttributes =
5256
AnnotatedElementUtils.getMergedAnnotationAttributes(serviceInterface,
5357
MessagingGateway.class.getName(), false, true);
@@ -68,14 +72,37 @@ public AnnotationGatewayProxyFactoryBean(Class<?> serviceInterface) {
6872
protected void onInit() {
6973
ConfigurableListableBeanFactory beanFactory = (ConfigurableListableBeanFactory) getBeanFactory();
7074

71-
String defaultPayloadExpression =
72-
beanFactory.resolveEmbeddedValue(
73-
this.gatewayAttributes.getString("defaultPayloadExpression"));
75+
populateGatewayMethodMetadata();
76+
77+
JavaUtils.INSTANCE
78+
.acceptIfHasText(resolveAttribute("defaultRequestChannel"), this::setDefaultRequestChannelName)
79+
.acceptIfHasText(resolveAttribute("defaultReplyChannel"), this::setDefaultReplyChannelName)
80+
.acceptIfHasText(resolveAttribute("errorChannel"), this::setErrorChannelName)
81+
.acceptIfHasText(resolveAttribute("defaultRequestTimeout"),
82+
value -> setDefaultRequestTimeout(Long.parseLong(value)))
83+
.acceptIfHasText(resolveAttribute("defaultReplyTimeout"),
84+
value -> setDefaultReplyTimeout(Long.parseLong(value)));
85+
86+
String asyncExecutor = beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString("asyncExecutor"));
87+
if (asyncExecutor == null || AnnotationConstants.NULL.equals(asyncExecutor)) {
88+
setAsyncExecutor(null);
89+
}
90+
else if (StringUtils.hasText(asyncExecutor)) {
91+
setAsyncExecutor(beanFactory.getBean(asyncExecutor, Executor.class));
92+
}
93+
94+
super.onInit();
95+
}
96+
97+
private void populateGatewayMethodMetadata() {
98+
ConfigurableListableBeanFactory beanFactory = (ConfigurableListableBeanFactory) getBeanFactory();
99+
100+
String defaultPayloadExpression = resolveAttribute("defaultPayloadExpression");
74101

75102
@SuppressWarnings("unchecked")
76103
Map<String, Object>[] defaultHeaders = (Map<String, Object>[]) this.gatewayAttributes.get("defaultHeaders");
77104

78-
String mapper = beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString("mapper"));
105+
String mapper = resolveAttribute("mapper");
79106

80107
boolean hasMapper = StringUtils.hasText(mapper);
81108
boolean hasDefaultPayloadExpression = StringUtils.hasText(defaultPayloadExpression);
@@ -86,31 +113,9 @@ protected void onInit() {
86113
Assert.state(!hasMapper || !hasDefaultHeaders,
87114
"'defaultHeaders' are not allowed when a 'mapper' is provided");
88115

89-
90-
String defaultRequestChannel =
91-
beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString("defaultRequestChannel"));
92-
if (StringUtils.hasText(defaultRequestChannel)) {
93-
setDefaultRequestChannelName(defaultRequestChannel);
94-
}
95-
96-
String defaultReplyChannel =
97-
beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString("defaultReplyChannel"));
98-
if (StringUtils.hasText(defaultReplyChannel)) {
99-
setDefaultReplyChannelName(defaultReplyChannel);
100-
}
101-
102-
String errorChannel = beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString("errorChannel"));
103-
if (StringUtils.hasText(errorChannel)) {
104-
setErrorChannelName(errorChannel);
105-
}
106-
107-
String asyncExecutor = beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString("asyncExecutor"));
108-
if (asyncExecutor == null || AnnotationConstants.NULL.equals(asyncExecutor)) {
109-
setAsyncExecutor(null);
110-
}
111-
else if (StringUtils.hasText(asyncExecutor)) {
112-
setAsyncExecutor(beanFactory.getBean(asyncExecutor, Executor.class));
113-
}
116+
JavaUtils.INSTANCE
117+
.acceptIfHasText(mapper,
118+
value -> setMapper(beanFactory.getBean(value, MethodArgsMessageMapper.class)));
114119

115120
if (hasDefaultHeaders || hasDefaultPayloadExpression) {
116121
GatewayMethodMetadata gatewayMethodMetadata = new GatewayMethodMetadata();
@@ -119,46 +124,34 @@ else if (StringUtils.hasText(asyncExecutor)) {
119124
gatewayMethodMetadata.setPayloadExpression(defaultPayloadExpression);
120125
}
121126

122-
Map<String, Expression> headerExpressions = new HashMap<>();
123-
for (Map<String, Object> header : defaultHeaders) {
124-
String headerValue = beanFactory.resolveEmbeddedValue((String) header.get("value"));
125-
boolean hasValue = StringUtils.hasText(headerValue);
126-
127-
String headerExpression = beanFactory.resolveEmbeddedValue((String) header.get("expression"));
127+
Map<String, Expression> headerExpressions = Arrays.stream(defaultHeaders)
128+
.collect(Collectors.toMap(
129+
header -> beanFactory.resolveEmbeddedValue((String) header.get("name")),
130+
header -> {
131+
String headerValue = beanFactory.resolveEmbeddedValue((String) header.get("value"));
132+
boolean hasValue = StringUtils.hasText(headerValue);
128133

129-
Assert.state(!(hasValue == StringUtils.hasText(headerExpression)),
130-
"exactly one of 'value' or 'expression' is required on a gateway's header.");
134+
String headerExpression =
135+
beanFactory.resolveEmbeddedValue((String) header.get("expression"));
131136

132-
Expression expression = hasValue ?
133-
new LiteralExpression(headerValue) :
134-
EXPRESSION_PARSER.parseExpression(headerExpression);
137+
Assert.state(!(hasValue == StringUtils.hasText(headerExpression)),
138+
"exactly one of 'value' or 'expression' is required on a gateway's header.");
135139

136-
String headerName = beanFactory.resolveEmbeddedValue((String) header.get("name"));
137-
headerExpressions.put(headerName, expression);
138-
}
140+
return hasValue ?
141+
new LiteralExpression(headerValue) :
142+
EXPRESSION_PARSER.parseExpression(headerExpression);
143+
}));
139144

140145
gatewayMethodMetadata.setHeaderExpressions(headerExpressions);
141146

142147
setGlobalMethodMetadata(gatewayMethodMetadata);
143148
}
149+
}
144150

145-
if (StringUtils.hasText(mapper)) {
146-
setMapper(beanFactory.getBean(mapper, MethodArgsMessageMapper.class));
147-
}
148-
149-
String defaultRequestTimeout =
150-
beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString("defaultRequestTimeout"));
151-
if (StringUtils.hasText(defaultRequestTimeout)) {
152-
setDefaultRequestTimeout(Long.parseLong(defaultRequestTimeout));
153-
}
154-
155-
String defaultReplyTimeout =
156-
beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString("defaultReplyTimeout"));
157-
if (StringUtils.hasText(defaultReplyTimeout)) {
158-
setDefaultReplyTimeout(Long.parseLong(defaultReplyTimeout));
159-
}
160-
161-
super.onInit();
151+
@Nullable
152+
private String resolveAttribute(String attributeName) {
153+
ConfigurableListableBeanFactory beanFactory = (ConfigurableListableBeanFactory) getBeanFactory();
154+
return beanFactory.resolveEmbeddedValue(this.gatewayAttributes.getString(attributeName));
162155
}
163156

164157
}

0 commit comments

Comments
 (0)