Fix new and some old Sonar smells

This commit is contained in:
Artem Bilan
2020-07-14 10:48:23 -04:00
parent df1f260a59
commit a19e37237b
5 changed files with 189 additions and 179 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -44,6 +44,7 @@ import javax.jms.Topic;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.context.Lifecycle;
import org.springframework.core.convert.ConversionService;
import org.springframework.expression.Expression;
@@ -53,6 +54,7 @@ import org.springframework.integration.handler.AbstractReplyProducingMessageHand
import org.springframework.integration.handler.ExpressionEvaluatingMessageProcessor;
import org.springframework.integration.jms.util.JmsAdapterUtils;
import org.springframework.integration.support.AbstractIntegrationMessageBuilder;
import org.springframework.integration.util.JavaUtils;
import org.springframework.jms.connection.ConnectionFactoryUtils;
import org.springframework.jms.listener.DefaultMessageListenerContainer;
import org.springframework.jms.support.JmsUtils;
@@ -520,61 +522,67 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
"Exactly one of 'requestDestination', 'requestDestinationName', " +
"or 'requestDestinationExpression' is required.");
ConversionService conversionService = getConversionService();
BeanFactory beanFactory = getBeanFactory();
if (this.requestDestinationExpressionProcessor != null) {
this.requestDestinationExpressionProcessor.setBeanFactory(getBeanFactory());
this.requestDestinationExpressionProcessor.setBeanFactory(beanFactory);
if (conversionService != null) {
this.requestDestinationExpressionProcessor.setConversionService(conversionService);
}
}
if (this.replyDestinationExpressionProcessor != null) {
this.replyDestinationExpressionProcessor.setBeanFactory(getBeanFactory());
this.replyDestinationExpressionProcessor.setBeanFactory(beanFactory);
if (conversionService != null) {
this.replyDestinationExpressionProcessor.setConversionService(conversionService);
}
}
/*
* This is needed because there is no way to detect 2 or more gateways using the same reply queue
* with no correlation key.
*/
boolean hasAReplyDest = this.replyDestination != null || this.replyDestinationName != null
|| this.replyDestinationExpressionProcessor != null;
if (this.useReplyContainer && (this.correlationKey == null && hasAReplyDest)) {
logger.warn("The gateway cannot use a reply listener container with a specified " +
"destination(Name/Expression) " +
"without a 'correlation-key'; " +
"a container will NOT be used; " +
"to avoid this problem, set the 'correlation-key' attribute; " +
"some consumers, including the Spring Integration <jms:inbound-gateway/>, " +
"support the use of the value 'JMSCorrelationID' " +
"for this purpose. Alternatively, do not specify a reply destination " +
"and a temporary queue will be used for replies.");
this.useReplyContainer = false;
}
if (this.useReplyContainer) {
Assert.state(!"JMSCorrelationID*".equals(this.correlationKey),
"Using an existing 'JMSCorrelationID' mapped from the 'requestMessage' ('JMSCorrelationID*') " +
"can't be used when using a 'reply-container'");
GatewayReplyListenerContainer container = new GatewayReplyListenerContainer();
setContainerProperties(container);
container.afterPropertiesSet();
this.replyContainer = container;
if (isAsync() && this.correlationKey == null) {
logger.warn("'async=true' requires a correlationKey; ignored");
setAsync(false);
}
}
else {
if (isAsync()) {
logger.warn("'async=true' is ignored when a reply container is not being used");
setAsync(false);
}
}
initializeReplyContainer();
this.initialized = true;
}
}
private void initializeReplyContainer() {
/*
* This is needed because there is no way to detect 2 or more gateways using the same reply queue
* with no correlation key.
*/
boolean hasAReplyDest = this.replyDestination != null || this.replyDestinationName != null
|| this.replyDestinationExpressionProcessor != null;
if (this.useReplyContainer && (this.correlationKey == null && hasAReplyDest)) {
logger.warn("The gateway cannot use a reply listener container with a specified " +
"destination(Name/Expression) " +
"without a 'correlation-key'; " +
"a container will NOT be used; " +
"to avoid this problem, set the 'correlation-key' attribute; " +
"some consumers, including the Spring Integration <jms:inbound-gateway/>, " +
"support the use of the value 'JMSCorrelationID' " +
"for this purpose. Alternatively, do not specify a reply destination " +
"and a temporary queue will be used for replies.");
this.useReplyContainer = false;
}
if (this.useReplyContainer) {
Assert.state(!"JMSCorrelationID*".equals(this.correlationKey),
"Using an existing 'JMSCorrelationID' mapped from the 'requestMessage' ('JMSCorrelationID*') " +
"can't be used when using a 'reply-container'");
GatewayReplyListenerContainer container = new GatewayReplyListenerContainer();
setContainerProperties(container);
container.afterPropertiesSet();
this.replyContainer = container;
if (isAsync() && this.correlationKey == null) {
logger.warn("'async=true' requires a correlationKey; ignored");
setAsync(false);
}
}
else {
if (isAsync()) {
logger.warn("'async=true' is ignored when a reply container is not being used");
setAsync(false);
}
}
}
private void setContainerProperties(GatewayReplyListenerContainer container) {
container.setConnectionFactory(this.connectionFactory);
if (this.replyDestination != null) {
container.setDestination(this.replyDestination);
}
@@ -594,61 +602,57 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
container.setMessageSelector(messageSelector);
}
container.setMessageListener(this);
applyReplyContainerProperties(container);
}
private void applyReplyContainerProperties(GatewayReplyListenerContainer container) {
if (this.replyContainerProperties != null) {
if (this.replyContainerProperties.isSessionTransacted() != null) {
container.setSessionTransacted(this.replyContainerProperties.isSessionTransacted());
}
if (this.replyContainerProperties.getCacheLevel() != null) {
container.setCacheLevel(this.replyContainerProperties.getCacheLevel());
}
if (this.replyContainerProperties.getConcurrentConsumers() != null) {
container.setConcurrentConsumers(this.replyContainerProperties.getConcurrentConsumers());
}
if (this.replyContainerProperties.getIdleConsumerLimit() != null) {
container.setIdleConsumerLimit(this.replyContainerProperties.getIdleConsumerLimit());
}
if (this.replyContainerProperties.getIdleTaskExecutionLimit() != null) {
container.setIdleTaskExecutionLimit(this.replyContainerProperties.getIdleTaskExecutionLimit());
}
if (this.replyContainerProperties.getMaxConcurrentConsumers() != null) {
container.setMaxConcurrentConsumers(this.replyContainerProperties.getMaxConcurrentConsumers());
}
if (this.replyContainerProperties.getMaxMessagesPerTask() != null) {
container.setMaxMessagesPerTask(this.replyContainerProperties.getMaxMessagesPerTask());
}
if (this.replyContainerProperties.getReceiveTimeout() != null) {
container.setReceiveTimeout(this.replyContainerProperties.getReceiveTimeout());
}
if (this.replyContainerProperties.getRecoveryInterval() != null) {
container.setRecoveryInterval(this.replyContainerProperties.getRecoveryInterval());
}
if (StringUtils.hasText(this.replyContainerProperties.getSessionAcknowledgeModeName())) {
Integer acknowledgeMode = JmsAdapterUtils.parseAcknowledgeMode(
this.replyContainerProperties.getSessionAcknowledgeModeName());
if (acknowledgeMode != null) {
if (JmsAdapterUtils.SESSION_TRANSACTED == acknowledgeMode) {
container.setSessionTransacted(true);
}
else {
container.setSessionAcknowledgeMode(acknowledgeMode);
}
}
}
else if (this.replyContainerProperties.getSessionAcknowledgeMode() != null) {
Integer sessionAcknowledgeMode = this.replyContainerProperties.getSessionAcknowledgeMode();
if (Session.SESSION_TRANSACTED == sessionAcknowledgeMode) {
container.setSessionTransacted(true);
}
else {
container.setSessionAcknowledgeMode(sessionAcknowledgeMode);
}
JavaUtils.INSTANCE
.acceptIfNotNull(this.replyContainerProperties.isSessionTransacted(),
container::setSessionTransacted)
.acceptIfNotNull(this.replyContainerProperties.getCacheLevel(),
container::setCacheLevel)
.acceptIfNotNull(this.replyContainerProperties.getConcurrentConsumers(),
container::setConcurrentConsumers)
.acceptIfNotNull(this.replyContainerProperties.getIdleConsumerLimit(),
container::setIdleConsumerLimit)
.acceptIfNotNull(this.replyContainerProperties.getIdleTaskExecutionLimit(),
container::setIdleTaskExecutionLimit)
.acceptIfNotNull(this.replyContainerProperties.getMaxConcurrentConsumers(),
container::setMaxConcurrentConsumers)
.acceptIfNotNull(this.replyContainerProperties.getMaxMessagesPerTask(),
container::setMaxMessagesPerTask)
.acceptIfNotNull(this.replyContainerProperties.getReceiveTimeout(),
container::setReceiveTimeout)
.acceptIfNotNull(this.replyContainerProperties.getRecoveryInterval(),
container::setRecoveryInterval)
.acceptIfHasText(this.replyContainerProperties.getSessionAcknowledgeModeName(),
acknowledgeModeName -> {
Integer acknowledgeMode = JmsAdapterUtils.parseAcknowledgeMode(
this.replyContainerProperties.getSessionAcknowledgeModeName());
if (acknowledgeMode != null) {
if (JmsAdapterUtils.SESSION_TRANSACTED == acknowledgeMode) {
container.setSessionTransacted(true);
}
else {
container.setSessionAcknowledgeMode(acknowledgeMode);
}
}
})
.acceptIfNotNull(this.replyContainerProperties.getSessionAcknowledgeMode(),
acknowledgeMode -> {
if (Session.SESSION_TRANSACTED == acknowledgeMode) {
container.setSessionTransacted(true);
}
else {
container.setSessionAcknowledgeMode(acknowledgeMode);
}
})
.acceptIfNotNull(this.replyContainerProperties.getTaskExecutor(),
container::setTaskExecutor);
}
if (this.replyContainerProperties.getTaskExecutor() != null) {
container.setTaskExecutor(this.replyContainerProperties.getTaskExecutor());
}
else {
if (this.replyContainerProperties.getTaskExecutor() == null) {
// set the beanName so the default TE threads get a meaningful name
String containerBeanName = this.getComponentName();
containerBeanName = ((!StringUtils.hasText(containerBeanName)
@@ -985,8 +989,8 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
* If the replyTo is not temporary, and the connection is lost while waiting for a reply, reconnect for
* up to receiveTimeout.
*/
private javax.jms.Message retryableReceiveReply(Session session, Destination replyTo, String messageSelector)
throws JMSException {
private javax.jms.Message retryableReceiveReply(Session session, Destination replyTo, // NOSONAR
String messageSelector) throws JMSException {
Connection consumerConnection = null; //NOSONAR
Session consumerSession = session;
@@ -996,17 +1000,15 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
long replyTimeout = isTemporaryReplyTo
? Long.MIN_VALUE
: this.receiveTimeout < 0
? Long.MAX_VALUE
: System.currentTimeMillis() + this.receiveTimeout;
? Long.MAX_VALUE
: System.currentTimeMillis() + this.receiveTimeout;
try {
do {
try {
messageConsumer = consumerSession.createConsumer(replyTo, messageSelector);
javax.jms.Message reply = receiveReplyMessage(messageConsumer);
if (reply == null) {
if (replyTimeout > System.currentTimeMillis()) {
throw new JMSException("Consumer closed before timeout");
}
if (reply == null && replyTimeout > System.currentTimeMillis()) {
throw new JMSException("Consumer closed before timeout");
}
return reply;
}
@@ -1027,7 +1029,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
logger.debug("Could not reconnect, retrying: " + ee.getMessage());
}
try {
Thread.sleep(1000);
Thread.sleep(1000); // NOSONAR
}
catch (@SuppressWarnings("unused") InterruptedException e1) {
Thread.currentThread().interrupt();
@@ -1377,10 +1379,10 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
}
else {
int n = 0;
while (this.replyDestination == null && n++ < 100) {
while (this.replyDestination == null && n++ < 100) { // NOSONAR
logger.debug("Waiting for container to create destination");
try {
Thread.sleep(100);
Thread.sleep(100); // NOSONAR
}
catch (InterruptedException e) {
Thread.currentThread().interrupt();
@@ -1488,16 +1490,15 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler imp
synchronized (JmsOutboundGateway.this.lifeCycleMonitor) {
if (System.currentTimeMillis() - JmsOutboundGateway.this.lastSend >
JmsOutboundGateway.this.idleReplyContainerTimeout
&& JmsOutboundGateway.this.replies.size() == 0) {
&& JmsOutboundGateway.this.replies.size() == 0 &&
JmsOutboundGateway.this.replyContainer.isRunning()) {
if (JmsOutboundGateway.this.replyContainer.isRunning()) {
if (logger.isDebugEnabled()) {
logger.debug(getComponentName() + ": Stopping idle reply container.");
}
JmsOutboundGateway.this.replyContainer.stop();
JmsOutboundGateway.this.idleTask.cancel(false);
JmsOutboundGateway.this.idleTask = null;
if (logger.isDebugEnabled()) {
logger.debug(getComponentName() + ": Stopping idle reply container.");
}
JmsOutboundGateway.this.replyContainer.stop();
JmsOutboundGateway.this.idleTask.cancel(false);
JmsOutboundGateway.this.idleTask = null;
}
}
}

View File

@@ -49,7 +49,7 @@ public class KafkaTemplateSpec<K, V>
}
@Override
public KafkaTemplateSpec<K, V> id(String id) {
public KafkaTemplateSpec<K, V> id(String id) { // NOSONAR - visibility
return super.id(id);
}

View File

@@ -32,7 +32,6 @@ import org.springframework.integration.context.OrderlyShutdownCapable;
import org.springframework.integration.gateway.MessagingGatewaySupport;
import org.springframework.integration.kafka.support.RawRecordHeaderErrorMessageStrategy;
import org.springframework.integration.support.AbstractIntegrationMessageBuilder;
import org.springframework.integration.support.ErrorMessageStrategy;
import org.springframework.integration.support.ErrorMessageUtils;
import org.springframework.integration.support.MessageBuilder;
import org.springframework.kafka.core.KafkaTemplate;
@@ -43,7 +42,6 @@ import org.springframework.kafka.listener.adapter.RecordMessagingMessageListener
import org.springframework.kafka.listener.adapter.RetryingMessageListenerAdapter;
import org.springframework.kafka.support.Acknowledgment;
import org.springframework.kafka.support.KafkaHeaders;
import org.springframework.kafka.support.converter.BatchMessageConverter;
import org.springframework.kafka.support.converter.ConversionException;
import org.springframework.kafka.support.converter.KafkaMessageHeaders;
import org.springframework.kafka.support.converter.RecordMessageConverter;
@@ -110,7 +108,7 @@ public class KafkaInboundGateway<K, V, R> extends MessagingGatewaySupport implem
/**
* Set the message converter; must be a {@link RecordMessageConverter} or
* {@link BatchMessageConverter} depending on mode.
* {@link org.springframework.kafka.support.converter.BatchMessageConverter} depending on mode.
* @param messageConverter the converter.
*/
public void setMessageConverter(RecordMessageConverter messageConverter) {
@@ -216,7 +214,7 @@ public class KafkaInboundGateway<K, V, R> extends MessagingGatewaySupport implem
* If there's a retry template, it will set the attributes holder via the listener. If
* there's no retry template, but there's an error channel, we create a new attributes
* holder here. If an attributes holder exists (by either method), we set the
* attributes for use by the {@link ErrorMessageStrategy}.
* attributes for use by the {@link org.springframework.integration.support.ErrorMessageStrategy}.
* @param record the record.
* @param message the message.
*/

View File

@@ -428,23 +428,11 @@ public class KafkaMessageSource<K, V> extends AbstractMessageSource<Object> impl
if (this.paused && this.recordsIterator == null) {
this.logger.debug("Consumer is paused; no records will be returned");
}
ConsumerRecord<K, V> record;
if (this.recordsIterator != null) {
record = nextRecord();
}
else {
synchronized (this.consumerMonitor) {
ConsumerRecords<K, V> records = this.consumer
.poll(this.assignedPartitions.isEmpty() ? this.assignTimeout : this.pollTimeout);
if (records == null || records.count() == 0) {
return null;
}
this.remainingCount.set(records.count());
this.recordsIterator = records.iterator();
record = nextRecord();
}
}
return recordToMessage(record);
ConsumerRecord<K, V> record = pollRecord();
return record != null
? recordToMessage(record)
: null;
}
protected void createConsumer() {
@@ -461,48 +449,7 @@ public class KafkaMessageSource<K, V> extends AbstractMessageSource<Object> impl
this.consumer.subscribe(topicPattern, rebalanceCallback);
}
else if (partitions != null) {
List<TopicPartition> topicPartitionsToAssign =
Arrays.stream(partitions)
.map(TopicPartitionOffset::getTopicPartition)
.collect(Collectors.toList());
this.consumer.assign(topicPartitionsToAssign);
this.assignedPartitions.addAll(topicPartitionsToAssign);
for (TopicPartitionOffset partition : partitions) {
if (TopicPartitionOffset.SeekPosition.BEGINNING.equals(partition.getPosition())) {
this.consumer.seekToBeginning(Collections.singleton(partition.getTopicPartition()));
}
else if (TopicPartitionOffset.SeekPosition.END.equals(partition.getPosition())) {
this.consumer.seekToEnd(Collections.singleton(partition.getTopicPartition()));
}
else {
TopicPartition topicPartition = partition.getTopicPartition();
Long offset = partition.getOffset();
if (offset != null) {
long newOffset = offset;
if (offset < 0) {
if (!partition.isRelativeToCurrent()) {
this.consumer.seekToEnd(Collections.singleton(topicPartition));
continue;
}
newOffset = Math.max(0, this.consumer.position(topicPartition) + offset);
}
else if (partition.isRelativeToCurrent()) {
newOffset = this.consumer.position(topicPartition) + offset;
}
try {
this.consumer.seek(topicPartition, newOffset);
}
catch (Exception e) {
this.logger.error("Failed to set initial offset for " + topicPartition
+ " at " + newOffset + ". Position is " + this.consumer
.position(topicPartition), e);
}
}
}
}
assignAndSeekPartitionts(partitions);
}
else {
this.consumer.subscribe(Arrays.asList(this.consumerProperties.getTopics()), // NOSONAR
@@ -511,6 +458,70 @@ public class KafkaMessageSource<K, V> extends AbstractMessageSource<Object> impl
}
}
private void assignAndSeekPartitionts(TopicPartitionOffset[] partitions) {
List<TopicPartition> topicPartitionsToAssign =
Arrays.stream(partitions)
.map(TopicPartitionOffset::getTopicPartition)
.collect(Collectors.toList());
this.consumer.assign(topicPartitionsToAssign);
this.assignedPartitions.addAll(topicPartitionsToAssign);
for (TopicPartitionOffset partition : partitions) {
if (TopicPartitionOffset.SeekPosition.BEGINNING.equals(partition.getPosition())) {
this.consumer.seekToBeginning(Collections.singleton(partition.getTopicPartition()));
}
else if (TopicPartitionOffset.SeekPosition.END.equals(partition.getPosition())) {
this.consumer.seekToEnd(Collections.singleton(partition.getTopicPartition()));
}
else {
TopicPartition topicPartition = partition.getTopicPartition();
Long offset = partition.getOffset();
if (offset != null) {
long newOffset = offset;
if (offset < 0) {
if (!partition.isRelativeToCurrent()) {
this.consumer.seekToEnd(Collections.singleton(topicPartition));
continue;
}
newOffset = Math.max(0, this.consumer.position(topicPartition) + offset);
}
else if (partition.isRelativeToCurrent()) {
newOffset = this.consumer.position(topicPartition) + offset;
}
try {
this.consumer.seek(topicPartition, newOffset);
}
catch (Exception e) {
this.logger.error("Failed to set initial offset for " + topicPartition
+ " at " + newOffset + ". Position is " + this.consumer
.position(topicPartition), e);
}
}
}
}
}
@Nullable
private ConsumerRecord<K, V> pollRecord() {
if (this.recordsIterator != null) {
return nextRecord();
}
else {
synchronized (this.consumerMonitor) {
ConsumerRecords<K, V> records = this.consumer
.poll(this.assignedPartitions.isEmpty() ? this.assignTimeout : this.pollTimeout);
if (records == null || records.count() == 0) {
return null;
}
this.remainingCount.set(records.count());
this.recordsIterator = records.iterator();
return nextRecord();
}
}
}
private ConsumerRecord<K, V> nextRecord() {
ConsumerRecord<K, V> record;
record = this.recordsIterator.next();

View File

@@ -566,8 +566,8 @@ public class KafkaProducerMessageHandler<K, V> extends AbstractReplyProducingMes
ListenableFuture<SendResult<K, V>> future, MessageChannel metadataChannel)
throws InterruptedException, ExecutionException {
final MessageChannel sendFailureChannel = getSendFailureChannel();
if (sendFailureChannel != null || metadataChannel != null) {
final MessageChannel failureChannel = getSendFailureChannel();
if (failureChannel != null || metadataChannel != null) {
future.addCallback(new ListenableFutureCallback<SendResult<K, V>>() { // NOSONAR
@Override
@@ -581,8 +581,8 @@ public class KafkaProducerMessageHandler<K, V> extends AbstractReplyProducingMes
@Override
public void onFailure(Throwable ex) {
if (sendFailureChannel != null) {
KafkaProducerMessageHandler.this.messagingTemplate.send(sendFailureChannel,
if (failureChannel != null) {
KafkaProducerMessageHandler.this.messagingTemplate.send(failureChannel,
KafkaProducerMessageHandler.this.errorMessageStrategy.buildErrorMessage(
new KafkaSendFailureException(message, producerRecord, ex), null));
}