Skip to content

Commit 8843b1a

Browse files
garyrussellartembilan
authored andcommitted
GH-1441: Fix Payload Detection with MessageHeaders
Resolves #1441 Previously, `MessageHeaders` had to be annotated with `@Headers` so that it was ignored during payload parameter resolution; otherwise it caused ambiguity. Ignore `MessageHeaders` even when not so annotated. Also fix some tests that were checking the same topic and count down latch so were unconditionally passing. Change one of those tests to verify the fix. **cherry-pick to 2.4.x, 2.3.x**
1 parent 519face commit 8843b1a

File tree

2 files changed

+23
-13
lines changed

2 files changed

+23
-13
lines changed

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

Lines changed: 4 additions & 2 deletions
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.
@@ -33,6 +33,7 @@
3333
import org.springframework.core.MethodParameter;
3434
import org.springframework.lang.Nullable;
3535
import org.springframework.messaging.Message;
36+
import org.springframework.messaging.MessageHeaders;
3637
import org.springframework.messaging.MessagingException;
3738
import org.springframework.messaging.handler.annotation.Header;
3839
import org.springframework.messaging.handler.annotation.Headers;
@@ -379,7 +380,8 @@ private Type determineInferredType() { // NOSONAR - complexity
379380
* We ignore parameters with type Message because they are not involved with conversion.
380381
*/
381382
boolean isHeaderOrHeaders = methodParameter.hasParameterAnnotation(Header.class)
382-
|| methodParameter.hasParameterAnnotation(Headers.class);
383+
|| methodParameter.hasParameterAnnotation(Headers.class)
384+
|| methodParameter.getParameterType().equals(MessageHeaders.class);
383385
boolean isPayload = methodParameter.hasParameterAnnotation(Payload.class);
384386
if (isHeaderOrHeaders && isPayload && MessagingMessageListenerAdapter.this.logger.isWarnEnabled()) {
385387
MessagingMessageListenerAdapter.this.logger.warn(this.method.getName()

spring-rabbit/src/test/java/org/springframework/amqp/rabbit/annotation/EnableRabbitIntegrationTests.java

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@
120120
import org.springframework.core.task.TaskExecutor;
121121
import org.springframework.data.web.JsonPath;
122122
import org.springframework.lang.NonNull;
123+
import org.springframework.messaging.MessageHeaders;
123124
import org.springframework.messaging.converter.GenericMessageConverter;
124125
import org.springframework.messaging.handler.annotation.Header;
125126
import org.springframework.messaging.handler.annotation.Payload;
@@ -560,8 +561,9 @@ public void testRabbitHandlerNoDefaultValidationCount() throws InterruptedExcept
560561
public void testDifferentTypes() throws InterruptedException {
561562
Foo1 foo = new Foo1();
562563
foo.setBar("bar");
564+
this.service.foos.clear();
563565
this.jsonRabbitTemplate.convertAndSend("differentTypes", foo);
564-
assertThat(this.service.latch.await(10, TimeUnit.SECONDS)).isTrue();
566+
assertThat(this.service.dtLatch1.await(10, TimeUnit.SECONDS)).isTrue();
565567
assertThat(this.service.foos.get(0)).isInstanceOf(Foo2.class);
566568
assertThat(((Foo2) this.service.foos.get(0)).getBar()).isEqualTo("bar");
567569
assertThat(TestUtils.getPropertyValue(this.registry.getListenerContainer("different"), "concurrentConsumers")).isEqualTo(2);
@@ -571,8 +573,9 @@ public void testDifferentTypes() throws InterruptedException {
571573
public void testDifferentTypesWithConcurrency() throws InterruptedException {
572574
Foo1 foo = new Foo1();
573575
foo.setBar("bar");
574-
this.jsonRabbitTemplate.convertAndSend("differentTypes", foo);
575-
assertThat(this.service.latch.await(10, TimeUnit.SECONDS)).isTrue();
576+
this.service.foos.clear();
577+
this.jsonRabbitTemplate.convertAndSend("differentTypes2", foo);
578+
assertThat(this.service.dtLatch2.await(10, TimeUnit.SECONDS)).isTrue();
576579
assertThat(this.service.foos.get(0)).isInstanceOf(Foo2.class);
577580
assertThat(((Foo2) this.service.foos.get(0)).getBar()).isEqualTo("bar");
578581
MessageListenerContainer container = this.registry.getListenerContainer("differentWithConcurrency");
@@ -584,8 +587,9 @@ public void testDifferentTypesWithConcurrency() throws InterruptedException {
584587
public void testDifferentTypesWithVariableConcurrency() throws InterruptedException {
585588
Foo1 foo = new Foo1();
586589
foo.setBar("bar");
587-
this.jsonRabbitTemplate.convertAndSend("differentTypes", foo);
588-
assertThat(this.service.latch.await(10, TimeUnit.SECONDS)).isTrue();
590+
this.service.foos.clear();
591+
this.jsonRabbitTemplate.convertAndSend("differentTypes3", foo);
592+
assertThat(this.service.dtLatch3.await(10, TimeUnit.SECONDS)).isTrue();
589593
assertThat(this.service.foos.get(0)).isInstanceOf(Foo2.class);
590594
assertThat(((Foo2) this.service.foos.get(0)).getBar()).isEqualTo("bar");
591595
MessageListenerContainer container = this.registry.getListenerContainer("differentWithVariableConcurrency");
@@ -1086,7 +1090,11 @@ public static class MyService {
10861090

10871091
final List<Object> foos = new ArrayList<>();
10881092

1089-
final CountDownLatch latch = new CountDownLatch(1);
1093+
final CountDownLatch dtLatch1 = new CountDownLatch(1);
1094+
1095+
final CountDownLatch dtLatch2 = new CountDownLatch(1);
1096+
1097+
final CountDownLatch dtLatch3 = new CountDownLatch(1);
10901098

10911099
final CountDownLatch validationLatch = new CountDownLatch(1);
10921100

@@ -1237,21 +1245,21 @@ public void handleIt(Date body) {
12371245
containerFactory = "jsonListenerContainerFactoryNoClassMapper")
12381246
public void handleDifferent(@Validated Foo2 foo) {
12391247
foos.add(foo);
1240-
latch.countDown();
1248+
dtLatch1.countDown();
12411249
}
12421250

12431251
@RabbitListener(id = "differentWithConcurrency", queues = "differentTypes2",
1244-
containerFactory = "jsonListenerContainerFactory", concurrency = "#{3}")
1245-
public void handleDifferentWithConcurrency(Foo2 foo) {
1252+
containerFactory = "jsonListenerContainerFactoryNoClassMapper", concurrency = "#{3}")
1253+
public void handleDifferentWithConcurrency(Foo2 foo, MessageHeaders headers) {
12461254
foos.add(foo);
1247-
latch.countDown();
1255+
dtLatch2.countDown();
12481256
}
12491257

12501258
@RabbitListener(id = "differentWithVariableConcurrency", queues = "differentTypes3",
12511259
containerFactory = "jsonListenerContainerFactory", concurrency = "3-4")
12521260
public void handleDifferentWithVariableConcurrency(Foo2 foo) {
12531261
foos.add(foo);
1254-
latch.countDown();
1262+
dtLatch3.countDown();
12551263
}
12561264

12571265
@RabbitListener(id = "notStarted", containerFactory = "rabbitAutoStartFalseListenerContainerFactory",

0 commit comments

Comments
 (0)