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
This commit is contained in:
Gary Russell
2015-02-10 15:42:31 -05:00
parent 49b4957270
commit 195ee70568
34 changed files with 168 additions and 162 deletions

View File

@@ -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 @@ package org.springframework.integration.aggregator;
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 @@ import org.springframework.util.Assert;
* @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 @@ public abstract class AbstractAggregatingMessageGroupProcessor implements Messag
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);
}

View File

@@ -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 java.lang.reflect.Method;
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 @@ import org.springframework.util.Assert;
* @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,8 +50,8 @@ public class MethodInvokingCorrelationStrategy implements CorrelationStrategy, B
@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);
}
}
@@ -59,4 +59,5 @@ public class MethodInvokingCorrelationStrategy implements CorrelationStrategy, B
public Object getCorrelationKey(Message<?> message) {
return processor.processMessage(message);
}
}

View File

@@ -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.apache.commons.logging.LogFactory;
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 final class FixedSubscriberChannel implements SubscribableChannel, BeanNa
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;
}
}

View File

@@ -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

View File

@@ -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 @@ import org.springframework.util.PatternMatchUtils;
* 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 @@ class IdempotentReceiverAutoProxyCreator extends AbstractAutoProxyCreator {
}
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>>();

View File

@@ -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 abstract class IntegrationNamespaceUtils {
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));
}

View File

@@ -147,7 +147,7 @@ public class UnicastingDispatcher extends AbstractDispatcher {
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);
}
}

View File

@@ -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 @@ public abstract class AbstractPollingEndpoint extends AbstractEndpoint implement
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) {

View File

@@ -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.MessageHistory;
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 @@ import org.springframework.util.Assert;
*
* @author Mark Fisher
* @author Artem Bilan
* @author Gary Russell
*/
public abstract class MessageProducerSupport extends AbstractEndpoint implements MessageProducer, TrackableComponent {
@@ -100,15 +100,12 @@ public abstract class MessageProducerSupport extends AbstractEndpoint implements
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;
}
}
}

View File

@@ -22,6 +22,7 @@ import java.util.HashMap;
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,7 +204,8 @@ class GatewayMethodInboundMessageMapper implements InboundMessageMapper<Object[]
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 +
@@ -211,8 +213,7 @@ class GatewayMethodInboundMessageMapper implements InboundMessageMapper<Object[]
}
}
else {
Object value = argumentValue.get(key);
headers.put((String) key, value);
headers.put((String) key, entry.getValue());
}
}
}

View File

@@ -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,6 +41,7 @@ import org.springframework.util.StringUtils;
*
* @author David Liu
* @author Artem Bilan
* @author Gary Russell
* since 4.1
*/
public abstract class AbstractMessageProducingHandler extends AbstractMessageHandler
@@ -48,9 +49,9 @@ public abstract class AbstractMessageProducingHandler extends AbstractMessageHan
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 abstract class AbstractMessageProducingHandler extends AbstractMessageHan
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 abstract class AbstractMessageProducingHandler extends AbstractMessageHan
@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());

View File

@@ -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.beans.factory.BeanFactoryAware;
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 class ServiceActivatingHandler extends AbstractReplyProducingMessageHandl
@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

View File

@@ -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.
@@ -23,6 +23,7 @@ import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -40,6 +41,7 @@ import org.springframework.util.StringUtils;
* @author Mark Fisher
* @author Oleg Zhurakousky
* @author Stephane Nicoll
* @author Gary Russell
* @since 2.1
*/
public abstract class AbstractHeaderMapper<T> implements RequestReplyHeaderMapper<T> {
@@ -195,8 +197,9 @@ public abstract class AbstractHeaderMapper<T> implements RequestReplyHeaderMappe
}
private void populateUserDefinedHeaders(Map<String, Object> headers, T target) {
for (String headerName : headers.keySet()) {
Object value = headers.get(headerName);
for (Entry<String, Object> entry : headers.entrySet()) {
String headerName = entry.getKey();
Object value = entry.getValue();
if (value != null && !isMessageChannel(headerName, value)) {
try {
if (!headerName.startsWith(this.standardHeaderPrefix)) {

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2011 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.
@@ -27,9 +27,10 @@ import org.springframework.util.CollectionUtils;
/**
* A Message Router that resolves the {@link MessageChannel} based on the
* {@link Message Message's} payload type.
*
*
* @author Mark Fisher
* @author Oleg Zhurakousky
* @author Gary Russell
*/
public class PayloadTypeRouter extends AbstractMappingMessageRouter {
@@ -102,7 +103,7 @@ public class PayloadTypeRouter extends AbstractMappingMessageRouter {
}
for (Class<?> iface : type.getInterfaces()) {
if (iface.getName().equals(candidate)) {
return (level % 2 == 1) ? level + 2 : level + 1;
return (level % 2 != 0) ? level + 2 : level + 1;
}
// no match at this level, continue up the hierarchy
for (Class<?> superInterface : iface.getInterfaces()) {

View File

@@ -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.
@@ -18,6 +18,7 @@ package org.springframework.integration.transformer;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -40,6 +41,7 @@ import org.springframework.messaging.MessagingException;
* @author Mark Fisher
* @author David Turanski
* @author Artem Bilan
* @author Gary Russell
*/
public class HeaderEnricher extends IntegrationObjectSupport implements Transformer, BeanNameAware, InitializingBean {
@@ -127,16 +129,16 @@ public class HeaderEnricher extends IntegrationObjectSupport implements Transfor
}
}
@SuppressWarnings("rawtypes")
private void addHeadersFromMessageProcessor(Message<?> message, Map<String, Object> headerMap) {
if (this.messageProcessor != null) {
Object result = this.messageProcessor.processMessage(message);
if (result instanceof Map) {
Map resultMap = (Map) result;
for (Object key : resultMap.keySet()) {
Map<?, ?> resultMap = (Map<?, ?>) result;
for (Entry<?, ?> entry : resultMap.entrySet()) {
Object key = entry.getKey();
if (key instanceof String) {
if (this.defaultOverwrite || headerMap.get(key) == null) {
headerMap.put((String) key, resultMap.get(key));
headerMap.put((String) key, entry.getValue());
}
}
else if (logger.isDebugEnabled()) {

View File

@@ -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.
@@ -19,9 +19,10 @@ package org.springframework.integration.transformer;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import org.springframework.integration.support.json.JsonObjectMapperProvider;
import org.springframework.integration.support.json.JsonObjectMapper;
import org.springframework.integration.support.json.JsonObjectMapperProvider;
import org.springframework.util.CollectionUtils;
import org.springframework.util.StringUtils;
@@ -50,6 +51,7 @@ import org.springframework.util.StringUtils;
*
* @author Oleg Zhurakousky
* @author Artem Bilan
* @author Gary Russell
* @since 2.0
*/
public class ObjectToMapTransformer extends AbstractPayloadTransformer<Object, Map<?,?>> {
@@ -104,9 +106,8 @@ public class ObjectToMapTransformer extends AbstractPayloadTransformer<Object, M
if (StringUtils.hasText(propertyPrefix)) {
propertyPrefix = propertyPrefix + ".";
}
for (String key : inputMap.keySet()) {
Object value = inputMap.get(key);
this.doProcessElement(propertyPrefix + key, value, resultMap);
for (Entry<String, Object> entry : inputMap.entrySet()) {
this.doProcessElement(propertyPrefix + entry.getKey(), entry.getValue(), resultMap);
}
}

View File

@@ -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.
@@ -42,6 +42,7 @@ import org.springframework.util.Assert;
* with the {@code path} as {@code key} and {@code 0} as initial {@code routingSlipIndex}.
*
* @author Artem Bilan
* @author Gary Russell
* @since 4.1
*/
public class RoutingSlipHeaderValueMessageProcessor
@@ -74,17 +75,21 @@ public class RoutingSlipHeaderValueMessageProcessor
@Override
public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
Assert.notNull(beanFactory, "BeanFactory must not be null");
this.beanFactory = beanFactory;
this.evaluationContext = IntegrationContextUtils.getEvaluationContext(beanFactory);
this.beanFactory = beanFactory;//NOSONAR (inconsistent sync)
this.evaluationContext = IntegrationContextUtils.getEvaluationContext(beanFactory);//NOSONAR (inconsistent sync)
}
@Override
public Map<List<Object>, Integer> processMessage(Message<?> message) {
if (this.routingSlip == null) {
// use a local variable to avoid the second access to volatile field on the happy path
Map<List<Object>, Integer> routingSlip = this.routingSlip;
if (routingSlip == null) {
synchronized (this) {
if (this.routingSlip == null) {
List<Object> routingSlipValues = new ArrayList<Object>(this.routingSlipPath.size());
for (Object path : this.routingSlipPath) {
routingSlip = this.routingSlip;
if (routingSlip == null) {
List<Object> routingSlipPath = this.routingSlipPath;
List<Object> routingSlipValues = new ArrayList<Object>(routingSlipPath.size());
for (Object path : routingSlipPath) {
if (path instanceof String) {
String entry = (String) path;
if (this.beanFactory.containsBean(entry)) {
@@ -108,10 +113,11 @@ public class RoutingSlipHeaderValueMessageProcessor
}
}
this.routingSlip = Collections.singletonMap(Collections.unmodifiableList(routingSlipValues), 0);
routingSlip = Collections.singletonMap(Collections.unmodifiableList(routingSlipValues), 0);
this.routingSlip = routingSlip;
}
}
}
return this.routingSlip;
return routingSlip;
}
}