Question: Limitations of argument type inferral from target method signature

[KaiStapel]

This is just a question: In #390 you created the mechanism for inferring the argument type from the target method signature. You introduced the following limitation:

Methods with a single unannotated parameter or one with a @payload annotation
are considered for this feature.

What is the reasoning for limiting the feature for "single unannotated parameter" methods as opposed to simply "single parameter" methods?

I constantly run into the problem of breaking my listeners when I add a @Valid annotation to the input parameter. Then I have to analyze for hours what the problem is until I find the solution of also having to add a @Payload annotation to make it work again.

This is why I came to this question. Is this limitation really necessary? If so, why?

[artembilan]

Would you mind to show the code produces the problem for you?

I'm not sure what drove you to use @Valid since @RabbitListener doesn't do any validation...

The reason for the limitation is really about a multi-parameter methods when we can't decide which one is intended for payload injection.

[KaiStapel]

Env: Spring Boot 2.3.4.RELEASE with spring-boot-starter-amqp

Before (working):

  @RabbitListener(queues = "some-queue")
  public void listen(SomeDto someDto) {
    [...]
  }

Then (broken):

  @RabbitListener(queues = "some-queue")
  public void listen(@Valid SomeDto someDto) {
    [...]
  }

Finally (working again):

  @RabbitListener(queues = "some-queue")
  public void listen(@Valid @Payload SomeDto someDto) {
    [...]
  }

The @Valid annotation triggers Dto validation. I believe this is outside of @RabbitListener and has nothing to do with this question besides being an annotation that breakes the documented limitation "with a single unannotated parameter".

The reason for the limitation is really about a multi-parameter methods when we can't decide which one is intended for payload injection.

Exactly, if the reason is about multi-parameter methods, then why are you introducing the requirement for "no annotation" for single parameter methods? This is exactly my question.

[artembilan]

Well, we probably didn't think about any other possible annotation on the single parameter.
That's why my question to you: why do you use that @Valid annotation since it doesn't trigger anything when this method is called by the Listener Container? How that DTO validation is triggered? Are you sure that you can confirm the validation is working for you at this point?

We indeed going to look into the solution and take away that limitation, so you still can use @Valid without @Payload.

[KaiStapel]

That would be great.

I am pretty sure this annotation works, but I can only guess how exactly. Probably some AOP magic that uses the annotation to wrap the class.

There are even tutorials out there that are suggesting the double annotation. They probably all have gone through the same route:

• https://blog.trifork.com/2016/02/29/spring-amqp-payload-validation/
• https://stackoverflow.com/a/47797222/598117

The second link suggests that there is validation within the Spring Messaging framework: See DefaultMessageHandlerMethodFactory#setValidator()

[artembilan]

Ah, right! I forgot that @RabbitListener logic is based on the DefaultMessageHandlerMethodFactory.
Thanks for nailing this concern!
Yes, I think we will revise the logic for skipping a @Payload annotation requirement when the param is single.

If you have an idea what and where to do already, feel free to contribute the fix!

[KaiStapel]

@artembilan Please have a look at my proposal #1270. I did not have time to change/extend tests though. Sorry.

[garyrussell]

FYI, we have test cases at the message handler level, but not for the type inference:

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

Lines 564 to 566 in 60b7a58

	public void validatePayload(@Validated String payload) {
	invocations.put("validatePayload", true);
	}

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

Lines 336 to 363 in 60b7a58

	@Test
	public void validatePayloadValid() throws Exception {
	String methodName = "validatePayload";
	DefaultMessageHandlerMethodFactory customFactory = new DefaultMessageHandlerMethodFactory();
	customFactory.setValidator(testValidator("invalid value"));
	initializeFactory(customFactory);
	Method method = getListenerMethod(methodName, String.class);
	MessagingMessageListenerAdapter listener = createInstance(customFactory, method);
	Channel channel = mock(Channel.class);
	listener.onMessage(MessageTestUtils.createTextMessage("test"), channel); // test is a valid value
	assertListenerMethodInvocation(sample, methodName);
	}
	@Test
	public void validatePayloadInvalid() {
	DefaultMessageHandlerMethodFactory customFactory = new DefaultMessageHandlerMethodFactory();
	customFactory.setValidator(testValidator("invalid value"));
	Method method = getListenerMethod("validatePayload", String.class);
	MessagingMessageListenerAdapter listener = createInstance(customFactory, method);
	Channel channel = mock(Channel.class);
	assertThatThrownBy(() -> listener.onMessage(MessageTestUtils.createTextMessage("invalid value"), channel))
	.isInstanceOf(ListenerExecutionFailedException.class);
	}

[garyrussell]

I found a regression caused by this #1215 while testing this; so I will take this over.

[garyrussell]

Regression was a false alarm. New PR issued.