From 921c797646460f6eda862e2f49ebbf53eeaf4c7d Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Fri, 15 Mar 2019 13:32:18 -0400 Subject: [PATCH] Fix new Sonar Smells * Fix synchronization smell in the `MessagePublishingInterceptor` * Cache `Pointcut` in the `PublisherAnnotationAdvisor`; fix `@SuppressWarnings` also * Fix complexity in the `IntegrationMBeanExporter` --- .../aop/MessagePublishingInterceptor.java | 20 ++--- .../aop/PublisherAnnotationAdvisor.java | 33 ++++--- .../monitor/IntegrationMBeanExporter.java | 87 ++++++++----------- 3 files changed, 62 insertions(+), 78 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/aop/MessagePublishingInterceptor.java b/spring-integration-core/src/main/java/org/springframework/integration/aop/MessagePublishingInterceptor.java index eaf69b9a71..14b4d5f431 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/aop/MessagePublishingInterceptor.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/aop/MessagePublishingInterceptor.java @@ -104,7 +104,7 @@ public class MessagePublishingInterceptor implements MethodInterceptor, BeanFact this.beanFactory = beanFactory; this.messagingTemplate.setBeanFactory(beanFactory); if (this.channelResolver == null) { - this.channelResolver = IntegrationContextUtils.getChannelResolver(beanFactory); + this.channelResolver = IntegrationContextUtils.getChannelResolver(this.beanFactory); } } @@ -123,7 +123,7 @@ public class MessagePublishingInterceptor implements MethodInterceptor, BeanFact final StandardEvaluationContext context = ExpressionUtils.createStandardEvaluationContext(this.beanFactory); Class targetClass = AopUtils.getTargetClass(invocation.getThis()); final Method method = AopUtils.getMostSpecificMethod(invocation.getMethod(), targetClass); - String[] argumentNames = this.resolveArgumentNames(method); + String[] argumentNames = resolveArgumentNames(method); context.setVariable(PublisherMetadataSource.METHOD_NAME_VARIABLE_NAME, method.getName()); if (invocation.getArguments().length > 0 && argumentNames != null) { Map argumentMap = new HashMap<>(); @@ -180,16 +180,12 @@ public class MessagePublishingInterceptor implements MethodInterceptor, BeanFact this.messagingTemplate.send(channel, message); } else { - if (this.defaultChannelName != null) { - synchronized (this) { - if (this.defaultChannelName != null && this.messagingTemplate.getDefaultDestination() == null) { - Assert.state(this.channelResolver != null, - "ChannelResolver is required to resolve channel names."); - this.messagingTemplate.setDefaultChannel( - this.channelResolver.resolveDestination(this.defaultChannelName)); - } - this.defaultChannelName = null; - } + String channelNameToUse = this.defaultChannelName; + if (channelNameToUse != null && this.messagingTemplate.getDefaultDestination() == null) { + Assert.state(this.channelResolver != null, "ChannelResolver is required to resolve channel names."); + this.messagingTemplate.setDefaultChannel( + this.channelResolver.resolveDestination(channelNameToUse)); + this.defaultChannelName = null; } this.messagingTemplate.send(message); } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/aop/PublisherAnnotationAdvisor.java b/spring-integration-core/src/main/java/org/springframework/integration/aop/PublisherAnnotationAdvisor.java index 8a76726d40..569a126428 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/aop/PublisherAnnotationAdvisor.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/aop/PublisherAnnotationAdvisor.java @@ -19,8 +19,7 @@ package org.springframework.integration.aop; import java.lang.annotation.Annotation; import java.lang.reflect.Method; import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; +import java.util.stream.Collectors; import org.aopalliance.aop.Advice; @@ -52,23 +51,24 @@ import org.springframework.util.Assert; @SuppressWarnings("serial") public class PublisherAnnotationAdvisor extends AbstractPointcutAdvisor implements BeanFactoryAware { - private final Set> publisherAnnotationTypes; - private final MessagePublishingInterceptor interceptor; - @SuppressWarnings("unchecked") - public PublisherAnnotationAdvisor(Class... publisherAnnotationTypes) { - this.publisherAnnotationTypes = new HashSet<>(Arrays.asList(publisherAnnotationTypes)); - PublisherMetadataSource metadataSource = - new MethodAnnotationPublisherMetadataSource(this.publisherAnnotationTypes); - this.interceptor = new MessagePublishingInterceptor(metadataSource); - } + private final Pointcut pointcut; - @SuppressWarnings("unchecked") public PublisherAnnotationAdvisor() { this(Publisher.class); } + @SuppressWarnings("varargs") + @SafeVarargs + public PublisherAnnotationAdvisor(Class... publisherAnnotationTypes) { + PublisherMetadataSource metadataSource = + new MethodAnnotationPublisherMetadataSource( + Arrays.stream(publisherAnnotationTypes).collect(Collectors.toSet())); + this.interceptor = new MessagePublishingInterceptor(metadataSource); + this.pointcut = buildPointcut(publisherAnnotationTypes); + } + /** * A channel bean name to be used as default for publishing. @@ -91,12 +91,12 @@ public class PublisherAnnotationAdvisor extends AbstractPointcutAdvisor implemen @Override public Pointcut getPointcut() { - return this.buildPointcut(); + return this.pointcut; } - private Pointcut buildPointcut() { + private static Pointcut buildPointcut(Class[] publisherAnnotationTypes) { ComposablePointcut result = null; - for (Class publisherAnnotationType : this.publisherAnnotationTypes) { + for (Class publisherAnnotationType : publisherAnnotationTypes) { Pointcut cpc = new MetaAnnotationMatchingPointcut(publisherAnnotationType, true); Pointcut mpc = new MetaAnnotationMatchingPointcut(null, publisherAnnotationType); if (result == null) { @@ -187,8 +187,7 @@ public class PublisherAnnotationAdvisor extends AbstractPointcutAdvisor implemen @Override - @SuppressWarnings("rawtypes") - public boolean matches(Method method, Class targetClass) { + public boolean matches(Method method, Class targetClass) { if (AnnotationUtils.getAnnotation(method, this.annotationType) != null) { return true; } diff --git a/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java b/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java index e6abcb0325..b96e6cd5f6 100644 --- a/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java +++ b/spring-integration-jmx/src/main/java/org/springframework/integration/monitor/IntegrationMBeanExporter.java @@ -944,11 +944,16 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati endpoint = null; } } - return buildMessageHandlerMetrics(monitor, name, endpointName, source, endpoint); + + MessageHandlerMetrics messageHandlerMetrics = buildMessageHandlerMetrics(monitor, name, source, endpoint); + if (endpointName != null) { + this.endpointsByMonitor.put(messageHandlerMetrics, endpointName); + } + return messageHandlerMetrics; } private MessageHandlerMetrics buildMessageHandlerMetrics(MessageHandlerMetrics monitor, - String name, String endpointName, String source, IntegrationConsumer endpoint) { + String name, String source, IntegrationConsumer endpoint) { MessageHandlerMetrics result = monitor; String managedType = source; @@ -961,20 +966,7 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati if (managedName != null && name.startsWith(SI_PACKAGE)) { MessageChannel inputChannel = endpoint.getInputChannel(); if (inputChannel != null) { - if (!this.anonymousHandlerCounters.containsKey(inputChannel)) { - this.anonymousHandlerCounters.put(inputChannel, new AtomicLong()); - } - AtomicLong count = this.anonymousHandlerCounters.get(inputChannel); - long total = count.incrementAndGet(); - String suffix = ""; - /* - * Short hack to makes sure object names are unique if more than one endpoint has the same input - * channel - */ - if (total > 1) { - suffix = "#" + total; - } - managedName = inputChannel + suffix; + managedName = buildAnonymousManagedName(this.anonymousHandlerCounters, inputChannel); managedType = "anonymous"; } } @@ -993,15 +985,21 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati managedType = "handler"; } - if (endpointName != null) { - this.endpointsByMonitor.put(monitor, endpointName); - } - result.setManagedType(managedType); result.setManagedName(managedName); return result; } + private String buildAnonymousManagedName(Map anonymousCache, MessageChannel messageChannel) { + AtomicLong count = anonymousCache.computeIfAbsent(messageChannel, (key) -> new AtomicLong()); + long total = count.incrementAndGet(); + /* + * Short hack to makes sure object names are unique if more than one endpoint has the same input + * channel + */ + return messageChannel + (total > 1 ? "#" + total : ""); + } + /** * Wrap the monitor in a lifecycle so it exposes the start/stop operations */ @@ -1076,14 +1074,22 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati name = getInternalComponentName(name); source = "internal"; } - return buildMessageSourceMetricsIfAny(monitor, name, endpointName, source, endpoint); + + MessageSourceMetrics messageSourceMetrics = buildMessageSourceMetricsIfAny(monitor, name, source, endpoint); + if (endpointName != null) { + this.endpointsByMonitor.put(messageSourceMetrics, endpointName); + } + return messageSourceMetrics; } private MessageSourceMetrics buildMessageSourceMetricsIfAny(MessageSourceMetrics monitor, String name, - String endpointName, String source, Object endpoint) { + String source, Object endpoint) { MessageSourceMetrics result = monitor; - if (name != null && name.startsWith(SI_PACKAGE)) { + String managedType = source; + String managedName = name; + + if (managedName != null && managedName.startsWith(SI_PACKAGE)) { Object target = endpoint; if (endpoint instanceof Advised) { TargetSource targetSource = ((Advised) endpoint).getTargetSource(); @@ -1091,11 +1097,11 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati target = targetSource.getTarget(); } catch (Exception e) { - logger.error("Could not get handler from bean = " + name); + logger.error("Could not get handler from bean = " + managedName); } } - Object outputChannel = null; + MessageChannel outputChannel = null; if (target instanceof MessagingGatewaySupport) { outputChannel = ((MessagingGatewaySupport) target).getRequestChannel(); } @@ -1104,21 +1110,8 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati } if (outputChannel != null) { - if (!this.anonymousSourceCounters.containsKey(outputChannel)) { - this.anonymousSourceCounters.put(outputChannel, new AtomicLong()); - } - AtomicLong count = this.anonymousSourceCounters.get(outputChannel); - long total = count.incrementAndGet(); - String suffix = ""; - /* - * Short hack to makes sure object names are unique if more than one endpoint has the same input - * channel - */ - if (total > 1) { - suffix = "#" + total; - } - name = outputChannel + suffix; - source = "anonymous"; + managedName = buildAnonymousManagedName(this.anonymousSourceCounters, outputChannel); + managedType = "anonymous"; } } @@ -1126,17 +1119,13 @@ public class IntegrationMBeanExporter extends MBeanExporter implements Applicati result = wrapMessageSourceInLifecycleMetrics(result, endpoint); } - if (name == null) { - name = result.toString(); - source = "source"; + if (managedName == null) { + managedName = result.toString(); + managedType = "source"; } - if (endpointName != null) { - this.endpointsByMonitor.put(result, endpointName); - } - - result.setManagedType(source); - result.setManagedName(name); + result.setManagedType(managedType); + result.setManagedName(managedName); return result; }