Allow telling Spring to ignore an argument when identifying the type

[anton-johansson]

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

Line 863 in 28b92a7

if (method == null) {

Would it be possible to add support for telling Spring to ignore a parameter? We have our own Context class that we inject via a custom HandlerMethodArgumentResolver. We usually like to have this as the first parameter. But when we do so, MessagingMessageListenerAdapter incorrectly interprets this as the event payload type.

Some options:

• Allow us to configure a list of classes to ignore.
• Automatically ignore any class that can be resolved via a HandlerMethodArgumentResolver.

[artembilan]

Any chances that you can share some simple project to let us understand what is going on with your custom resolver?

[anton-johansson]

Any chances that you can share some simple project to let us understand what is going on with your custom resolver?

Thanks for the quick reply! Sadly, I cannot. :/ But a short description:

• I've set up a HandlerMethodArgumentResolver that resolves Context.class (which is an interface) by simply loading it from a thread local.
• I have created my own annotation called @Consumes.
• I have set up a bean post processor that finds these methods (BeanPostProcessor).
• I have a custom RecordInterceptor that extracts a header from the record and creates a Context based on that, and sets some thread local stuff, and logging context. It also clears it in #afterRecord.
• I'm creating my own bean of type MessageHandlerMethodFactory that adds my argument resolver (via #setCustomArgumentResolvers).
• Finally, I have a KafkaListenerConfigurer that takes all beans/methods found by the post processors and registers the MethodKafkaListenerEndpoint.

I guess my 2nd suggested option with automatically ignoring any class that can be resolved via HandlerMethodArgumentResolver won't really work, since the payload resolver itself is a HandlerMethodArgumentResolver if I recall the code correctly.

I didn't see any other neat way to tell the framework to exclude my class. At least not without overriding a lot of the native Spring code, and I reeeally don't want that. I want to rely on your amazing code as much as possible. 🤩

[artembilan]

OK. With the custom HandlerMethodArgumentResolver your @Configuration can implements KafkaListenerConfigurer.
There you do:

setCustomMethodArgumentResolvers(HandlerMethodArgumentResolver... methodArgumentResolvers)

And that should be enough for the DefaultMessageHandlerMethodFactory created by the KafkaListenerAnnotationBeanPostProcessor.
The logic in that DefaultMessageHandlerMethodFactory is like this:

	protected List<HandlerMethodArgumentResolver> initArgumentResolvers() {
		List<HandlerMethodArgumentResolver> resolvers = new ArrayList<>();
		ConfigurableBeanFactory beanFactory = (this.beanFactory instanceof ConfigurableBeanFactory cbf ?
				cbf : null);

		// Annotation-based argument resolution
		resolvers.add(new HeaderMethodArgumentResolver(this.conversionService, beanFactory));
		resolvers.add(new HeadersMethodArgumentResolver());

		// Type-based argument resolution
		resolvers.add(new MessageMethodArgumentResolver(this.messageConverter));

		if (this.customArgumentResolvers != null) {
			resolvers.addAll(this.customArgumentResolvers);
		}

		Assert.notNull(this.messageConverter, "MessageConverter not configured");
		resolvers.add(new PayloadMethodArgumentResolver(this.messageConverter, this.validator));

		return resolvers;
	}

So, your custom HandlerMethodArgumentResolver would be applied before PayloadMethodArgumentResolver and there should not be any conflicts as you mention with an attempt to resolve a payload into your Context argument.

[anton-johansson]

Yeah, but that's not the problem, I think. It's not about resolving arguments, it's about actually building the payload object.

When resolving the type (happens in MessagingMessageListenerAdapter#determineInferredType), it does not seem to care about argument resolvers. It goes through each parameter, one by one. The first one being Context, which should not be treated as the payload. However, the payload argument resolver (PayloadMethodArgumentResolver) matches against the first argument (Context) and simply chooses that one. When it has a type, it will try to convert the payload to that type, Context, which is not possible.

Oh, I forgot to mention an important detail. I also use this:

            factory.setRecordMessageConverter(new ByteArrayJsonMessageConverter());

This makes sure to convert the byte array to the correct type. However, it doesn't work of course, because it believes it should convert it to the Context class.

[artembilan]

The logic there in the MessagingMessageListenerAdapter#determineInferredType is like this:

if (!isNotConvertible && !isMessageWithNoTypeInfo(parameterType)
					&& (methodParameter.getParameterAnnotations().length == 0
					|| methodParameter.hasParameterAnnotation(Payload.class))) {
				if (genericParameterType == null) {
					genericParameterType = extractGenericParameterTypFromMethodParameter(methodParameter);
				}
				else {
					this.logger.debug(() -> "Ambiguous parameters for target payload for method " + method
							+ "; no inferred type available");
					break;
				}
			}

So, since your Context goes first and it comes without any annotation, it is contributing to that genericParameterType.
Or consider the POJO method signature to have really payload as a first one, or have some custom annotation on that Context param.
I would go the Context as last one since it is really custom and has nothing to do with Kafka infrastructure.

[anton-johansson]

I would go the Context as last one since it is really custom and has nothing to do with Kafka infrastructure.

I do hear you, but it's just a bit annoying. We keep Context context as the first parameter everywhere, and it makes it quite clean. It's a good convention IMO. I know it has nothing to do with the Kafka infrastructure, but it feels like a nice addition to allow configuring certain classes to simply skip?

[anton-johansson]

Another acceptable approach would be to allow this:

void consumeMyEvent(Context context, @Payload MyData data) { ... }

However, this does not work either, because it does not look forward for any parameter annotated with @Payload. I think many devs would expect annotating the desired payload with @Payload would be enough, regardless of the order of parameters?

[artembilan]

I think many devs would expect annotating the desired payload with @Payload would be enough, regardless of the order of parameters?
That's what I think, too.

So, if you have many params which might be as a payload candidate, then do specify a @Payload explictly to avoid ambiguity.
I think this is what we can achieve in the algorithm of the MessagingMessageListenerAdapter#determineInferredType we have discussed before.

If you find this as a good candidate, then we will schedule the fix.

Thanks

[anton-johansson]

I think that sounds good enough to me! It actually makes the code quite readable IMO.

A bit curious about the potential fix. How would you go on about it? Would you first need to loop through each parameter once, to look for a @Payload argument? And then loop again, if there is no @Payload argument, and use the current strategy?

[artembilan]

I think the algorithm will be simple: we would have extra variable for ‘@Payload` and its value has a precedence. The rest of it would remain the same, even ambiguity error.