Determine if we should re-instate proxying of MultipartFile method params

[onobc]

In #3258 we removed the ability to proxy (for projection) method params of type MultipartFile. The purpose of this issue is to dig in deeper on that decision and decide if we should support this.

• understand the current root cause of the breakage of the proxy handler supporting this
• summarize findings and decide on whether to support this
• add support back in (or not) depending on outcome from above

[onobc]

Resolver resolution process

The ModelAttributeMethodProcessor first attempts to create the attribute representing the method param via its protected createAttribute method (default impl returns null).

When the createAttribute returns null then it will go through its constructAttribute method which does quite a bit of magic binding/wrapping/etc..

Note

The purpose of the createAttribute as stated by javadocs is..

Extension point to create the model attribute if not found in the model, with subsequent parameter binding through bean properties (unless suppressed).
By default, as of 6.1 this method returns null in which case org.springframework.validation.DataBinder.construct is used instead to create the model attribute. The main purpose of this method then is to allow to create the model attribute in some other, alternative way.

So... our ProxyingHandlerMethodArgumentResolver overrides createAttribute returning a projection proxy (not null) and therefore the parent constructAttribute method is not consulted. This results in the failure we are seeing. I have yet to dig in deeper to understand what the proxy resolver createAttribute is missing from the parent resolver constructAttribute. I don't think it matters though (see below).

Proxy resolver timeline

Prior to spring-data-commons 3.3.4, the "supports" method of the proxy resolver did the following:

spring-data-commons/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java

Lines 90 to 97 in 5bb0c4a

	if (parameter.getParameterAnnotation(ProjectedPayload.class) != null) {
	return true;
	}
	// Annotated type
	if (AnnotatedElementUtils.findMergedAnnotation(type, ProjectedPayload.class) != null) {
	return true;
	}

In 3.3.4 it was updated as follows:

spring-data-commons/src/main/java/org/springframework/data/web/ProxyingHandlerMethodArgumentResolver.java

Lines 92 to 100 in 0b53d93

	if (parameter.getParameterAnnotation(ProjectedPayload.class) != null
	|| parameter.getParameterAnnotation(ModelAttribute.class) != null) {
	return true;
	}
	// Annotated type
	if (AnnotatedElementUtils.findMergedAnnotation(type, ProjectedPayload.class) != null) {
	return true;
	}

Note

Because it was not possible to annotate a method param w/ @ ProjectedPayload until version 3.5.0 (only class level types), the first conditional in former code snippet above would never have fired. Also, since MultipartFile is not annotated w/ @ ProjectedPayload at the type level, the second conditional in former code snippet above would never have fired. IOW - prior to 3.3.4 MultipartFile was never supported by the proxy resolver.

However, after the change made in latter code snippet above (3.3.4) the proxy resolver started saying "Yep, I can handle a multipart file method param if it is annotated with @ ModelAttribute" (but it lied).

Suggestion

I suggest we leave this fix as-is based on the following:

1. This truly never worked (see previous section)

2. There is quite a bit of binding code in the parent resolver constructAttribute that we would have to repeat in the proxy resolver createAttribute and I am not confident that code would make the same sense in the proxy resolver as it does in the parent resolver.

If users want multipart file projection support they can create a feature request.

[onobc]

cc: @mp911de

[mp911de]

If users want multipart file projection support they can create a feature request.

I fully agree with that sentiment. Let's close this ticket and put it aside for the time being.

[onobc]

Closing based on above summary and suggestions.