chore(citrus-jms): review and code cleanup

pr: #1224 `citrus-jms` module.
citrusframework · Oct 24, 2024 · fa0f5a3 · fa0f5a3
1 parent 875c8c4
commit fa0f5a3
Show file tree

Hide file tree

Showing 9 changed files with 26 additions and 30 deletions.
diff --git a/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/actions/PurgeJmsQueuesAction.java b/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/actions/PurgeJmsQueuesAction.java
@@ -144,7 +144,7 @@ private void purgeQueue(Queue queue, Session session) throws JMSException {
      */
     private void purgeDestination(Destination destination, Session session, String destinationName) throws JMSException {
         if (logger.isDebugEnabled()) {
-            logger.debug("Try to purge destination " + destinationName);
+            logger.debug("Try to purge destination {}", destinationName);
         }
 
         int messagesPurged = 0;
@@ -155,7 +155,7 @@ private void purgeDestination(Destination destination, Session session, String d
                 message = (receiveTimeout >= 0) ? messageConsumer.receive(receiveTimeout) : messageConsumer.receive();
 
                 if (message != null) {
-                    logger.debug("Removed message from destination " + destinationName);
+                    logger.debug("Removed message from destination {}", destinationName);
                     messagesPurged++;
 
                     try {
@@ -167,7 +167,7 @@ private void purgeDestination(Destination destination, Session session, String d
             } while (message != null);
 
             if (logger.isDebugEnabled()) {
-                logger.debug("Purged " + messagesPurged + " messages from destination");
+                logger.debug("Purged {} messages from destination", messagesPurged);
             }
         } finally {
             JmsUtils.closeMessageConsumer(messageConsumer);

diff --git a/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsConsumer.java b/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsConsumer.java
@@ -80,7 +80,7 @@ private jakarta.jms.Message receive(String destinationName, String selector) {
         jakarta.jms.Message receivedJmsMessage;
 
         if (logger.isDebugEnabled()) {
-            logger.debug("Receiving JMS message on destination: '" + getDestinationNameWithSelector(destinationName, selector) + "'");
+            logger.debug("Receiving JMS message on destination: '{}'", getDestinationNameWithSelector(destinationName, selector));
         }
 
         if (StringUtils.hasText(selector)) {
@@ -93,7 +93,7 @@ private jakarta.jms.Message receive(String destinationName, String selector) {
             throw new MessageTimeoutException(endpointConfiguration.getTimeout(), getDestinationNameWithSelector(destinationName, selector));
         }
 
-        logger.info("Received JMS message on destination: '" + getDestinationNameWithSelector(destinationName, selector) + "'");
+        logger.info("Received JMS message on destination: '{}'", getDestinationNameWithSelector(destinationName, selector));
 
         return receivedJmsMessage;
     }
@@ -108,7 +108,7 @@ private jakarta.jms.Message receive(Destination destination, String selector) {
         jakarta.jms.Message receivedJmsMessage;
 
         if (logger.isDebugEnabled()) {
-            logger.debug("Receiving JMS message on destination: '" + getDestinationNameWithSelector(endpointConfiguration.getDestinationName(destination), selector) + "'");
+            logger.debug("Receiving JMS message on destination: '{}'", getDestinationNameWithSelector(endpointConfiguration.getDestinationName(destination), selector));
         }
 
         if (StringUtils.hasText(selector)) {
@@ -121,7 +121,7 @@ private jakarta.jms.Message receive(Destination destination, String selector) {
             throw new MessageTimeoutException(endpointConfiguration.getTimeout(), getDestinationNameWithSelector(endpointConfiguration.getDestinationName(destination), selector));
         }
 
-        logger.info("Received JMS message on destination: '" + getDestinationNameWithSelector(endpointConfiguration.getDestinationName(destination), selector) + "'");
+        logger.info("Received JMS message on destination: '{}'", getDestinationNameWithSelector(endpointConfiguration.getDestinationName(destination), selector));
 
         return receivedJmsMessage;
     }

diff --git a/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsProducer.java b/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsProducer.java
@@ -81,7 +81,7 @@ public void send(final Message message, final TestContext context) {
      */
     private void send(Message message, String destinationName, TestContext context) {
         if (logger.isDebugEnabled()) {
-            logger.debug("Sending JMS message to destination: '" + destinationName + "'");
+            logger.debug("Sending JMS message to destination: '{}'", destinationName);
         }
 
         endpointConfiguration.getJmsTemplate().send(destinationName, session -> {
@@ -90,7 +90,7 @@ private void send(Message message, String destinationName, TestContext context)
             return jmsMessage;
         });
 
-        logger.info("Message was sent to JMS destination: '" + destinationName + "'");
+        logger.info("Message was sent to JMS destination: '{}'", destinationName);
     }
 
     /**
@@ -101,7 +101,7 @@ private void send(Message message, String destinationName, TestContext context)
      */
     private void send(Message message, Destination destination, TestContext context) {
         if (logger.isDebugEnabled()) {
-            logger.debug("Sending JMS message to destination: '" + endpointConfiguration.getDestinationName(destination) + "'");
+            logger.debug("Sending JMS message to destination: '{}'", endpointConfiguration.getDestinationName(destination));
         }
 
         endpointConfiguration.getJmsTemplate().send(destination, session -> {
@@ -110,7 +110,7 @@ private void send(Message message, Destination destination, TestContext context)
             return jmsMessage;
         });
 
-        logger.info("Message was sent to JMS destination: '" + endpointConfiguration.getDestinationName(destination) + "'");
+        logger.info("Message was sent to JMS destination: '{}'", endpointConfiguration.getDestinationName(destination));
     }
 
     @Override

diff --git a/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsSyncConsumer.java b/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsSyncConsumer.java
@@ -79,7 +79,7 @@ public void send(final Message message, final TestContext context) {
         ObjectHelper.assertNotNull(replyDestination, "Failed to find JMS reply destination for message correlation key: '" + correlationKey + "'");
 
         if (logger.isDebugEnabled()) {
-            logger.debug("Sending JMS message to destination: '" + endpointConfiguration.getDestinationName(replyDestination) + "'");
+            logger.debug("Sending JMS message to destination: '{}'", endpointConfiguration.getDestinationName(replyDestination));
         }
 
         endpointConfiguration.getJmsTemplate().send(replyDestination, session -> {
@@ -90,7 +90,7 @@ public void send(final Message message, final TestContext context) {
 
         context.onOutboundMessage(message);
 
-        logger.info("Message was sent to JMS destination: '" + endpointConfiguration.getDestinationName(replyDestination) + "'");
+        logger.info("Message was sent to JMS destination: '{}'", endpointConfiguration.getDestinationName(replyDestination));
     }
 
     /**
@@ -107,8 +107,7 @@ public void saveReplyDestination(JmsMessage jmsMessage, TestContext context) {
             correlationManager.saveCorrelationKey(correlationKeyName, correlationKey, context);
             correlationManager.store(correlationKey, jmsMessage.getReplyTo());
         }  else {
-            logger.warn("Unable to retrieve reply to destination for message \n" +
-                    jmsMessage + "\n - no reply to destination found in message headers!");
+            logger.warn("Unable to retrieve reply to destination for message \n{}\n - no reply to destination found in message headers!", jmsMessage);
         }
     }
 

diff --git a/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsSyncProducer.java b/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsSyncProducer.java
@@ -91,7 +91,7 @@ public void send(Message message, TestContext context) {
             Destination destination;
             if (endpointConfiguration.getDestination() != null) {
                 if (logger.isDebugEnabled()) {
-                    logger.debug("Sending JMS message to destination: '" + endpointConfiguration.getDestinationName(endpointConfiguration.getDestination()) + "'");
+                    logger.debug("Sending JMS message to destination: '{}'", endpointConfiguration.getDestinationName(endpointConfiguration.getDestination()));
                 }
 
                 destination = endpointConfiguration.getDestination();
@@ -103,7 +103,7 @@ public void send(Message message, TestContext context) {
                 }
             } else if (endpointConfiguration.getJmsTemplate().getDefaultDestination() != null) {
                 if (logger.isDebugEnabled()) {
-                    logger.debug("Sending JMS message to destination: '" + endpointConfiguration.getDestinationName(endpointConfiguration.getJmsTemplate().getDefaultDestination()) + "'");
+                    logger.debug("Sending JMS message to destination: '{}'", endpointConfiguration.getDestinationName(endpointConfiguration.getJmsTemplate().getDefaultDestination()));
                 }
 
                 destination = endpointConfiguration.getJmsTemplate().getDefaultDestination();
@@ -201,8 +201,7 @@ protected void createConnection() throws JMSException {
                 connection = ((TopicConnectionFactory) endpointConfiguration.getConnectionFactory()).createTopicConnection();
                 connection.setClientID(getName());
             } else {
-                logger.warn("Not able to create a connection with connection factory '" + endpointConfiguration.getConnectionFactory() + "'" +
-                        " when using setting 'publish-subscribe-domain' (=" + endpointConfiguration.isPubSubDomain() + ")");
+                logger.warn("Not able to create a connection with connection factory '{}' when using setting 'publish-subscribe-domain' (={})", endpointConfiguration.getConnectionFactory(), endpointConfiguration.isPubSubDomain());
 
                 connection = endpointConfiguration.getConnectionFactory().createConnection();
             }
@@ -224,8 +223,7 @@ protected void createSession(Connection connection) throws JMSException {
             } else if (endpointConfiguration.isPubSubDomain() && endpointConfiguration.getConnectionFactory() instanceof TopicConnectionFactory) {
                 session = ((TopicConnection) connection).createTopicSession(false, Session.AUTO_ACKNOWLEDGE);
             } else {
-                logger.warn("Not able to create a session with connection factory '" + endpointConfiguration.getConnectionFactory() + "'" +
-                        " when using setting 'publish-subscribe-domain' (=" + endpointConfiguration.isPubSubDomain() + ")");
+                logger.warn("Not able to create a session with connection factory '{}' when using setting 'publish-subscribe-domain' (={})", endpointConfiguration.getConnectionFactory(), endpointConfiguration.isPubSubDomain());
 
                 session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
             }
@@ -269,7 +267,7 @@ private void deleteTemporaryDestination(Destination destination) {
                 ((TemporaryTopic) destination).delete();
             }
         } catch (JMSException e) {
-            logger.error("Error while deleting temporary destination '" + destination + "'", e);
+            logger.error("Error while deleting temporary destination '{}'", destination, e);
         }
     }
 
@@ -310,7 +308,7 @@ private Destination getReplyDestination(Session session, Message message) throws
      */
     private Destination resolveDestination(String destinationName) throws JMSException {
         if (logger.isDebugEnabled()) {
-            logger.debug("Sending JMS message to destination: '" + destinationName + "'");
+            logger.debug("Sending JMS message to destination: '{}'", destinationName);
         }
 
         return resolveDestinationName(destinationName, session);

diff --git a/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsTopicSubscriber.java b/endpoints/citrus-jms/src/main/java/org/citrusframework/jms/endpoint/JmsTopicSubscriber.java
@@ -50,7 +50,7 @@
 public class JmsTopicSubscriber extends JmsConsumer implements Runnable {
 
     /** Logger */
-    private static final Logger logger = LoggerFactory.getLogger(JmsConsumer.class);
+    private static final Logger logger = LoggerFactory.getLogger(JmsTopicSubscriber.class);
 
     /** Boolean flag for continued message consumption, if false stop */
     private boolean running = true;
@@ -118,7 +118,7 @@ public void run() {
 
             TopicSubscriber subscriber;
             if (endpointConfiguration.isDurableSubscription()) {
-                logger.debug(String.format("Create JMS topic durable subscription '%s'", Optional.ofNullable(endpointConfiguration.getDurableSubscriberName()).orElseGet(this::getName)));
+                logger.debug("Create JMS topic durable subscription '{}'", Optional.ofNullable(endpointConfiguration.getDurableSubscriberName()).orElseGet(this::getName));
                 subscriber = session.createDurableSubscriber(topic, Optional.ofNullable(endpointConfiguration.getDurableSubscriberName()).orElseGet(this::getName));
             } else {
                 logger.debug("Create JMS topic subscription");
@@ -137,12 +137,12 @@ public void run() {
                     Message message = endpointConfiguration.getMessageConverter().convertInbound(event, endpointConfiguration, context);
 
                     if (logger.isDebugEnabled()) {
-                        logger.debug(String.format("Received topic event '%s'", message.getId()));
+                        logger.debug("Received topic event '{}'", message.getId());
                     }
                     messageQueue.createProducer().send(message, context);
                 } else {
                     if (logger.isDebugEnabled()) {
-                        logger.debug("Topic subscriber received null message - continue after " + endpointConfiguration.getPollingInterval() + " milliseconds");
+                        logger.debug("Topic subscriber received null message - continue after {} milliseconds", endpointConfiguration.getPollingInterval());
                     }
 
                     try {

diff --git a/...nts/citrus-jms/src/main/java/org/citrusframework/jms/message/SoapJmsMessageConverter.java b/...nts/citrus-jms/src/main/java/org/citrusframework/jms/message/SoapJmsMessageConverter.java
@@ -122,7 +122,7 @@ public org.citrusframework.message.Message convertInbound(Message jmsMessage, Jm
     public Message createJmsMessage(org.citrusframework.message.Message message, Session session, JmsEndpointConfiguration endpointConfiguration, TestContext context) {
         String payload = message.getPayload(String.class);
 
-        logger.debug("Creating SOAP message from payload: " + payload);
+        logger.debug("Creating SOAP message from payload: {}", payload);
 
         try {
             SoapMessage soapMessage = soapMessageFactory.createWebServiceMessage();

diff --git a/...us-jms/src/test/java/org/citrusframework/jms/integration/SyncJmsTopicCommunicationIT.java b/...us-jms/src/test/java/org/citrusframework/jms/integration/SyncJmsTopicCommunicationIT.java
@@ -18,7 +18,6 @@
 
 import org.citrusframework.annotations.CitrusTestSource;
 import org.citrusframework.testng.spring.TestNGCitrusSpringSupport;
-import org.testng.annotations.Ignore;
 import org.testng.annotations.Test;
 
 import static org.citrusframework.common.TestLoader.SPRING;

diff --git a/...rus-jms/src/test/java/org/citrusframework/jms/integration/service/LoggingInterceptor.java b/...rus-jms/src/test/java/org/citrusframework/jms/integration/service/LoggingInterceptor.java
@@ -28,7 +28,7 @@ public class LoggingInterceptor implements ChannelInterceptor {
     @Override
     public Message<?> preSend(Message<?> message, MessageChannel channel) {
         if (logger.isDebugEnabled()) {
-            logger.debug(channel.toString() + ": " + message.getPayload());
+            logger.debug("{}: {}", channel.toString(), message.getPayload());
         }
 
         if (message.getPayload() instanceof Throwable) {