A flag to retain default converters when user has customized messageconverters for HttpRequestHandlingController, HttpInboundGateway [INT-3142]

[spring-operator]

Biju Kunjummen opened INT-3142 and commented

This is just a proposal for a small improvement: The HttpRequestHandlingController and the inbound gateway components wipe out the default converters if user has customized the converters. The change in behavior will be to retain the default converters through a flag. This will bring it a little more in line with the behavior in Spring MVC also.

---

Affects: 3.0 M3

Issue Links:

• Add support for full RESTFul request-mappings to the HTTP inbound adapters [INT-2312] #6250 Add support for full RESTFul request-mappings to the HTTP inbound adapters
  ("is depended on by")
• Set the "writeAcceptCharset" to false by default in StringHttpConverter within the http inbound gateway [INT-3120] #7097 Set the "writeAcceptCharset" to false by default in StringHttpConverter within the http inbound gateway

[spring-operator]

Artem Bilan commented

Thanks, Biju Kunjummen!
Can you point me out where something similar is presented in the MVC?
We have a message-converters attribute, so end-user can always inject his own <util:list> bean with all desired converters.
However in case we allow retain-default-converters, we should decide how the behaviour should be, if end-user wants override some default converter, but retain others.

[spring-operator]

Biju Kunjummen commented

Hi Artem, here is an example with Spring MVC and messageconverter customization:

<mvc:annotation-driven conversion-service="conversionService"> 
   <mvc:message-converters register-defaults="true">
       <bean class="org.springframework.http.converter.json.MappingJackson2HttpMessageConverter">
           <property name="objectMapper">
               <bean class="custom.CustomObjectMapper"/>
           </property>
       </bean>
   </mvc:message-converters>
</mvc:annotation-driven>

Internally this is handled by org.springframework.web.servlet.config.AnnotationDrivenBeanDefinitionParser

[spring-operator]

Artem Bilan commented

OK, thanks
I see the case.
Makes sence.
To do it in the SI we need Mark Fisher or Gary Russell approval.
What is interest, that this WebMvcConfigurationSupport#getMessageConverters and this RequestMappingHandlerAdapter (ExceptionHandlerExceptionResolver) don't follow with this concept - incoherence, even when AnnotationDrivenBeanDefinitionParser does the stuff for them. However only if we use configuration as you show.
So, IMO I won't mind to add new attribute register-default-converters true/false, who will make sence only, if we add message-converters, similar to AnnotationDrivenBeanDefinitionParser#getMessageConverters

[spring-operator]

Gary Russell commented

This is related to #7097, in that if (for some reason) someone wanted to set StringHttpConverter. writeAcceptCharset to 'true' (after 3120 is implemented), they would have to (currently) rebuild the entire default set. Perhaps with this change, if we detect an override of a default converter, we should just replace it in the list ??

[spring-operator]

Artem Bilan commented

Welcome back, Gary!
Will it be OK, if we do it as it is in the AnnotationDrivenBeanDefinitionParser#getMessageConverters? Place default converters in the end of the result list.

[spring-operator]

Gary Russell commented

Yes, I think that's perfectly fine, thanks.

[spring-operator]

Artem Bilan commented

So, Biju Kunjummen, go ahead and do the PR.
Thank you for your contribution!

[spring-operator]

Artem Bilan commented

PR: #881

[spring-operator]

Gary Russell commented

Merged