Correctly handle null listener in RedisMessageListenerContainer.remove(…).

Closes #3009
This commit is contained in:
sujl95(TheWing)
2024-09-30 01:30:27 +09:00
committed by Mark Paluch
parent 20ceb4cf05
commit b1453dabc0
2 changed files with 55 additions and 24 deletions

View File

@@ -101,6 +101,7 @@ import org.springframework.util.backoff.FixedBackOff;
* @author Thomas Darimont
* @author Mark Paluch
* @author John Blum
* @author SEONGJUN LEE
* @see MessageListener
* @see SubscriptionListener
*/
@@ -770,33 +771,35 @@ public class RedisMessageListenerContainer implements InitializingBean, Disposab
}
private void remove(@Nullable MessageListener listener, Topic topic, ByteArrayWrapper holder,
Map<ByteArrayWrapper, Collection<MessageListener>> mapping, List<byte[]> topicToRemove) {
Map<ByteArrayWrapper, Collection<MessageListener>> mapping, List<byte[]> topicToRemove) {
Collection<MessageListener> listeners = mapping.get(holder);
Collection<MessageListener> listenersToRemove = null;
if (listeners == null || listeners.isEmpty()) {
return;
}
if (listeners != null) {
// remove only one listener
listeners.remove(listener);
listenersToRemove = Collections.singletonList(listener);
Collection<MessageListener> listenersToRemove = (listener == null) ? new ArrayList<>(listeners)
: Collections.singletonList(listener);
// start removing listeners
for (MessageListener messageListener : listenersToRemove) {
Set<Topic> topics = listenerTopics.get(messageListener);
if (topics != null) {
topics.remove(topic);
}
if (CollectionUtils.isEmpty(topics)) {
listenerTopics.remove(messageListener);
}
// Remove the specified listener(s) from the original collection
listeners.removeAll(listenersToRemove);
// Start removing listeners
for (MessageListener messageListener : listenersToRemove) {
Set<Topic> topics = listenerTopics.get(messageListener);
if (topics != null) {
topics.remove(topic);
}
// if we removed everything, remove the empty holder collection
if (listeners.isEmpty()) {
mapping.remove(holder);
topicToRemove.add(holder.getArray());
if (CollectionUtils.isEmpty(topics)) {
listenerTopics.remove(messageListener);
}
}
// If all listeners were removed, clean up the mapping and the holder
if (listeners.isEmpty()) {
mapping.remove(holder);
topicToRemove.add(holder.getArray());
}
}
private Subscriber createSubscriber(RedisConnectionFactory connectionFactory, Executor executor) {

View File

@@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -30,10 +31,7 @@ import org.junit.jupiter.api.Test;
import org.springframework.core.task.SimpleAsyncTaskExecutor;
import org.springframework.core.task.SyncTaskExecutor;
import org.springframework.data.redis.RedisConnectionFailureException;
import org.springframework.data.redis.connection.RedisConnection;
import org.springframework.data.redis.connection.RedisConnectionFactory;
import org.springframework.data.redis.connection.Subscription;
import org.springframework.data.redis.connection.SubscriptionListener;
import org.springframework.data.redis.connection.*;
import org.springframework.data.redis.connection.jedis.JedisConnectionFactory;
import org.springframework.data.redis.listener.adapter.MessageListenerAdapter;
import org.springframework.data.redis.listener.adapter.RedisListenerExecutionFailedException;
@@ -221,4 +219,34 @@ class RedisMessageListenerContainerUnitTests {
void failsOnDuplicateInit() {
assertThatIllegalStateException().isThrownBy(() -> container.afterPropertiesSet());
}
@Test
void shouldRemoveSpecificListenerFromMappingAndListenerTopics() {
MessageListener listener1 = mock(MessageListener.class);
MessageListener listener2 = mock(MessageListener.class);
Topic topic = new ChannelTopic("topic1");
container.addMessageListener(listener1, Collections.singletonList(topic));
container.addMessageListener(listener2, Collections.singletonList(topic));
container.removeMessageListener(listener1, Collections.singletonList(topic));
container.addMessageListener(listener2, Collections.singletonList(topic));
verify(listener1, never()).onMessage(any(), any());
}
@Test
void shouldRemoveAllListenersWhenListenerIsNull() {
MessageListener listener1 = mock(MessageListener.class);
MessageListener listener2 = mock(MessageListener.class);
Topic topic = new ChannelTopic("topic1");
container.addMessageListener(listener1, Collections.singletonList(topic));
container.addMessageListener(listener2, Collections.singletonList(topic));
container.removeMessageListener(null, Collections.singletonList(topic));
verify(listener1, never()).onMessage(any(), any());
verify(listener2, never()).onMessage(any(), any());
}
}