Skip to content

Commit 921c797

Browse files
artembilangaryrussell
authored andcommitted
Fix new Sonar Smells
* Fix synchronization smell in the `MessagePublishingInterceptor` * Cache `Pointcut` in the `PublisherAnnotationAdvisor`; fix `@SuppressWarnings` also * Fix complexity in the `IntegrationMBeanExporter`
1 parent 9cf52b7 commit 921c797

File tree

3 files changed

+62
-78
lines changed

3 files changed

+62
-78
lines changed

spring-integration-core/src/main/java/org/springframework/integration/aop/MessagePublishingInterceptor.java

+8-12
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
104104
this.beanFactory = beanFactory;
105105
this.messagingTemplate.setBeanFactory(beanFactory);
106106
if (this.channelResolver == null) {
107-
this.channelResolver = IntegrationContextUtils.getChannelResolver(beanFactory);
107+
this.channelResolver = IntegrationContextUtils.getChannelResolver(this.beanFactory);
108108
}
109109
}
110110

@@ -123,7 +123,7 @@ public final Object invoke(final MethodInvocation invocation) throws Throwable {
123123
final StandardEvaluationContext context = ExpressionUtils.createStandardEvaluationContext(this.beanFactory);
124124
Class<?> targetClass = AopUtils.getTargetClass(invocation.getThis());
125125
final Method method = AopUtils.getMostSpecificMethod(invocation.getMethod(), targetClass);
126-
String[] argumentNames = this.resolveArgumentNames(method);
126+
String[] argumentNames = resolveArgumentNames(method);
127127
context.setVariable(PublisherMetadataSource.METHOD_NAME_VARIABLE_NAME, method.getName());
128128
if (invocation.getArguments().length > 0 && argumentNames != null) {
129129
Map<Object, Object> argumentMap = new HashMap<>();
@@ -180,16 +180,12 @@ private void publishMessage(Method method, StandardEvaluationContext context) {
180180
this.messagingTemplate.send(channel, message);
181181
}
182182
else {
183-
if (this.defaultChannelName != null) {
184-
synchronized (this) {
185-
if (this.defaultChannelName != null && this.messagingTemplate.getDefaultDestination() == null) {
186-
Assert.state(this.channelResolver != null,
187-
"ChannelResolver is required to resolve channel names.");
188-
this.messagingTemplate.setDefaultChannel(
189-
this.channelResolver.resolveDestination(this.defaultChannelName));
190-
}
191-
this.defaultChannelName = null;
192-
}
183+
String channelNameToUse = this.defaultChannelName;
184+
if (channelNameToUse != null && this.messagingTemplate.getDefaultDestination() == null) {
185+
Assert.state(this.channelResolver != null, "ChannelResolver is required to resolve channel names.");
186+
this.messagingTemplate.setDefaultChannel(
187+
this.channelResolver.resolveDestination(channelNameToUse));
188+
this.defaultChannelName = null;
193189
}
194190
this.messagingTemplate.send(message);
195191
}

spring-integration-core/src/main/java/org/springframework/integration/aop/PublisherAnnotationAdvisor.java

+16-17
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@
1919
import java.lang.annotation.Annotation;
2020
import java.lang.reflect.Method;
2121
import java.util.Arrays;
22-
import java.util.HashSet;
23-
import java.util.Set;
22+
import java.util.stream.Collectors;
2423

2524
import org.aopalliance.aop.Advice;
2625

@@ -52,23 +51,24 @@
5251
@SuppressWarnings("serial")
5352
public class PublisherAnnotationAdvisor extends AbstractPointcutAdvisor implements BeanFactoryAware {
5453

55-
private final Set<Class<? extends Annotation>> publisherAnnotationTypes;
56-
5754
private final MessagePublishingInterceptor interceptor;
5855

59-
@SuppressWarnings("unchecked")
60-
public PublisherAnnotationAdvisor(Class<? extends Annotation>... publisherAnnotationTypes) {
61-
this.publisherAnnotationTypes = new HashSet<>(Arrays.asList(publisherAnnotationTypes));
62-
PublisherMetadataSource metadataSource =
63-
new MethodAnnotationPublisherMetadataSource(this.publisherAnnotationTypes);
64-
this.interceptor = new MessagePublishingInterceptor(metadataSource);
65-
}
56+
private final Pointcut pointcut;
6657

67-
@SuppressWarnings("unchecked")
6858
public PublisherAnnotationAdvisor() {
6959
this(Publisher.class);
7060
}
7161

62+
@SuppressWarnings("varargs")
63+
@SafeVarargs
64+
public PublisherAnnotationAdvisor(Class<? extends Annotation>... publisherAnnotationTypes) {
65+
PublisherMetadataSource metadataSource =
66+
new MethodAnnotationPublisherMetadataSource(
67+
Arrays.stream(publisherAnnotationTypes).collect(Collectors.toSet()));
68+
this.interceptor = new MessagePublishingInterceptor(metadataSource);
69+
this.pointcut = buildPointcut(publisherAnnotationTypes);
70+
}
71+
7272

7373
/**
7474
* A channel bean name to be used as default for publishing.
@@ -91,12 +91,12 @@ public Advice getAdvice() {
9191

9292
@Override
9393
public Pointcut getPointcut() {
94-
return this.buildPointcut();
94+
return this.pointcut;
9595
}
9696

97-
private Pointcut buildPointcut() {
97+
private static Pointcut buildPointcut(Class<? extends Annotation>[] publisherAnnotationTypes) {
9898
ComposablePointcut result = null;
99-
for (Class<? extends Annotation> publisherAnnotationType : this.publisherAnnotationTypes) {
99+
for (Class<? extends Annotation> publisherAnnotationType : publisherAnnotationTypes) {
100100
Pointcut cpc = new MetaAnnotationMatchingPointcut(publisherAnnotationType, true);
101101
Pointcut mpc = new MetaAnnotationMatchingPointcut(null, publisherAnnotationType);
102102
if (result == null) {
@@ -187,8 +187,7 @@ private static final class MetaAnnotationMethodMatcher extends AnnotationMethodM
187187

188188

189189
@Override
190-
@SuppressWarnings("rawtypes")
191-
public boolean matches(Method method, Class targetClass) {
190+
public boolean matches(Method method, Class<?> targetClass) {
192191
if (AnnotationUtils.getAnnotation(method, this.annotationType) != null) {
193192
return true;
194193
}

spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java

+38-49
Original file line numberDiff line numberDiff line change
@@ -944,11 +944,16 @@ private MessageHandlerMetrics enhanceHandlerMonitor(MessageHandlerMetrics monito
944944
endpoint = null;
945945
}
946946
}
947-
return buildMessageHandlerMetrics(monitor, name, endpointName, source, endpoint);
947+
948+
MessageHandlerMetrics messageHandlerMetrics = buildMessageHandlerMetrics(monitor, name, source, endpoint);
949+
if (endpointName != null) {
950+
this.endpointsByMonitor.put(messageHandlerMetrics, endpointName);
951+
}
952+
return messageHandlerMetrics;
948953
}
949954

950955
private MessageHandlerMetrics buildMessageHandlerMetrics(MessageHandlerMetrics monitor,
951-
String name, String endpointName, String source, IntegrationConsumer endpoint) {
956+
String name, String source, IntegrationConsumer endpoint) {
952957

953958
MessageHandlerMetrics result = monitor;
954959
String managedType = source;
@@ -961,20 +966,7 @@ private MessageHandlerMetrics buildMessageHandlerMetrics(MessageHandlerMetrics m
961966
if (managedName != null && name.startsWith(SI_PACKAGE)) {
962967
MessageChannel inputChannel = endpoint.getInputChannel();
963968
if (inputChannel != null) {
964-
if (!this.anonymousHandlerCounters.containsKey(inputChannel)) {
965-
this.anonymousHandlerCounters.put(inputChannel, new AtomicLong());
966-
}
967-
AtomicLong count = this.anonymousHandlerCounters.get(inputChannel);
968-
long total = count.incrementAndGet();
969-
String suffix = "";
970-
/*
971-
* Short hack to makes sure object names are unique if more than one endpoint has the same input
972-
* channel
973-
*/
974-
if (total > 1) {
975-
suffix = "#" + total;
976-
}
977-
managedName = inputChannel + suffix;
969+
managedName = buildAnonymousManagedName(this.anonymousHandlerCounters, inputChannel);
978970
managedType = "anonymous";
979971
}
980972
}
@@ -993,15 +985,21 @@ private MessageHandlerMetrics buildMessageHandlerMetrics(MessageHandlerMetrics m
993985
managedType = "handler";
994986
}
995987

996-
if (endpointName != null) {
997-
this.endpointsByMonitor.put(monitor, endpointName);
998-
}
999-
1000988
result.setManagedType(managedType);
1001989
result.setManagedName(managedName);
1002990
return result;
1003991
}
1004992

993+
private String buildAnonymousManagedName(Map<Object, AtomicLong> anonymousCache, MessageChannel messageChannel) {
994+
AtomicLong count = anonymousCache.computeIfAbsent(messageChannel, (key) -> new AtomicLong());
995+
long total = count.incrementAndGet();
996+
/*
997+
* Short hack to makes sure object names are unique if more than one endpoint has the same input
998+
* channel
999+
*/
1000+
return messageChannel + (total > 1 ? "#" + total : "");
1001+
}
1002+
10051003
/**
10061004
* Wrap the monitor in a lifecycle so it exposes the start/stop operations
10071005
*/
@@ -1076,26 +1074,34 @@ private MessageSourceMetrics enhanceSourceMonitor(MessageSourceMetrics monitor)
10761074
name = getInternalComponentName(name);
10771075
source = "internal";
10781076
}
1079-
return buildMessageSourceMetricsIfAny(monitor, name, endpointName, source, endpoint);
1077+
1078+
MessageSourceMetrics messageSourceMetrics = buildMessageSourceMetricsIfAny(monitor, name, source, endpoint);
1079+
if (endpointName != null) {
1080+
this.endpointsByMonitor.put(messageSourceMetrics, endpointName);
1081+
}
1082+
return messageSourceMetrics;
10801083
}
10811084

10821085
private MessageSourceMetrics buildMessageSourceMetricsIfAny(MessageSourceMetrics monitor, String name,
1083-
String endpointName, String source, Object endpoint) {
1086+
String source, Object endpoint) {
10841087

10851088
MessageSourceMetrics result = monitor;
1086-
if (name != null && name.startsWith(SI_PACKAGE)) {
1089+
String managedType = source;
1090+
String managedName = name;
1091+
1092+
if (managedName != null && managedName.startsWith(SI_PACKAGE)) {
10871093
Object target = endpoint;
10881094
if (endpoint instanceof Advised) {
10891095
TargetSource targetSource = ((Advised) endpoint).getTargetSource();
10901096
try {
10911097
target = targetSource.getTarget();
10921098
}
10931099
catch (Exception e) {
1094-
logger.error("Could not get handler from bean = " + name);
1100+
logger.error("Could not get handler from bean = " + managedName);
10951101
}
10961102
}
10971103

1098-
Object outputChannel = null;
1104+
MessageChannel outputChannel = null;
10991105
if (target instanceof MessagingGatewaySupport) {
11001106
outputChannel = ((MessagingGatewaySupport) target).getRequestChannel();
11011107
}
@@ -1104,39 +1110,22 @@ else if (target instanceof SourcePollingChannelAdapter) {
11041110
}
11051111

11061112
if (outputChannel != null) {
1107-
if (!this.anonymousSourceCounters.containsKey(outputChannel)) {
1108-
this.anonymousSourceCounters.put(outputChannel, new AtomicLong());
1109-
}
1110-
AtomicLong count = this.anonymousSourceCounters.get(outputChannel);
1111-
long total = count.incrementAndGet();
1112-
String suffix = "";
1113-
/*
1114-
* Short hack to makes sure object names are unique if more than one endpoint has the same input
1115-
* channel
1116-
*/
1117-
if (total > 1) {
1118-
suffix = "#" + total;
1119-
}
1120-
name = outputChannel + suffix;
1121-
source = "anonymous";
1113+
managedName = buildAnonymousManagedName(this.anonymousSourceCounters, outputChannel);
1114+
managedType = "anonymous";
11221115
}
11231116
}
11241117

11251118
if (endpoint instanceof Lifecycle) {
11261119
result = wrapMessageSourceInLifecycleMetrics(result, endpoint);
11271120
}
11281121

1129-
if (name == null) {
1130-
name = result.toString();
1131-
source = "source";
1132-
}
1133-
1134-
if (endpointName != null) {
1135-
this.endpointsByMonitor.put(result, endpointName);
1122+
if (managedName == null) {
1123+
managedName = result.toString();
1124+
managedType = "source";
11361125
}
11371126

1138-
result.setManagedType(source);
1139-
result.setManagedName(name);
1127+
result.setManagedType(managedType);
1128+
result.setManagedName(managedName);
11401129
return result;
11411130
}
11421131

0 commit comments

Comments
 (0)