From f070de0b7ef3df2d7e0e666c20ea661401d2b213 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 18 Nov 2016 13:34:17 -0500 Subject: [PATCH] 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. --- .../AbstractAmqpOutboundEndpoint.java | 2 +- .../channel/DefaultHeaderChannelRegistry.java | 2 +- .../integration/channel/QueueChannel.java | 2 +- ...rviceActivatorAnnotationPostProcessor.java | 2 +- .../dsl/StandardIntegrationFlow.java | 4 +- .../context/IntegrationFlowRegistration.java | 70 ++++++++----------- .../integration/store/SimpleMessageStore.java | 4 +- .../converter/SimpleMessageConverter.java | 2 +- .../integration/util/SimplePool.java | 9 ++- .../IntegrationGraphControllerRegistrar.java | 2 +- .../ip/tcp/TcpOutboundGateway.java | 2 +- .../connection/AbstractConnectionFactory.java | 2 +- .../CachingClientConnectionFactory.java | 2 +- .../integration/jdbc/JdbcMessageHandler.java | 2 +- .../integration/jms/JmsOutboundGateway.java | 9 +-- .../integration/mail/ImapMailReceiver.java | 2 +- .../mongodb/store/MongoDbMessageStore.java | 2 +- .../redis/outbound/RedisOutboundGateway.java | 2 +- .../syslog/RFC5424SyslogParser.java | 2 +- .../test/util/OnlyOnceTrigger.java | 3 +- 20 files changed, 63 insertions(+), 64 deletions(-) diff --git a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AbstractAmqpOutboundEndpoint.java b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AbstractAmqpOutboundEndpoint.java index 0153a021c2..d7c095d935 100644 --- a/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AbstractAmqpOutboundEndpoint.java +++ b/spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AbstractAmqpOutboundEndpoint.java @@ -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(); } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/channel/DefaultHeaderChannelRegistry.java b/spring-integration-core/src/main/java/org/springframework/integration/channel/DefaultHeaderChannelRegistry.java index 258ac63407..f5bc908543 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/channel/DefaultHeaderChannelRegistry.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/channel/DefaultHeaderChannelRegistry.java @@ -229,7 +229,7 @@ public class DefaultHeaderChannelRegistry extends IntegrationObjectSupport } - private final class MessageChannelWrapper { + private static final class MessageChannelWrapper { private final MessageChannel channel; diff --git a/spring-integration-core/src/main/java/org/springframework/integration/channel/QueueChannel.java b/spring-integration-core/src/main/java/org/springframework/integration/channel/QueueChannel.java index 55c58f4ef1..688a600241 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/channel/QueueChannel.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/channel/QueueChannel.java @@ -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(); diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java index d9c10716a4..aa2fe1480a 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/annotation/ServiceActivatorAnnotationPostProcessor.java @@ -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; diff --git a/spring-integration-core/src/main/java/org/springframework/integration/dsl/StandardIntegrationFlow.java b/spring-integration-core/src/main/java/org/springframework/integration/dsl/StandardIntegrationFlow.java index 1ea637c53c..9ea711f730 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/dsl/StandardIntegrationFlow.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/dsl/StandardIntegrationFlow.java @@ -35,7 +35,7 @@ public class StandardIntegrationFlow implements IntegrationFlow, SmartLifecycle private final List lifecycles = new LinkedList(); - 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(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; }*/ diff --git a/spring-integration-core/src/main/java/org/springframework/integration/dsl/context/IntegrationFlowRegistration.java b/spring-integration-core/src/main/java/org/springframework/integration/dsl/context/IntegrationFlowRegistration.java index 4a4406a568..9be6e7daf6 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/dsl/context/IntegrationFlowRegistration.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/dsl/context/IntegrationFlowRegistration.java @@ -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 receiveAndConvert(Class 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 receiveAndConvert(Class targetClass) { + throw new UnsupportedOperationException("The 'receive()/receiveAndConvert()' " + + "isn't supported on the 'IntegrationFlow' input channel."); + } + + }; + this.messagingTemplate.setBeanFactory(this.beanFactory); } return this.messagingTemplate; } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/store/SimpleMessageStore.java b/spring-integration-core/src/main/java/org/springframework/integration/store/SimpleMessageStore.java index 147bb5c248..ee84507709 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/store/SimpleMessageStore.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/store/SimpleMessageStore.java @@ -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); diff --git a/spring-integration-core/src/main/java/org/springframework/integration/support/converter/SimpleMessageConverter.java b/spring-integration-core/src/main/java/org/springframework/integration/support/converter/SimpleMessageConverter.java index 7fd9f90477..8a2d958b7f 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/support/converter/SimpleMessageConverter.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/support/converter/SimpleMessageConverter.java @@ -141,7 +141,7 @@ public class SimpleMessageConverter implements MessageConverter, BeanFactoryAwar } - private class DefaultOutboundMessageMapper implements OutboundMessageMapper { + private static class DefaultOutboundMessageMapper implements OutboundMessageMapper { DefaultOutboundMessageMapper() { super(); diff --git a/spring-integration-core/src/main/java/org/springframework/integration/util/SimplePool.java b/spring-integration-core/src/main/java/org/springframework/integration/util/SimplePool.java index be6c143e4e..741e485a29 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/util/SimplePool.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/util/SimplePool.java @@ -122,18 +122,22 @@ public class SimplePool implements Pool { * 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 implements Pool { * 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 implements Pool { /** * 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 implements Pool { } } + @Override public synchronized void removeAllIdleItems() { T item; while ((item = this.available.poll()) != null) { diff --git a/spring-integration-http/src/main/java/org/springframework/integration/http/config/IntegrationGraphControllerRegistrar.java b/spring-integration-http/src/main/java/org/springframework/integration/http/config/IntegrationGraphControllerRegistrar.java index 24899c9c27..fb81b06e17 100644 --- a/spring-integration-http/src/main/java/org/springframework/integration/http/config/IntegrationGraphControllerRegistrar.java +++ b/spring-integration-http/src/main/java/org/springframework/integration/http/config/IntegrationGraphControllerRegistrar.java @@ -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; } diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/TcpOutboundGateway.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/TcpOutboundGateway.java index 187508d1ee..cf44d5f7d5 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/TcpOutboundGateway.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/TcpOutboundGateway.java @@ -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) { diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java index acb3156a0c..f6dca98786 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/AbstractConnectionFactory.java @@ -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; diff --git a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/CachingClientConnectionFactory.java b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/CachingClientConnectionFactory.java index 949cf6d3e6..df02e36b29 100644 --- a/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/CachingClientConnectionFactory.java +++ b/spring-integration-ip/src/main/java/org/springframework/integration/ip/tcp/connection/CachingClientConnectionFactory.java @@ -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); } diff --git a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/JdbcMessageHandler.java b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/JdbcMessageHandler.java index 441a076230..b066df73a8 100644 --- a/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/JdbcMessageHandler.java +++ b/spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/JdbcMessageHandler.java @@ -169,7 +169,7 @@ public class JdbcMessageHandler extends AbstractMessageHandler { (PreparedStatementCallback>>) 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 { diff --git a/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsOutboundGateway.java b/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsOutboundGateway.java index 6c5015e754..2fc6e58a7d 100644 --- a/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsOutboundGateway.java +++ b/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsOutboundGateway.java @@ -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 { diff --git a/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java b/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java index e09de65bfa..4c0b8b8f4b 100755 --- a/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java +++ b/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java @@ -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(); diff --git a/spring-integration-mongodb/src/main/java/org/springframework/integration/mongodb/store/MongoDbMessageStore.java b/spring-integration-mongodb/src/main/java/org/springframework/integration/mongodb/store/MongoDbMessageStore.java index 09b9f57941..03f21d382d 100644 --- a/spring-integration-mongodb/src/main/java/org/springframework/integration/mongodb/store/MongoDbMessageStore.java +++ b/spring-integration-mongodb/src/main/java/org/springframework/integration/mongodb/store/MongoDbMessageStore.java @@ -755,7 +755,7 @@ public class MongoDbMessageStore extends AbstractMessageGroupStore } @WritingConverter - private class ThrowableToBytesConverter implements Converter { + private static class ThrowableToBytesConverter implements Converter { private final Converter serializingConverter = new SerializingConverter(); diff --git a/spring-integration-redis/src/main/java/org/springframework/integration/redis/outbound/RedisOutboundGateway.java b/spring-integration-redis/src/main/java/org/springframework/integration/redis/outbound/RedisOutboundGateway.java index 8e9f58004c..2bab621229 100644 --- a/spring-integration-redis/src/main/java/org/springframework/integration/redis/outbound/RedisOutboundGateway.java +++ b/spring-integration-redis/src/main/java/org/springframework/integration/redis/outbound/RedisOutboundGateway.java @@ -137,7 +137,7 @@ public class RedisOutboundGateway extends AbstractReplyProducingMessageHandler { (RedisCallback) connection -> connection.execute(command, actualArgs)); } - private class PayloadArgumentsStrategy implements ArgumentsStrategy { + private static class PayloadArgumentsStrategy implements ArgumentsStrategy { PayloadArgumentsStrategy() { super(); diff --git a/spring-integration-syslog/src/main/java/org/springframework/integration/syslog/RFC5424SyslogParser.java b/spring-integration-syslog/src/main/java/org/springframework/integration/syslog/RFC5424SyslogParser.java index dbe95767d2..cbedf034be 100644 --- a/spring-integration-syslog/src/main/java/org/springframework/integration/syslog/RFC5424SyslogParser.java +++ b/spring-integration-syslog/src/main/java/org/springframework/integration/syslog/RFC5424SyslogParser.java @@ -196,7 +196,7 @@ public class RFC5424SyslogParser { return fragments; } - protected class Reader { + protected static class Reader { private final String line; diff --git a/spring-integration-test/src/main/java/org/springframework/integration/test/util/OnlyOnceTrigger.java b/spring-integration-test/src/main/java/org/springframework/integration/test/util/OnlyOnceTrigger.java index b21da521e8..8dd1b691b9 100644 --- a/spring-integration-test/src/main/java/org/springframework/integration/test/util/OnlyOnceTrigger.java +++ b/spring-integration-test/src/main/java/org/springframework/integration/test/util/OnlyOnceTrigger.java @@ -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"); } }