Use LogAccessor from SF

* Change main classes to use a `LogAccessor` API to simplify code flow
* Fix tests according `LogAccessor` property
* Fix some Sonar smells
This commit is contained in:
Artem Bilan
2020-10-06 13:56:50 -04:00
parent a66e82b0aa
commit c7ff99a4e8
95 changed files with 1165 additions and 1432 deletions

View File

@@ -25,13 +25,11 @@ import javax.jms.JMSException;
import javax.jms.MessageProducer;
import javax.jms.Session;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.BeanFactoryAware;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.core.log.LogAccessor;
import org.springframework.integration.core.MessagingTemplate;
import org.springframework.integration.gateway.MessagingGatewaySupport;
import org.springframework.integration.support.DefaultMessageBuilderFactory;
@@ -66,7 +64,7 @@ public class ChannelPublishingJmsMessageListener
implements SessionAwareMessageListener<javax.jms.Message>, InitializingBean,
TrackableComponent, BeanFactoryAware {
protected final Log logger = LogFactory.getLog(getClass()); // NOSONAR final
protected final LogAccessor logger = new LogAccessor(getClass()); // NOSONAR final
private final GatewayDelegate gatewayDelegate = new GatewayDelegate();
@@ -315,13 +313,14 @@ public class ChannelPublishingJmsMessageListener
public void onMessage(javax.jms.Message jmsMessage, Session session) throws JMSException {
Message<?> requestMessage;
try {
Object result = jmsMessage;
final Object result;
if (this.extractRequestPayload) {
result = this.messageConverter.fromMessage(jmsMessage);
if (this.logger.isDebugEnabled()) {
this.logger.debug("converted JMS Message [" + jmsMessage + "] to integration Message payload ["
+ result + "]");
}
this.logger.debug(() -> "converted JMS Message [" + jmsMessage + "] to integration Message payload ["
+ result + "]");
}
else {
result = jmsMessage;
}
Map<String, Object> headers = this.headerMapper.toHeaders(jmsMessage);
@@ -349,14 +348,15 @@ public class ChannelPublishingJmsMessageListener
Message<?> replyMessage = this.gatewayDelegate.sendAndReceiveMessage(requestMessage);
if (replyMessage != null) {
Destination destination = getReplyDestination(jmsMessage, session);
if (this.logger.isDebugEnabled()) {
this.logger.debug("Reply destination: " + destination);
}
this.logger.debug(() -> "Reply destination: " + destination);
// convert SI Message to JMS Message
Object replyResult = replyMessage;
final Object replyResult;
if (this.extractReplyPayload) {
replyResult = replyMessage.getPayload();
}
else {
replyResult = replyMessage;
}
try {
javax.jms.Message jmsReply = this.messageConverter.toMessage(replyResult, session);
// map SI Message Headers to JMS Message Properties/Headers
@@ -364,9 +364,9 @@ public class ChannelPublishingJmsMessageListener
copyCorrelationIdFromRequestToReply(jmsMessage, jmsReply);
sendReply(jmsReply, destination, session);
}
catch (RuntimeException e) {
this.logger.error("Failed to generate JMS Reply Message from: " + replyResult, e);
throw e;
catch (RuntimeException ex) {
this.logger.error(ex, () -> "Failed to generate JMS Reply Message from: " + replyResult);
throw ex;
}
}
else {
@@ -404,8 +404,8 @@ public class ChannelPublishingJmsMessageListener
if (value != null) {
replyMessage.setStringProperty(this.correlationKey, value);
}
else if (this.logger.isWarnEnabled()) {
this.logger.warn("No property value available on request Message for correlationKey '"
else {
this.logger.warn(() -> "No property value available on request Message for correlationKey '"
+ this.correlationKey + "'");
}
}

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.
@@ -24,64 +24,64 @@ package org.springframework.integration.jms;
*/
abstract class DynamicJmsTemplateProperties {
private static final ThreadLocal<Integer> priorityHolder = new ThreadLocal<>();
private static final ThreadLocal<Integer> PRIORITY_HOLDER = new ThreadLocal<>();
private static final ThreadLocal<Long> receiveTimeoutHolder = new ThreadLocal<>();
private static final ThreadLocal<Long> RECEIVE_TIMEOUT_HOLDER = new ThreadLocal<>();
private static final ThreadLocal<Integer> deliverModeHolder = new ThreadLocal<>();
private static final ThreadLocal<Integer> DELIVER_MODE_HOLDER = new ThreadLocal<>();
private static final ThreadLocal<Long> timeToLiveHolder = new ThreadLocal<>();
private static final ThreadLocal<Long> TIME_TO_LIVE_HOLDER = new ThreadLocal<>();
private DynamicJmsTemplateProperties() {
}
public static Integer getPriority() {
return priorityHolder.get();
return PRIORITY_HOLDER.get();
}
public static void setPriority(Integer priority) {
priorityHolder.set(priority);
PRIORITY_HOLDER.set(priority);
}
public static void clearPriority() {
priorityHolder.remove();
PRIORITY_HOLDER.remove();
}
public static Long getReceiveTimeout() {
return receiveTimeoutHolder.get();
return RECEIVE_TIMEOUT_HOLDER.get();
}
public static void setReceiveTimeout(Long receiveTimeout) {
receiveTimeoutHolder.set(receiveTimeout);
RECEIVE_TIMEOUT_HOLDER.set(receiveTimeout);
}
public static void clearReceiveTimeout() {
receiveTimeoutHolder.remove();
RECEIVE_TIMEOUT_HOLDER.remove();
}
public static Integer getDeliveryMode() {
return deliverModeHolder.get();
return DELIVER_MODE_HOLDER.get();
}
public static void setDeliveryMode(Integer deliveryMode) {
deliverModeHolder.set(deliveryMode);
DELIVER_MODE_HOLDER.set(deliveryMode);
}
public static void clearDeliveryMode() {
deliverModeHolder.remove();
DELIVER_MODE_HOLDER.remove();
}
public static Long getTimeToLive() {
return timeToLiveHolder.get();
return TIME_TO_LIVE_HOLDER.get();
}
public static void setTimeToLive(Long timeToLive) {
timeToLiveHolder.set(timeToLive);
TIME_TO_LIVE_HOLDER.set(timeToLive);
}
public static void clearTimeToLive() {
timeToLiveHolder.remove();
TIME_TO_LIVE_HOLDER.remove();
}
}

View File

@@ -730,9 +730,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
synchronized (this.lifeCycleMonitor) {
this.lastSend = System.currentTimeMillis();
if (!this.replyContainer.isRunning()) {
if (logger.isDebugEnabled()) {
logger.debug(getComponentName() + ": Starting reply container.");
}
logger.debug(() -> getComponentName() + ": Starting reply container.");
this.replyContainer.start();
this.idleTask = getTaskScheduler().scheduleAtFixedRate(new IdleContainerStopper(),
this.idleReplyContainerTimeout / 2);
@@ -764,13 +762,14 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
}
private AbstractIntegrationMessageBuilder<?> buildReply(javax.jms.Message jmsReply) throws JMSException {
Object result = jmsReply;
Object result;
if (this.extractReplyPayload) {
result = this.messageConverter.fromMessage(jmsReply);
if (logger.isDebugEnabled()) {
logger.debug("converted JMS Message [" + jmsReply + "] to integration Message payload [" + result +
"]");
}
logger.debug(() ->
"converted JMS Message [" + jmsReply + "] to integration Message payload [" + result + "]");
}
else {
result = jmsReply;
}
Map<String, Object> jmsReplyHeaders = this.headerMapper.toHeaders(jmsReply);
@@ -805,9 +804,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
jmsRequest.setJMSReplyTo(replyTo);
connection.start();
if (logger.isDebugEnabled()) {
logger.debug("ReplyTo: " + replyTo);
}
logger.debug(() -> "ReplyTo: " + replyTo);
Integer priority = StaticMessageHeaderAccessor.getPriority(requestMessage);
if (priority == null) {
@@ -847,7 +844,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
Session session = null;
Destination replyTo = null;
try {
session = this.createSession(connection);
session = createSession(connection);
// convert to JMS Message
Object objectToSend = requestMessage;
@@ -859,12 +856,11 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
// map headers
this.headerMapper.fromHeaders(requestMessage.getHeaders(), jmsRequest);
replyTo = determineReplyDestination(requestMessage, session);
jmsRequest.setJMSReplyTo(replyTo);
Destination theReplyTo = determineReplyDestination(requestMessage, session);
jmsRequest.setJMSReplyTo(theReplyTo);
connection.start();
if (logger.isDebugEnabled()) {
logger.debug("ReplyTo: " + replyTo);
}
logger.debug(() -> "ReplyTo: " + theReplyTo);
replyTo = theReplyTo;
Integer priority = StaticMessageHeaderAccessor.getPriority(requestMessage);
if (priority == null) {
@@ -888,7 +884,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
}
finally {
JmsUtils.closeSession(session);
this.deleteDestinationIfTemporary(replyTo);
deleteDestinationIfTemporary(replyTo);
ConnectionFactoryUtils.releaseConnection(connection, this.connectionFactory, true);
}
}
@@ -920,7 +916,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
messageSelector = "JMSCorrelationID = '" + jmsRequest.getJMSCorrelationID() + "'";
}
this.sendRequestMessage(jmsRequest, messageProducer, priority);
sendRequestMessage(jmsRequest, messageProducer, priority);
return retryableReceiveReply(session, replyTo, messageSelector);
}
finally {
@@ -939,8 +935,8 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
try {
messageProducer = session.createProducer(reqDestination);
messageConsumer = session.createConsumer(replyTo);
this.sendRequestMessage(jmsRequest, messageProducer, priority);
return this.receiveReplyMessage(messageConsumer);
sendRequestMessage(jmsRequest, messageProducer, priority);
return receiveReplyMessage(messageConsumer);
}
finally {
JmsUtils.closeMessageProducer(messageProducer);
@@ -1015,9 +1011,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
}
catch (JMSException e) { // NOSONAR - exception as flow control
exception = e;
if (logger.isDebugEnabled()) {
logger.debug("Connection lost waiting for reply, retrying: " + e.getMessage());
}
logger.debug(() -> "Connection lost waiting for reply, retrying: " + e.getMessage());
do {
try {
consumerConnection = createConnection();
@@ -1026,9 +1020,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
}
catch (JMSException ee) { // NOSONAR - exception as flow control
exception = ee;
if (logger.isDebugEnabled()) {
logger.debug("Could not reconnect, retrying: " + ee.getMessage());
}
logger.debug(() -> "Could not reconnect, retrying: " + ee.getMessage());
try {
Thread.sleep(1000); // NOSONAR
}
@@ -1078,9 +1070,8 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
jmsRequest.setJMSCorrelationID(null);
}
LinkedBlockingQueue<javax.jms.Message> replyQueue = null;
if (logger.isDebugEnabled()) {
logger.debug(this.getComponentName() + " Sending message with correlationId " + correlation);
}
String correlationToLog = correlation;
logger.debug(() -> getComponentName() + " Sending message with correlationId " + correlationToLog);
SettableListenableFuture<AbstractIntegrationMessageBuilder<?>> future = null;
boolean async = isAsync();
if (!async) {
@@ -1121,10 +1112,8 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
this.sendRequestMessage(jmsRequest, messageProducer, priority);
correlation = jmsRequest.getJMSMessageID();
if (logger.isDebugEnabled()) {
logger.debug(getComponentName() + " Sent message with correlationId " + correlation);
}
String correlationToLog = correlation;
logger.debug(() -> getComponentName() + " Sent message with correlationId " + correlationToLog);
this.replies.put(correlation, replyQueue);
/*
@@ -1133,9 +1122,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
synchronized (this.earlyOrLateReplies) {
TimedReply timedReply = this.earlyOrLateReplies.remove(correlation);
if (timedReply != null) {
if (logger.isDebugEnabled()) {
logger.debug("Found early reply with correlationId " + correlation);
}
logger.debug(() -> "Found early reply with correlationId " + correlationToLog);
replyQueue.add(timedReply.getReply());
}
}
@@ -1162,20 +1149,20 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
try {
reply = replyQueue.poll(this.receiveTimeout, TimeUnit.MILLISECONDS);
}
catch (InterruptedException e) {
logger.error("Interrupted while awaiting reply; treated as a timeout", e);
catch (InterruptedException ex) {
Thread.currentThread().interrupt();
logger.error(ex, "Interrupted while awaiting reply; treated as a timeout");
}
}
if (logger.isDebugEnabled()) {
if (reply == null) {
logger.debug(getComponentName() + " Timed out waiting for reply with CorrelationId "
+ correlationId);
javax.jms.Message replyToLog = reply;
logger.debug(() -> {
if (replyToLog == null) {
return getComponentName() + " Timed out waiting for reply with CorrelationId " + correlationId;
}
else {
logger.debug(getComponentName() + " Obtained reply with CorrelationId " + correlationId);
return getComponentName() + " Obtained reply with CorrelationId " + correlationId;
}
}
});
return reply;
}
@@ -1197,13 +1184,11 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
future.setException(new JmsTimeoutException("No reply in " + this.receiveTimeout + " ms"));
}
else {
if (logger.isDebugEnabled()) {
logger.debug("Reply expired and reply not required for " + correlationId);
}
logger.debug(() -> "Reply expired and reply not required for " + correlationId);
}
}
catch (Exception e) {
logger.error("Exception while expiring future", e);
catch (Exception ex) {
logger.error(ex, "Exception while expiring future");
}
}
}
@@ -1265,9 +1250,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
public void onMessage(javax.jms.Message message) {
String correlation = null;
try {
if (logger.isTraceEnabled()) {
logger.trace(getComponentName() + " Received " + message);
}
logger.trace(() -> getComponentName() + " Received " + message);
if (this.correlationKey == null ||
this.correlationKey.equals("JMSCorrelationID") ||
this.correlationKey.equals("JMSCorrelationID*")) {
@@ -1284,10 +1267,9 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
onMessageSync(message, correlation);
}
}
catch (Exception e) {
if (logger.isWarnEnabled()) {
logger.warn("Failed to consume reply with correlationId " + correlation, e);
}
catch (Exception ex) {
String correlationToLog = correlation;
logger.warn(ex, () -> "Failed to consume reply with correlationId " + correlationToLog);
}
}
@@ -1298,7 +1280,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
future.set(buildReply(message));
}
else {
logger.warn("Late reply for " + correlationId);
logger.warn(() -> "Late reply for " + correlationId);
}
}
@@ -1318,24 +1300,18 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
synchronized (this.earlyOrLateReplies) {
queue = this.replies.get(correlationId);
if (queue == null) {
if (logger.isDebugEnabled()) {
logger.debug("Reply for correlationId " + correlationId + " received early or late");
}
logger.debug(() -> "Reply for correlationId " + correlationId + " received early or late");
this.earlyOrLateReplies.put(correlationId, new TimedReply(message));
}
}
}
if (queue != null) {
if (logger.isDebugEnabled()) {
logger.debug("Received reply with correlationId " + correlationId);
}
logger.debug(() -> "Received reply with correlationId " + correlationId);
queue.add(message);
}
}
catch (Exception e) {
if (logger.isWarnEnabled()) {
logger.warn("Failed to consume reply with correlationId " + correlationId, e);
}
catch (Exception ex) {
logger.warn(() -> "Failed to consume reply with correlationId " + correlationId);
}
}
@@ -1466,9 +1442,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
while (lateReplyIterator.hasNext()) {
Entry<String, TimedReply> entry = lateReplyIterator.next();
if (entry.getValue().getTimeStamp() < expired) {
if (logger.isDebugEnabled()) {
logger.debug("Removing late reply for correlationId " + entry.getKey());
}
logger.debug(() -> "Removing late reply for correlationId " + entry.getKey());
lateReplyIterator.remove();
}
}
@@ -1494,9 +1468,7 @@ public class JmsOutboundGateway extends AbstractReplyProducingMessageHandler
&& JmsOutboundGateway.this.replies.size() == 0 &&
JmsOutboundGateway.this.replyContainer.isRunning()) {
if (logger.isDebugEnabled()) {
logger.debug(getComponentName() + ": Stopping idle reply container.");
}
logger.debug(() -> getComponentName() + ": Stopping idle reply container.");
JmsOutboundGateway.this.replyContainer.stop();
JmsOutboundGateway.this.idleTask.cancel(false);
JmsOutboundGateway.this.idleTask = null;

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.
@@ -104,7 +104,7 @@ public class JmsChannelParser extends AbstractChannelParser {
if (!("auto".equals(cache) || "consumer".equals(cache))) {
parserContext.getReaderContext().warning(
"'cache' attribute not actively supported for listener container of type \"simple\". " +
"Effective runtime behavior will be equivalent to \"consumer\" / \"auto\".", element);
"Effective runtime behavior will be equivalent to \"consumer\" / \"auto\".", element);
}
}
else {
@@ -123,10 +123,8 @@ public class JmsChannelParser extends AbstractChannelParser {
IntegrationNamespaceUtils.setValueIfAttributeDefined(builder, element, "concurrency");
String prefetch = element.getAttribute("prefetch");
if (StringUtils.hasText(prefetch)) {
if (containerType.startsWith("default")) {
builder.addPropertyValue("maxMessagesPerTask", Integer.valueOf(prefetch));
}
if (StringUtils.hasText(prefetch) && containerType.startsWith("default")) {
builder.addPropertyValue("maxMessagesPerTask", Integer.valueOf(prefetch));
}
return builder;
}

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.
@@ -17,6 +17,7 @@
package org.springframework.integration.jms;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doNothing;
@@ -27,11 +28,11 @@ import javax.jms.InvalidDestinationException;
import javax.jms.JMSException;
import javax.jms.Session;
import org.apache.commons.logging.Log;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.springframework.beans.DirectFieldAccessor;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.core.log.LogAccessor;
import org.springframework.core.task.SimpleAsyncTaskExecutor;
import org.springframework.integration.channel.QueueChannel;
import org.springframework.integration.test.util.TestUtils;
@@ -46,13 +47,14 @@ import org.springframework.messaging.support.GenericMessage;
/**
* @author Mark Fisher
* @author Gary Russell
* @author Artem Bilan
*/
public class ChannelPublishingJmsMessageListenerTests {
private final Session session = new StubSession("test");
@Test(expected = InvalidDestinationException.class)
@Test
public void noReplyToAndNoDefault() throws JMSException {
final QueueChannel requestChannel = new QueueChannel();
startBackgroundReplier(requestChannel);
@@ -63,15 +65,16 @@ public class ChannelPublishingJmsMessageListenerTests {
javax.jms.Message jmsMessage = session.createTextMessage("test");
listener.setBeanFactory(mock(BeanFactory.class));
listener.afterPropertiesSet();
listener.onMessage(jmsMessage, session);
assertThatExceptionOfType(InvalidDestinationException.class)
.isThrownBy(() -> listener.onMessage(jmsMessage, session));
}
@Test
public void testBadConversion() throws Exception {
final QueueChannel requestChannel = new QueueChannel();
ChannelPublishingJmsMessageListener listener = new ChannelPublishingJmsMessageListener();
Log logger = spy(TestUtils.getPropertyValue(listener, "logger", Log.class));
doNothing().when(logger).error(anyString(), any(Throwable.class));
LogAccessor logger = spy(TestUtils.getPropertyValue(listener, "logger", LogAccessor.class));
doNothing().when(logger).error(any(Throwable.class), anyString());
new DirectFieldAccessor(listener).setPropertyValue("logger", logger);
listener.setRequestChannel(requestChannel);
QueueChannel errorChannel = new QueueChannel();
@@ -80,7 +83,7 @@ public class ChannelPublishingJmsMessageListenerTests {
listener.setMessageConverter(new TestMessageConverter() {
@Override
public Object fromMessage(javax.jms.Message message) throws JMSException, MessageConversionException {
public Object fromMessage(javax.jms.Message message) throws MessageConversionException {
return null;
}
@@ -97,7 +100,7 @@ public class ChannelPublishingJmsMessageListenerTests {
private void startBackgroundReplier(final PollableChannel channel) {
new SimpleAsyncTaskExecutor().execute(() -> {
Message<?> request = channel.receive(50000);
Message<?> reply = new GenericMessage<String>(((String) request.getPayload()).toUpperCase());
Message<?> reply = new GenericMessage<>(((String) request.getPayload()).toUpperCase());
((MessageChannel) request.getHeaders().getReplyChannel()).send(reply, 5000);
});
}
@@ -105,13 +108,12 @@ public class ChannelPublishingJmsMessageListenerTests {
private static class TestMessageConverter implements MessageConverter {
@Override
public Object fromMessage(javax.jms.Message message) throws JMSException, MessageConversionException {
public Object fromMessage(javax.jms.Message message) throws MessageConversionException {
return "test-from";
}
@Override
public javax.jms.Message toMessage(Object object, Session session)
throws JMSException, MessageConversionException {
public javax.jms.Message toMessage(Object object, Session session) throws MessageConversionException {
return new StubTextMessage("test-to");
}