INT-3632: Sonar Improvements

JIRA: https://jira.spring.io/browse/INT-3632 - Use Map.entrySet() - Double check locking only works when the field(s) are volatile - Remove unnecessary instanceof tests
artembilan · Feb 10, 2015 · 195ee70 · 195ee70
1 parent 49b4957
commit 195ee70
Show file tree

Hide file tree

Showing 34 changed files with 168 additions and 162 deletions.
diff --git a/.../org/springframework/integration/aggregator/AbstractAggregatingMessageGroupProcessor.java b/.../org/springframework/integration/aggregator/AbstractAggregatingMessageGroupProcessor.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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. You may obtain a copy of the License at
@@ -16,6 +16,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 
 import org.apache.commons.logging.Log;
@@ -41,6 +42,7 @@
  * @author Alexander Peters
  * @author Mark Fisher
  * @author Dave Syer
+ * @author Gary Russell
  * @since 2.0
  */
 public abstract class AbstractAggregatingMessageGroupProcessor implements MessageGroupProcessor,
@@ -84,13 +86,13 @@ protected Map<String, Object> aggregateHeaders(MessageGroup group) {
 		Map<String, Object> aggregatedHeaders = new HashMap<String, Object>();
 		Set<String> conflictKeys = new HashSet<String>();
 		for (Message<?> message : group.getMessages()) {
-			MessageHeaders currentHeaders = message.getHeaders();
-			for (String key : currentHeaders.keySet()) {
+			for (Entry<String, Object> entry : message.getHeaders().entrySet()) {
+				String key = entry.getKey();
 				if (MessageHeaders.ID.equals(key) || MessageHeaders.TIMESTAMP.equals(key)
 						|| IntegrationMessageHeaderAccessor.SEQUENCE_SIZE.equals(key) || IntegrationMessageHeaderAccessor.SEQUENCE_NUMBER.equals(key)) {
 					continue;
 				}
-				Object value = currentHeaders.get(key);
+				Object value = entry.getValue();
 				if (!aggregatedHeaders.containsKey(key)) {
 					aggregatedHeaders.put(key, value);
 				}

diff --git a/...in/java/org/springframework/integration/aggregator/MethodInvokingCorrelationStrategy.java b/...in/java/org/springframework/integration/aggregator/MethodInvokingCorrelationStrategy.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -21,7 +21,6 @@
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
-import org.springframework.integration.handler.MessageProcessor;
 import org.springframework.integration.handler.MethodInvokingMessageProcessor;
 import org.springframework.messaging.Message;
 import org.springframework.util.Assert;
@@ -32,10 +31,11 @@
  * @author Marius Bogoevici
  * @author Dave Syer
  * @author Artem Bilan
+ * @author Gary Russell
  */
 public class MethodInvokingCorrelationStrategy implements CorrelationStrategy, BeanFactoryAware {
 
-	private final MessageProcessor<?> processor;
+	private final MethodInvokingMessageProcessor<?> processor;
 
 	public MethodInvokingCorrelationStrategy(Object object, String methodName) {
 		this.processor = new MethodInvokingMessageProcessor<Object>(object, methodName, true);
@@ -50,13 +50,14 @@ public MethodInvokingCorrelationStrategy(Object object, Method method) {
 
 	@Override
 	public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
-		if (beanFactory != null && this.processor instanceof BeanFactoryAware) {
-			((BeanFactoryAware) this.processor).setBeanFactory(beanFactory);
+		if (beanFactory != null) {
+			this.processor.setBeanFactory(beanFactory);
 		}
 	}
 
 	@Override
 	public Object getCorrelationKey(Message<?> message) {
 		return processor.processMessage(message);
 	}
+
 }
diff --git a/...on-core/src/main/java/org/springframework/integration/channel/FixedSubscriberChannel.java b/...on-core/src/main/java/org/springframework/integration/channel/FixedSubscriberChannel.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2015 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.
@@ -21,7 +21,6 @@
 import org.springframework.beans.factory.BeanNameAware;
 import org.springframework.integration.support.context.NamedComponent;
 import org.springframework.messaging.Message;
-import org.springframework.messaging.MessageDeliveryException;
 import org.springframework.messaging.MessageHandler;
 import org.springframework.messaging.MessagingException;
 import org.springframework.messaging.SubscribableChannel;
@@ -70,16 +69,14 @@ public boolean send(Message<?> message, long timeout) {
 			this.handler.handleMessage(message);
 			return true;
 		}
-		catch (Exception e) {
-			RuntimeException runtimeException = (e instanceof RuntimeException)
-					? (RuntimeException) e
-					: new MessageDeliveryException(message,
-							this.getComponentName() + " failed to deliver Message.", e);
+		catch (RuntimeException e) {
 			if (e instanceof MessagingException &&
 					((MessagingException) e).getFailedMessage() == null) {
-				runtimeException = new MessagingException(message, e);
+				throw new MessagingException(message, e);
+			}
+			else {
+				throw e;
 			}
-			throw runtimeException;
 		}
 	}
 

diff --git a/...c/main/java/org/springframework/integration/config/GlobalChannelInterceptorProcessor.java b/...c/main/java/org/springframework/integration/config/GlobalChannelInterceptorProcessor.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -70,7 +70,7 @@ final class GlobalChannelInterceptorProcessor implements BeanFactoryAware, Smart
 	@Override
 	public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
 		Assert.isInstanceOf(ListableBeanFactory.class, beanFactory);
-		this.beanFactory = (ListableBeanFactory) beanFactory;
+		this.beanFactory = (ListableBeanFactory) beanFactory;//NOSONAR (inconsistent sync)
 	}
 
 	@Override

diff --git a/.../main/java/org/springframework/integration/config/IdempotentReceiverAutoProxyCreator.java b/.../main/java/org/springframework/integration/config/IdempotentReceiverAutoProxyCreator.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2015 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,18 +37,19 @@
  * to {@link MessageHandler}s mapped by their {@code endpoint beanName}.
  *
  * @author Artem Bilan
+ * @author Gary Russell
  * @since 4.1
  */
 @SuppressWarnings("serial")
 class IdempotentReceiverAutoProxyCreator extends AbstractAutoProxyCreator {
 
-	private List<Map<String, String>> idempotentEndpointsMapping;
+	private volatile List<Map<String, String>> idempotentEndpointsMapping;
 
-	private Map<String, List<String>> idempotentEndpoints;
+	private volatile Map<String, List<String>> idempotentEndpoints; // double check locking requires volatile
 
 	public void setIdempotentEndpointsMapping(List<Map<String, String>> idempotentEndpointsMapping) {
 		Assert.notEmpty(idempotentEndpointsMapping);
-		this.idempotentEndpointsMapping = idempotentEndpointsMapping;
+		this.idempotentEndpointsMapping = idempotentEndpointsMapping;//NOSONAR (inconsistent sync)
 	}
 
 	@Override
@@ -82,7 +83,7 @@ protected Object[] getAdvicesAndAdvisorsForBean(Class<?> beanClass, String beanN
 	}
 
 	private void initIdempotentEndpointsIfNecessary() {
-		if (this.idempotentEndpoints == null) {
+		if (this.idempotentEndpoints == null) {//NOSONAR (inconsistent sync)
 			synchronized (this) {
 				if (this.idempotentEndpoints == null) {
 					this.idempotentEndpoints = new LinkedHashMap<String, List<String>>();

diff --git a/...e/src/main/java/org/springframework/integration/config/xml/IntegrationNamespaceUtils.java b/...e/src/main/java/org/springframework/integration/config/xml/IntegrationNamespaceUtils.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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. You may obtain a copy of the License at
@@ -300,8 +300,7 @@ public static BeanComponentDefinition parseInnerHandlerDefinition(Element elemen
 		String ref = element.getAttribute(REF_ATTRIBUTE);
 		if (StringUtils.hasText(ref) && innerComponentDefinition != null) {
 			parserContext.getReaderContext().error(
-				"Ambiguous definition. Inner bean " + (innerComponentDefinition == null ? innerComponentDefinition
-						: innerComponentDefinition.getBeanDefinition().getBeanClassName())
+				"Ambiguous definition. Inner bean " + (innerComponentDefinition.getBeanDefinition().getBeanClassName())
 						+ " declaration and \"ref\" " + ref + " are not allowed together on element " +
 					IntegrationNamespaceUtils.createElementDescription(element) + ".", parserContext.extractSource(element));
 		}

diff --git a/...n-core/src/main/java/org/springframework/integration/dispatcher/UnicastingDispatcher.java b/...n-core/src/main/java/org/springframework/integration/dispatcher/UnicastingDispatcher.java
@@ -147,7 +147,7 @@ private void handleExceptions(List<RuntimeException> allExceptions, Message<?> m
 			if (allExceptions != null && allExceptions.size() == 1) {
 				throw allExceptions.get(0);
 			}
-			throw new AggregateMessageDeliveryException(message,
+			throw new AggregateMessageDeliveryException(message,//NOSONAR - false positive
 					"All attempts to deliver Message to MessageHandlers failed.", allExceptions);
 		}
 	}

diff --git a/...-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java b/...-core/src/main/java/org/springframework/integration/endpoint/AbstractPollingEndpoint.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -116,10 +116,6 @@ protected void onInit() {
 				return;
 			}
 			Assert.notNull(this.trigger, "Trigger is required");
-			Executor providedExecutor = this.taskExecutor;
-			if (providedExecutor != null) {
-				this.taskExecutor = providedExecutor;
-			}
 			if (this.taskExecutor != null) {
 				if (!(this.taskExecutor instanceof ErrorHandlingTaskExecutor)) {
 					if (this.errorHandler == null) {

diff --git a/...n-core/src/main/java/org/springframework/integration/endpoint/MessageProducerSupport.java b/...n-core/src/main/java/org/springframework/integration/endpoint/MessageProducerSupport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -22,7 +22,6 @@
 import org.springframework.integration.history.TrackableComponent;
 import org.springframework.messaging.Message;
 import org.springframework.messaging.MessageChannel;
-import org.springframework.messaging.MessageDeliveryException;
 import org.springframework.messaging.MessagingException;
 import org.springframework.messaging.support.ErrorMessage;
 import org.springframework.util.Assert;
@@ -33,6 +32,7 @@
  *
  * @author Mark Fisher
  * @author Artem Bilan
+ * @author Gary Russell
  */
 public abstract class MessageProducerSupport extends AbstractEndpoint implements MessageProducer, TrackableComponent {
 
@@ -100,15 +100,12 @@ protected void sendMessage(Message<?> message) {
 		try {
 			this.messagingTemplate.send(this.outputChannel, message);
 		}
-		catch (Exception e) {
+		catch (RuntimeException e) {
 			if (this.errorChannel != null) {
 				this.messagingTemplate.send(this.errorChannel, new ErrorMessage(e));
 			}
-			else if (e instanceof RuntimeException) {
-				throw (RuntimeException) e;
-			}
-			else {
-				throw new MessageDeliveryException(message, "failed to send message", e);
+			else  {
+				throw e;
 			}
 		}
 	}

diff --git a/.../main/java/org/springframework/integration/gateway/GatewayMethodInboundMessageMapper.java b/.../main/java/org/springframework/integration/gateway/GatewayMethodInboundMessageMapper.java
@@ -22,6 +22,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -203,16 +204,16 @@ private Object evaluatePayloadExpression(String expressionString, Object argumen
 
 
 	private void copyHeaders(Map<?, ?> argumentValue, Map<String, Object> headers) {
-		for (Object key : argumentValue.keySet()) {
+		for (Entry<?, ?> entry : argumentValue.entrySet()) {
+			Object key = entry.getKey();
 			if (!(key instanceof String)) {
 				if (this.logger.isWarnEnabled()){
 					this.logger.warn("Invalid header name [" + key +
 							"], name type must be String. Skipping mapping of this header to MessageHeaders.");
 				}
 			}
 			else {
-				Object value = argumentValue.get(key);
-				headers.put((String) key, value);
+				headers.put((String) key, entry.getValue());
 			}
 		}
 	}

diff --git a/...rc/main/java/org/springframework/integration/handler/AbstractMessageProducingHandler.java b/...rc/main/java/org/springframework/integration/handler/AbstractMessageProducingHandler.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2015 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.
@@ -41,16 +41,17 @@
  *
  * @author David Liu
  * @author Artem Bilan
+ * @author Gary Russell
  * since 4.1
  */
 public abstract class AbstractMessageProducingHandler extends AbstractMessageHandler
 		implements MessageProducer {
 
 	protected final MessagingTemplate messagingTemplate = new MessagingTemplate();
 
-	private MessageChannel outputChannel;
+	private volatile MessageChannel outputChannel;
 
-	private String outputChannelName;
+	private volatile String outputChannelName;
 
 	/**
 	 * Set the timeout for sending reply Messages.
@@ -67,7 +68,7 @@ public void setOutputChannel(MessageChannel outputChannel) {
 
 	public void setOutputChannelName(String outputChannelName) {
 		Assert.hasText(outputChannelName, "'outputChannelName' must not be empty");
-		this.outputChannelName = outputChannelName;
+		this.outputChannelName = outputChannelName;//NOSONAR (inconsistent sync)
 	}
 
 	/**
@@ -82,7 +83,7 @@ public void setChannelResolver(DestinationResolver<MessageChannel> channelResolv
 	@Override
 	protected void onInit() throws Exception {
 		super.onInit();
-		Assert.state(!(this.outputChannelName != null && this.outputChannel != null),
+		Assert.state(!(this.outputChannelName != null && this.outputChannel != null),//NOSONAR (inconsistent sync)
 				"'outputChannelName' and 'outputChannel' are mutually exclusive.");
 		if (getBeanFactory() != null) {
 			this.messagingTemplate.setBeanFactory(getBeanFactory());

diff --git a/...-core/src/main/java/org/springframework/integration/handler/ServiceActivatingHandler.java b/...-core/src/main/java/org/springframework/integration/handler/ServiceActivatingHandler.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -22,7 +22,6 @@
 import org.springframework.context.Lifecycle;
 import org.springframework.integration.annotation.ServiceActivator;
 import org.springframework.messaging.Message;
-import org.springframework.messaging.MessageHandlingException;
 
 /**
  * @author Mark Fisher
@@ -87,15 +86,7 @@ public boolean isRunning() {
 
 	@Override
 	protected Object handleRequestMessage(Message<?> message) {
-		try {
-			return this.processor.processMessage(message);
-		}
-		catch (Exception e) {
-			if (e instanceof RuntimeException) {
-				throw (RuntimeException) e;
-			}
-			throw new MessageHandlingException(message, "failure occurred in '" + this + "'", e);
-		}
+		return this.processor.processMessage(message);
 	}
 
 	@Override