Sonar Fixes

https://sonar.spring.io/component_issues?id=org.springframework.integration%3Aspring-integration%3Amaster#resolved=false|types=BUG

In `IntegrationFlowRegistration` double check locking is ok for `inputChannel`
because we're assigning an existing object, but `MessagingTemplate` constructs
a new object for which double check locking doesn't work.

In any case, for both these items, the chance of concurrent access is extremely low
and is idempotent anyway, so remove double check locking.

Several inner classes can be static.

Other minor fixes.
This commit is contained in:
Gary Russell
2016-11-18 13:34:17 -05:00
committed by Artem Bilan
parent 1bba73fc06
commit f070de0b7e
20 changed files with 63 additions and 64 deletions

View File

@@ -349,7 +349,7 @@ public abstract class AbstractAmqpOutboundEndpoint extends AbstractReplyProducin
if (!this.running) {
if (!this.lazyConnect && this.connectionFactory != null) {
try {
Connection connection = this.connectionFactory.createConnection();
Connection connection = this.connectionFactory.createConnection(); // NOSONAR (close)
if (connection != null) {
connection.close();
}

View File

@@ -229,7 +229,7 @@ public class DefaultHeaderChannelRegistry extends IntegrationObjectSupport
}
private final class MessageChannelWrapper {
private static final class MessageChannelWrapper {
private final MessageChannel channel;

View File

@@ -119,7 +119,7 @@ public class QueueChannel extends AbstractPollableChannel implements QueueChanne
long nanos = TimeUnit.MILLISECONDS.toNanos(timeout);
long deadline = System.nanoTime() + nanos;
while (this.queue.size() == 0 && nanos > 0) {
this.queueSemaphore.tryAcquire(nanos, TimeUnit.NANOSECONDS);
this.queueSemaphore.tryAcquire(nanos, TimeUnit.NANOSECONDS); // NOSONAR - ok to ignore result
nanos = deadline - System.nanoTime();
}
return this.queue.poll();

View File

@@ -84,7 +84,7 @@ public class ServiceActivatorAnnotationPostProcessor extends AbstractMethodAnnot
return serviceActivator;
}
private final class ReplyProducingMessageHandlerWrapper extends AbstractReplyProducingMessageHandler
private static final class ReplyProducingMessageHandlerWrapper extends AbstractReplyProducingMessageHandler
implements Lifecycle {
private final MessageHandler target;

View File

@@ -35,7 +35,7 @@ public class StandardIntegrationFlow implements IntegrationFlow, SmartLifecycle
private final List<SmartLifecycle> lifecycles = new LinkedList<SmartLifecycle>();
private final boolean registerComponents = true;
private final boolean registerComponents = true; // NOSONAR
private boolean running;
@@ -43,7 +43,7 @@ public class StandardIntegrationFlow implements IntegrationFlow, SmartLifecycle
this.integrationComponents = new LinkedList<Object>(integrationComponents);
}
//TODO Figure out some custom DestinationResolver when we don't register singletons
//TODO Figure out some custom DestinationResolver when we don't register singletons - remove NOSONAR above when done
/*public void setRegisterComponents(boolean registerComponents) {
this.registerComponents = registerComponents;
}*/

View File

@@ -53,7 +53,7 @@ public class IntegrationFlowRegistration {
}
void setBeanFactory(ConfigurableListableBeanFactory beanFactory) {
this.beanFactory = beanFactory;
this.beanFactory = beanFactory; // NOSONAR (synchronization)
}
void setIntegrationFlowContext(IntegrationFlowContext integrationFlowContext) {
@@ -78,28 +78,24 @@ public class IntegrationFlowRegistration {
public MessageChannel getInputChannel() {
if (this.inputChannel == null) {
synchronized (this) {
if (this.inputChannel == null) {
if (this.integrationFlow instanceof StandardIntegrationFlow) {
StandardIntegrationFlow integrationFlow = (StandardIntegrationFlow) this.integrationFlow;
Object next = integrationFlow.getIntegrationComponents().iterator().next();
if (next instanceof MessageChannel) {
this.inputChannel = (MessageChannel) next;
}
else {
throw new IllegalStateException("The 'IntegrationFlow' [" + integrationFlow + "] " +
"doesn't start with 'MessageChannel' for direct message sending.");
}
}
else {
throw new IllegalStateException("Only 'StandardIntegrationFlow' instances " +
"(e.g. extracted from 'IntegrationFlow' Lambdas) can be used " +
"for direct 'send' operation. " +
"But [" + this.integrationFlow + "] ins't one of them.\n" +
"Consider 'BeanFactory.getBean()' usage for sending messages " +
"to the required 'MessageChannel'.");
}
if (this.integrationFlow instanceof StandardIntegrationFlow) {
StandardIntegrationFlow integrationFlow = (StandardIntegrationFlow) this.integrationFlow;
Object next = integrationFlow.getIntegrationComponents().iterator().next();
if (next instanceof MessageChannel) {
this.inputChannel = (MessageChannel) next;
}
else {
throw new IllegalStateException("The 'IntegrationFlow' [" + integrationFlow + "] " +
"doesn't start with 'MessageChannel' for direct message sending.");
}
}
else {
throw new IllegalStateException("Only 'StandardIntegrationFlow' instances " +
"(e.g. extracted from 'IntegrationFlow' Lambdas) can be used " +
"for direct 'send' operation. " +
"But [" + this.integrationFlow + "] ins't one of them.\n" +
"Consider 'BeanFactory.getBean()' usage for sending messages " +
"to the required 'MessageChannel'.");
}
}
return this.inputChannel;
@@ -115,25 +111,21 @@ public class IntegrationFlowRegistration {
*/
public MessagingTemplate getMessagingTemplate() {
if (this.messagingTemplate == null) {
synchronized (this) {
if (this.messagingTemplate == null) {
this.messagingTemplate = new MessagingTemplate(getInputChannel()) {
this.messagingTemplate = new MessagingTemplate(getInputChannel()) {
@Override
public Message<?> receive() {
return receiveAndConvert(Message.class);
}
@Override
public <T> T receiveAndConvert(Class<T> targetClass) {
throw new UnsupportedOperationException("The 'receive()/receiveAndConvert()' " +
"isn't supported on the 'IntegrationFlow' input channel.");
}
};
this.messagingTemplate.setBeanFactory(this.beanFactory);
@Override
public Message<?> receive() {
return receiveAndConvert(Message.class);
}
}
@Override
public <T> T receiveAndConvert(Class<T> targetClass) {
throw new UnsupportedOperationException("The 'receive()/receiveAndConvert()' " +
"isn't supported on the 'IntegrationFlow' input channel.");
}
};
this.messagingTemplate.setBeanFactory(this.beanFactory);
}
return this.messagingTemplate;
}

View File

@@ -278,13 +278,13 @@ public class SimpleMessageStore extends AbstractMessageGroupStore
throw outOfCapacityException;
}
group = getMessageGroupFactory().create(groupId);
this.groupIdToMessageGroup.putIfAbsent(groupId, group);
this.groupIdToMessageGroup.put(groupId, group);
upperBound = new UpperBound(this.groupCapacity);
for (Message<?> message : messages) {
upperBound.tryAcquire(-1);
group.add(message);
}
this.groupToUpperBound.putIfAbsent(groupId, upperBound);
this.groupToUpperBound.put(groupId, upperBound);
}
else {
upperBound = this.groupToUpperBound.get(groupId);

View File

@@ -141,7 +141,7 @@ public class SimpleMessageConverter implements MessageConverter, BeanFactoryAwar
}
private class DefaultOutboundMessageMapper implements OutboundMessageMapper<Object> {
private static class DefaultOutboundMessageMapper implements OutboundMessageMapper<Object> {
DefaultOutboundMessageMapper() {
super();

View File

@@ -122,18 +122,22 @@ public class SimplePool<T> implements Pool<T> {
* if it was recently reduced and too many items were in use to allow the new size
* to be set.
*/
public int getPoolSize() {
@Override
public synchronized int getPoolSize() {
return this.poolSize.get();
}
@Override
public int getIdleCount() {
return this.available.size();
}
@Override
public int getActiveCount() {
return this.inUse.size();
}
@Override
public int getAllocatedCount() {
return this.allocated.size();
}
@@ -153,6 +157,7 @@ public class SimplePool<T> implements Pool<T> {
* Obtains an item from the pool; waits up to waitTime milliseconds (default infinity).
* @throws MessagingException if no items become available in time.
*/
@Override
public T getItem() {
boolean permitted = false;
try {
@@ -205,6 +210,7 @@ public class SimplePool<T> implements Pool<T> {
/**
* Returns an item to the pool.
*/
@Override
public synchronized void releaseItem(T item) {
Assert.notNull(item, "Item cannot be null");
Assert.isTrue(this.allocated.contains(item),
@@ -234,6 +240,7 @@ public class SimplePool<T> implements Pool<T> {
}
}
@Override
public synchronized void removeAllIdleItems() {
T item;
while ((item = this.available.poll()) != null) {

View File

@@ -119,7 +119,7 @@ class IntegrationGraphControllerRegistrar implements ImportBeanDefinitionRegistr
private final String[] allowedOrigins;
private IntegrationGraphCorsConfigurer(String path, String[] allowedOrigins) {
private IntegrationGraphCorsConfigurer(String path, String[] allowedOrigins) { // NOSONAR
this.path = path;
this.allowedOrigins = allowedOrigins;
}

View File

@@ -312,7 +312,7 @@ public class TcpOutboundGateway extends AbstractReplyProducingMessageHandler
* before the reply, on a different thread.
*/
logger.debug("second chance");
this.secondChanceLatch.await(2, TimeUnit.SECONDS);
this.secondChanceLatch.await(2, TimeUnit.SECONDS); // NOSONAR don't care about result
waitForMessageAfterError = false;
}
else if (this.reply.getPayload() instanceof MessagingException) {

View File

@@ -905,7 +905,7 @@ public abstract class AbstractConnectionFactory extends IntegrationObjectSupport
+ ", port=" + getPort();
}
private final class PendingIO {
private static final class PendingIO {
private final long failedAt;

View File

@@ -95,7 +95,7 @@ public class CachingClientConnectionFactory extends AbstractClientConnectionFact
* @param poolSize the new pool size.
* @see SimplePool#setPoolSize(int)
*/
public synchronized void setPoolSize(int poolSize) {
public void setPoolSize(int poolSize) {
this.pool.setPoolSize(poolSize);
}

View File

@@ -169,7 +169,7 @@ public class JdbcMessageHandler extends AbstractMessageHandler {
(PreparedStatementCallback<List<Map<String, Object>>>) ps -> {
JdbcMessageHandler.this.preparedStatementSetter.setValues(ps, message);
ps.executeUpdate();
ResultSet keys = ps.getGeneratedKeys();
ResultSet keys = ps.getGeneratedKeys(); // NOSONAR closed in JdbcUtils
if (keys != null) {
try {

View File

@@ -785,7 +785,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
}
private Object sendAndReceiveWithContainer(Message<?> requestMessage) throws JMSException {
Connection connection = this.createConnection();
Connection connection = this.createConnection(); // NOSONAR - closed in ConnectionFactoryUtils.
Session session = null;
Destination replyTo = this.replyContainer.getReplyDestination();
try {
@@ -841,7 +841,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
}
private javax.jms.Message sendAndReceiveWithoutContainer(Message<?> requestMessage) throws JMSException {
Connection connection = this.createConnection();
Connection connection = this.createConnection(); // NOSONAR - closed in ConnectionFactoryUtils.
Session session = null;
Destination replyTo = null;
try {
@@ -1329,7 +1329,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
}
}
private class GatewayReplyListenerContainer extends DefaultMessageListenerContainer {
private static class GatewayReplyListenerContainer extends DefaultMessageListenerContainer {
private volatile Destination replyDestination;
@@ -1421,7 +1421,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
}
}
private final class TimedReply {
private static final class TimedReply {
private final long timeStamp = System.currentTimeMillis();
@@ -1470,6 +1470,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
new Date(now + JmsOutboundGateway.this.receiveTimeout));
}
}
}
private class IdleContainerStopper implements Runnable {

View File

@@ -262,7 +262,7 @@ public class ImapMailReceiver extends AbstractMailReceiver {
/**
* Callback used for handling the event-driven idle response.
*/
private class SimpleMessageCountListener extends MessageCountAdapter {
private static class SimpleMessageCountListener extends MessageCountAdapter {
SimpleMessageCountListener() {
super();

View File

@@ -755,7 +755,7 @@ public class MongoDbMessageStore extends AbstractMessageGroupStore
}
@WritingConverter
private class ThrowableToBytesConverter implements Converter<Throwable, byte[]> {
private static class ThrowableToBytesConverter implements Converter<Throwable, byte[]> {
private final Converter<Object, byte[]> serializingConverter = new SerializingConverter();

View File

@@ -137,7 +137,7 @@ public class RedisOutboundGateway extends AbstractReplyProducingMessageHandler {
(RedisCallback<Object>) connection -> connection.execute(command, actualArgs));
}
private class PayloadArgumentsStrategy implements ArgumentsStrategy {
private static class PayloadArgumentsStrategy implements ArgumentsStrategy {
PayloadArgumentsStrategy() {
super();

View File

@@ -196,7 +196,7 @@ public class RFC5424SyslogParser {
return fragments;
}
protected class Reader {
protected static class Reader {
private final String line;

View File

@@ -95,8 +95,7 @@ public class OnlyOnceTrigger implements Trigger {
public void await() {
try {
this.latch.await(5000, TimeUnit.MILLISECONDS);
if (latch.getCount() != 0) {
if (!this.latch.await(5000, TimeUnit.MILLISECONDS)) {
throw new RuntimeException("test latch.await() did not count down");
}
}