Skip to content

Commit c1d29ab

Browse files
Mikhail Polivakhaartembilan
Mikhail Polivakha
authored andcommitted
GH-3732: Fix NPE in Mqttv5PahoMessageDrivenChA
Fixes #3732 The `Mqttv5PahoMessageDrivenChannelAdapter` unconditionally tries to (un)subscribe to/from topics when the `mqqtClient` might not be initialized yet. * Add `mqqtClient` initialization check before adding or removing topic. Re-align logic with the `MqttPahoMessageDrivenChannelAdapter` **Cherry-pick to `5.5.x`**
1 parent 69efbd3 commit c1d29ab

File tree

3 files changed

+43
-24
lines changed

3 files changed

+43
-24
lines changed

Diff for: spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/AbstractMqttMessageDrivenChannelAdapter.java

+4-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
1717
package org.springframework.integration.mqtt.inbound;
1818

1919
import java.util.LinkedHashSet;
20+
import java.util.Objects;
2021
import java.util.Set;
2122
import java.util.concurrent.locks.Lock;
2223
import java.util.concurrent.locks.ReentrantLock;
@@ -40,6 +41,7 @@
4041
* @author Gary Russell
4142
* @author Artem Bilan
4243
* @author Trung Pham
44+
* @author Mikhail Polivakha
4345
*
4446
* @since 4.0
4547
*
@@ -331,15 +333,7 @@ public boolean equals(Object obj) {
331333
return false;
332334
}
333335
Topic other = (Topic) obj;
334-
if (this.topic == null) {
335-
if (other.topic != null) {
336-
return false;
337-
}
338-
}
339-
else if (!this.topic.equals(other.topic)) {
340-
return false;
341-
}
342-
return true;
336+
return Objects.equals(this.topic, other.topic);
343337
}
344338

345339
@Override

Diff for: spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/Mqttv5PahoMessageDrivenChannelAdapter.java

+24-13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2021 the original author or authors.
2+
* Copyright 2021-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -65,6 +65,7 @@
6565
* See {@link #setPayloadType} for more information about type conversion.
6666
*
6767
* @author Artem Bilan
68+
* @author Mikhail Polivakha
6869
*
6970
* @since 5.5.5
7071
*
@@ -141,13 +142,15 @@ public void setHeaderMapper(HeaderMapper<MqttProperties> headerMapper) {
141142
@Override
142143
protected void onInit() {
143144
super.onInit();
144-
try {
145-
this.mqttClient = new MqttAsyncClient(getUrl(), getClientId(), this.persistence);
146-
this.mqttClient.setCallback(this);
147-
this.mqttClient.setManualAcks(isManualAcks());
148-
}
149-
catch (MqttException ex) {
150-
throw new BeanCreationException("Cannot create 'MqttAsyncClient' for: " + getComponentName(), ex);
145+
if (this.mqttClient == null) {
146+
try {
147+
this.mqttClient = new MqttAsyncClient(getUrl(), getClientId(), this.persistence);
148+
this.mqttClient.setCallback(this);
149+
this.mqttClient.setManualAcks(isManualAcks());
150+
}
151+
catch (MqttException ex) {
152+
throw new BeanCreationException("Cannot create 'MqttAsyncClient' for: " + getComponentName(), ex);
153+
}
151154
}
152155
if (this.messageConverter == null) {
153156
setMessageConverter(getBeanFactory()
@@ -189,8 +192,10 @@ protected void doStop() {
189192
this.topicLock.lock();
190193
String[] topics = getTopic();
191194
try {
192-
this.mqttClient.unsubscribe(topics).waitForCompletion(getCompletionTimeout());
193-
this.mqttClient.disconnect().waitForCompletion(getCompletionTimeout());
195+
if (this.mqttClient != null && this.mqttClient.isConnected()) {
196+
this.mqttClient.unsubscribe(topics).waitForCompletion(getCompletionTimeout());
197+
this.mqttClient.disconnect().waitForCompletion(getCompletionTimeout());
198+
}
194199
}
195200
catch (MqttException ex) {
196201
logger.error(ex, () -> "Error unsubscribing from " + Arrays.toString(topics));
@@ -204,7 +209,9 @@ protected void doStop() {
204209
public void destroy() {
205210
super.destroy();
206211
try {
207-
this.mqttClient.close(true);
212+
if (this.mqttClient != null) {
213+
this.mqttClient.close(true);
214+
}
208215
}
209216
catch (MqttException ex) {
210217
logger.error(ex, "Failed to close 'MqttAsyncClient'");
@@ -215,8 +222,10 @@ public void destroy() {
215222
public void addTopic(String topic, int qos) {
216223
this.topicLock.lock();
217224
try {
218-
this.mqttClient.subscribe(topic, qos).waitForCompletion(getCompletionTimeout());
219225
super.addTopic(topic, qos);
226+
if (this.mqttClient != null && this.mqttClient.isConnected()) {
227+
this.mqttClient.subscribe(topic, qos).waitForCompletion(getCompletionTimeout());
228+
}
220229
}
221230
catch (MqttException ex) {
222231
throw new MessagingException("Failed to subscribe to topic " + topic, ex);
@@ -230,7 +239,9 @@ public void addTopic(String topic, int qos) {
230239
public void removeTopic(String... topic) {
231240
this.topicLock.lock();
232241
try {
233-
this.mqttClient.unsubscribe(topic).waitForCompletion(getCompletionTimeout());
242+
if (this.mqttClient != null && this.mqttClient.isConnected()) {
243+
this.mqttClient.unsubscribe(topic).waitForCompletion(getCompletionTimeout());
244+
}
234245
super.removeTopic(topic);
235246
}
236247
catch (MqttException ex) {

Diff for: spring-integration-mqtt/src/test/java/org/springframework/integration/mqtt/Mqttv5BackToBackTests.java

+15-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
2222
import java.util.ArrayList;
2323
import java.util.List;
2424

25+
import org.junit.jupiter.api.Assertions;
2526
import org.junit.jupiter.api.Test;
2627

2728
import org.springframework.beans.factory.annotation.Autowired;
@@ -55,6 +56,7 @@
5556
/**
5657
* @author Gary Russell
5758
* @author Artem Bilan
59+
* @author Mikhail Polivakha
5860
*
5961
* @since 5.5.5
6062
*
@@ -73,6 +75,18 @@ public class Mqttv5BackToBackTests implements MosquittoContainerTest {
7375
@Autowired
7476
private Config config;
7577

78+
@Test //GH-3732
79+
public void testNoNpeIsNotThrownInCaseDoInitIsNotInvokedBeforeTopicAddition() {
80+
Mqttv5PahoMessageDrivenChannelAdapter channelAdapter = new Mqttv5PahoMessageDrivenChannelAdapter("tcp://mock-url.com:8091", "mock-client-id", "123");
81+
Assertions.assertDoesNotThrow(() -> channelAdapter.addTopic("abc", 1));
82+
}
83+
84+
@Test //GH-3732
85+
public void testNoNpeIsNotThrownInCaseDoInitIsNotInvokedBeforeTopicRemoval() {
86+
Mqttv5PahoMessageDrivenChannelAdapter channelAdapter = new Mqttv5PahoMessageDrivenChannelAdapter("tcp://mock-url.com:8091", "mock-client-id", "123");
87+
Assertions.assertDoesNotThrow(() -> channelAdapter.removeTopic("abc"));
88+
}
89+
7690
@Test
7791
public void testSimpleMqttv5Interaction() {
7892
String testPayload = "foo";

0 commit comments

Comments
 (0)