From 26afbc223fe697c5db13353752689781db3a142f Mon Sep 17 00:00:00 2001
From: Marcin Grzejszczak <marcin@grzejszczak.pl>
Date: Fri, 22 Dec 2023 10:52:47 +0100
Subject: [PATCH] Adds NPE checks for JMS topics and queues

fixes gh-4505
---
 ...efaultJmsPublishObservationConvention.java | 24 +++++++++----
 ...tJmsPublishObservationConventionTests.java | 35 +++++++++++++++++--
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConvention.java b/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConvention.java
index 2b1ff7d15a..8a6ed5c22f 100644
--- a/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConvention.java
+++ b/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConvention.java
@@ -116,19 +116,31 @@ protected KeyValue destinationName(JmsPublishObservationContext context) {
             Destination jmsDestination = message.getJMSDestination();
             if (jmsDestination instanceof Queue) {
                 Queue queue = (Queue) jmsDestination;
-                return KeyValue.of(HighCardinalityKeyNames.DESTINATION_NAME, queue.getQueueName());
-            }
-            if (jmsDestination instanceof Topic) {
-                Topic topic = (Topic) jmsDestination;
-                return KeyValue.of(HighCardinalityKeyNames.DESTINATION_NAME, topic.getTopicName());
+                String queueName = queue.getQueueName();
+                if (queueName == null) {
+                    return getKeyValueTopic(jmsDestination);
+                }
+                return KeyValue.of(HighCardinalityKeyNames.DESTINATION_NAME, queueName);
             }
-            return DESTINATION_NAME_UNKNOWN;
+            return getKeyValueTopic(jmsDestination);
         }
         catch (JMSException e) {
             return DESTINATION_NAME_UNKNOWN;
         }
     }
 
+    private static KeyValue getKeyValueTopic(Destination jmsDestination) throws JMSException {
+        if (jmsDestination instanceof Topic) {
+            Topic topic = (Topic) jmsDestination;
+            String topicName = topic.getTopicName();
+            if (topicName == null) {
+                return DESTINATION_NAME_UNKNOWN;
+            }
+            return KeyValue.of(HighCardinalityKeyNames.DESTINATION_NAME, topicName);
+        }
+        return DESTINATION_NAME_UNKNOWN;
+    }
+
     protected KeyValue messageId(JmsPublishObservationContext context) {
         try {
             Message message = context.getCarrier();
diff --git a/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConventionTests.java b/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConventionTests.java
index c689a8bbc6..54346d8bb6 100644
--- a/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConventionTests.java
+++ b/micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConventionTests.java
@@ -20,8 +20,7 @@
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 /**
  * Tests for {@link DefaultJmsPublishObservationConvention}.
@@ -96,6 +95,13 @@ void shouldHaveTopicDestinationName() throws Exception {
             .contains(KeyValue.of("messaging.destination.name", "micrometer.test.topic"));
     }
 
+    @Test
+    void shouldHaveTopicNullDestinationName() throws Exception {
+        JmsPublishObservationContext context = new JmsPublishObservationContext(createMessageWithNullTopic());
+        assertThat(convention.getHighCardinalityKeyValues(context))
+            .contains(KeyValue.of("messaging.destination.name", "unknown"));
+    }
+
     @Test
     void shouldHaveUnknownDestinationNameWhenException() throws Exception {
         Message message = createMessageWithQueue();
@@ -163,6 +169,23 @@ void shouldHaveTempDestinationForTemporaryTopic() throws Exception {
             .contains(KeyValue.of("messaging.destination.temporary", "true"));
     }
 
+    @Test
+    void shouldHaveTopicDestinationNameEvenWhenTheTopicAlsoImplementsTheQueueInterface() throws Exception {
+        JmsPublishObservationContext context = new JmsPublishObservationContext(
+                createMessageWithTopicThatAlsoImplementsTheQueueInterface());
+
+        assertThat(convention.getHighCardinalityKeyValues(context))
+            .contains(KeyValue.of("messaging.destination.name", "micrometer.test.topic"));
+    }
+
+    private Message createMessageWithTopicThatAlsoImplementsTheQueueInterface() throws Exception {
+        Topic topic = mock(Topic.class, withSettings().extraInterfaces(Queue.class));
+        when(topic.getTopicName()).thenReturn("micrometer.test.topic");
+        Message message = mock(Message.class);
+        when(message.getJMSDestination()).thenReturn(topic);
+        return message;
+    }
+
     private Message createMessageWithQueue() throws Exception {
         Queue queue = mock(Queue.class);
         when(queue.getQueueName()).thenReturn("micrometer.test.queue");
@@ -187,6 +210,14 @@ private Message createMessageWithTopic() throws Exception {
         return message;
     }
 
+    private Message createMessageWithNullTopic() throws Exception {
+        Topic topic = mock(Topic.class);
+        when(topic.getTopicName()).thenReturn(null);
+        Message message = mock(Message.class);
+        when(message.getJMSDestination()).thenReturn(topic);
+        return message;
+    }
+
     private Message createMessageWithTempTopic() throws Exception {
         TemporaryTopic topic = mock(TemporaryTopic.class);
         when(topic.getTopicName()).thenReturn("micrometer.test.topic");