MethodParameter.equals is too coarse-grained for its use in HandlerMethodArgumentResolverComposite

[garyrussell]

	@Override
	public int hashCode() {
		return (getExecutable().hashCode() * 31 + this.parameterIndex);
	}

HandlerMethodArgumentResolverComposite uses MethodParameter as a cache key for HandlerMethodArgumentResolver.

Consider:

@FunctionalInterface
public interface GenericHandler<P> {

	Object handle(P payload, MessageHeaders headers);

}

		public static class MapHandler implements GenericHandler<Map<String, String>> {

			@Override
			public String handle(Map<String, String> mapPayload, MessageHeaders messageHeaders) {
				return "Hello " + mapPayload.get("key");
			}

		}

		public static class StringHandler implements GenericHandler<String> {

			@Override
			public String handle(String stringPayload, MessageHeaders messageHeaders) {
				return stringPayload + " World!";
			}

		}

MethodParameter.executable in both cases is the interface Method:

public abstract java.lang.Object org.springframework.integration.handler.GenericHandler.handle(java.lang.Object,org.springframework.messaging.MessageHeaders)

When invoking the second method, we get a cache hit and fail with a class cast exception because the wrong argument resolver is returned.

Spring Integration recently changed to use a shared DefaultMessageHandlerMethodFactory @Bean, and hence a shared HandlerMethodArgumentResolverComposite.

We can work around this by changing the bean scope to prototype, but I think the hashCode() and equals() should include at least the containingClass and/or parameterType field.

See spring-projects/spring-integration#3004

[jhoeller]

I've refined MethodParameter.equals to consider containingClass and nestingLevel as well, i.e. to the degree that it can differ between resolution attempts. I intend to backport this minimal change to 5.0.x and 4.3.x as well since it really is a bug, also next to DependencyDescriptor which has such an exhaustive equals implementation already (and delegates to MethodParameter.equals underneath). Beyond that, we intend to do a more extensive revision of MethodParameter for 5.2.

[jhoeller]

@garyrussell A minimal fix for this through a narrowed equals implementation is in all branches now. Could you give that one a try for your purposes, ideally against 5.1.9 snapshot?

[jhoeller]

Doing one more pass over this since the previous fix didn't take overridden getContainingClass() implementations into account...

[garyrussell]

Could you give that one a try for your purposes, ideally against 5.1.9 snapshot?

@jhoeller I already reverted the workaround on master yesterday and all is fine; thanks!

I am building our 5.1.x against 5.1.9 snapshots and don't see any problems there either (although we only had the problem on master, when we went to a shared handler factory across multiple components).

[jhoeller]

Cool, good to know :-) I've also taken overridden containing classes into account now, just in cases somebody uses those on MethodParameter instances that serve as a cache key.