Skip to content

Commit d04a20a

Browse files
garyrussellartembilan
authored andcommitted
GH-1034: DMLC: Detect target channel changed
Fixes #1034 If the connection factory refreshed the target connection, the DMLC is not made aware of it and so we never consume from the new channel. **cherry-pick to all 2.x** # Conflicts: # spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/DirectMessageListenerContainerMockTests.java # Conflicts: # spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/DirectMessageListenerContainer.java # spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/DirectMessageListenerContainerMockTests.java
1 parent ba6875d commit d04a20a

File tree

2 files changed

+73
-3
lines changed

2 files changed

+73
-3
lines changed

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/DirectMessageListenerContainer.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.springframework.amqp.ImmediateAcknowledgeAmqpException;
4444
import org.springframework.amqp.core.Message;
4545
import org.springframework.amqp.core.MessageProperties;
46+
import org.springframework.amqp.rabbit.connection.ChannelProxy;
4647
import org.springframework.amqp.rabbit.connection.Connection;
4748
import org.springframework.amqp.rabbit.connection.ConnectionFactory;
4849
import org.springframework.amqp.rabbit.connection.ConnectionFactoryUtils;
@@ -387,7 +388,8 @@ protected void actualStart() throws Exception {
387388
synchronized (this.consumersMonitor) {
388389
consumersToCancel = this.consumers.stream()
389390
.filter(c -> {
390-
boolean open = c.getChannel().isOpen() && !c.isAckFailed();
391+
boolean open = c.getChannel().isOpen() && !c.isAckFailed()
392+
&& !c.targetChanged();
391393
if (open && this.messagesPerAck > 1) {
392394
try {
393395
c.ackIfNecessary(now);
@@ -774,6 +776,8 @@ final class SimpleConsumer extends DefaultConsumer {
774776

775777
private final long ackTimeout = DirectMessageListenerContainer.this.ackTimeout;
776778

779+
private final Channel targetChannel;
780+
777781
private int pendingAcks;
778782

779783
private long lastAck = System.currentTimeMillis();
@@ -797,6 +801,12 @@ private SimpleConsumer(Connection connection, Channel channel, String queue) {
797801
this.connection = connection;
798802
this.queue = queue;
799803
this.ackRequired = !getAcknowledgeMode().isAutoAck() && !getAcknowledgeMode().isManual();
804+
if (channel instanceof ChannelProxy) {
805+
this.targetChannel = ((ChannelProxy) channel).getTargetChannel();
806+
}
807+
else {
808+
this.targetChannel = null;
809+
}
800810
}
801811

802812
private String getQueue() {
@@ -833,6 +843,15 @@ boolean isAckFailed() {
833843
return this.ackFailed;
834844
}
835845

846+
/**
847+
* True if the channel is a proxy and the underlying channel has changed.
848+
* @return true if the condition exists.
849+
*/
850+
boolean targetChanged() {
851+
return this.targetChannel != null
852+
&& !((ChannelProxy) getChannel()).getTargetChannel().equals(this.targetChannel);
853+
}
854+
836855
/**
837856
* Increment and return the current epoch for this consumer; consumersMonitor must
838857
* be held.

spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/DirectMessageListenerContainerMockTests.java

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2017 the original author or authors.
2+
* Copyright 2017-2019 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.
@@ -43,6 +43,7 @@
4343
import org.junit.Test;
4444
import org.mockito.Mockito;
4545

46+
import org.springframework.amqp.core.AcknowledgeMode;
4647
import org.springframework.amqp.core.MessageListener;
4748
import org.springframework.amqp.rabbit.connection.ChannelProxy;
4849
import org.springframework.amqp.rabbit.connection.Connection;
@@ -175,7 +176,7 @@ else if (i.getArgument(0).equals(17L)) {
175176
}
176177
Thread.sleep(200);
177178
consumer.get().handleDelivery("consumerTag", envelope(16), props, body);
178-
// should get 2 acks #10 and #6 (timeout)
179+
// should get 2 acks #10 and #16 (timeout)
179180
assertTrue(latch2.await(10, TimeUnit.SECONDS));
180181
consumer.get().handleDelivery("consumerTag", envelope(17), props, body);
181182
verify(channel).basicAck(10L, true);
@@ -311,6 +312,56 @@ public void testMonitorCancelsAfterBadAckEvenIfChannelReportsOpen() throws Excep
311312
container.stop();
312313
}
313314

315+
@Test
316+
public void testMonitorCancelsAfterTargetChannelChanges() throws Exception {
317+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
318+
Connection connection = mock(Connection.class);
319+
ChannelProxy channel = mock(ChannelProxy.class);
320+
Channel rabbitChannel1 = mock(Channel.class);
321+
Channel rabbitChannel2 = mock(Channel.class);
322+
AtomicReference<Channel> target = new AtomicReference<>(rabbitChannel1);
323+
willAnswer(inv -> {
324+
return target.get();
325+
}).given(channel).getTargetChannel();
326+
327+
given(connectionFactory.createConnection()).willReturn(connection);
328+
given(connection.createChannel(anyBoolean())).willReturn(channel);
329+
given(channel.isOpen()).willReturn(true);
330+
given(channel.queueDeclarePassive(Mockito.anyString()))
331+
.willAnswer(invocation -> mock(AMQP.Queue.DeclareOk.class));
332+
AtomicReference<Consumer> consumer = new AtomicReference<>();
333+
final CountDownLatch latch1 = new CountDownLatch(1);
334+
final CountDownLatch latch2 = new CountDownLatch(1);
335+
willAnswer(inv -> {
336+
consumer.set(inv.getArgument(6));
337+
latch1.countDown();
338+
return "consumerTag";
339+
}).given(channel).basicConsume(anyString(), anyBoolean(), anyString(), anyBoolean(), anyBoolean(),
340+
anyMap(), any(Consumer.class));
341+
342+
willAnswer(inv -> {
343+
consumer.get().handleCancelOk("consumerTag");
344+
latch2.countDown();
345+
return null;
346+
}).given(channel).basicCancel("consumerTag");
347+
348+
DirectMessageListenerContainer container = new DirectMessageListenerContainer(connectionFactory);
349+
container.setQueueNames("test");
350+
container.setPrefetchCount(2);
351+
container.setMonitorInterval(100);
352+
container.setMessageListener(msg -> {
353+
target.set(rabbitChannel2);
354+
});
355+
container.setAcknowledgeMode(AcknowledgeMode.MANUAL);
356+
container.afterPropertiesSet();
357+
container.start();
358+
359+
assertTrue(latch1.await(10, TimeUnit.SECONDS));
360+
consumer.get().handleDelivery("consumerTag", envelope(1L), new BasicProperties(), new byte[1]);
361+
assertTrue(latch2.await(10, TimeUnit.SECONDS));
362+
container.stop();
363+
}
364+
314365
private Envelope envelope(long tag) {
315366
return new Envelope(tag, false, "", "");
316367
}

0 commit comments

Comments
 (0)