Mutate existing default HttpMessageConverter instead of adding duplication

[quaff]

I found Spring Boot add two flavored HttpMessageConverters to default messageConverters

spring-boot/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/JacksonHttpMessageConvertersConfiguration.java

Lines 47 to 54 in 47516b5

	@Bean
	@ConditionalOnMissingBean(value = MappingJackson2HttpMessageConverter.class,
	ignoredType = {
	"org.springframework.hateoas.server.mvc.TypeConstrainedMappingJackson2HttpMessageConverter",
	"org.springframework.data.rest.webmvc.alps.AlpsJsonHttpMessageConverter" })
	MappingJackson2HttpMessageConverter mappingJackson2HttpMessageConverter(ObjectMapper objectMapper) {
	return new MappingJackson2HttpMessageConverter(objectMapper);
	}

spring-boot/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/HttpMessageConvertersAutoConfiguration.java

Lines 77 to 84 in 47516b5

	@Bean
	@ConditionalOnMissingBean
	public StringHttpMessageConverter stringHttpMessageConverter(Environment environment) {
	Encoding encoding = Binder.get(environment).bindOrCreate("server.servlet.encoding", Encoding.class);
	StringHttpMessageConverter converter = new StringHttpMessageConverter(encoding.getCharset());
	converter.setWriteAcceptCharset(false);
	return converter;
	}

Here is combined converters by HttpMessageConverters

1. org.springframework.http.converter.ByteArrayHttpMessageConverter@16b050d3
2. org.springframework.http.converter.StringHttpMessageConverter@23c51bca (inserted by Spring Boot, defaultCharset customized)
3. org.springframework.http.converter.StringHttpMessageConverter@27be770a
4. org.springframework.http.converter.ResourceHttpMessageConverter@7c7c3afd
5. org.springframework.http.converter.ResourceRegionHttpMessageConverter@65d583ec
6. org.springframework.http.converter.xml.SourceHttpMessageConverter@7177614d
7. org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter@25a54e4e
8. org.springframework.http.converter.json.MappingJackson2HttpMessageConverter@305bd1b8 (inserted by Spring Boot, objectMapper customized)
9. org.springframework.http.converter.json.MappingJackson2HttpMessageConverter@381323d5
10. org.springframework.http.converter.xml.Jaxb2RootElementHttpMessageConverter@71901dd8

there are two cons:

• duplication
• other AbstractJackson2HttpMessageConverters such as MappingJackson2CborHttpMessageConverter and MappingJackson2SmileHttpMessageConverter are not considered

[wilkinsona]

Duplicate of #21374 and the various other issues to which it links.

[quaff]

@wilkinsona I know it's intentional, but you are not considering other AbstractJackson2HttpMessageConverter not just MappingJackson2HttpMessageConverter, my point is why not call StringHttpMessageConverter::setDefaultCharset and AbstractJackson2HttpMessageConverter::setObjectMapper.

[wilkinsona]

Because it is intentional that there are multiple converters with different configurations. They may, for example, support different media types. I'm just repeating here what's already been said in #21374 and the issues to which it links. Please take the time to read those other issues in full.

[quaff]

I didn't find anything related to other AbstractJackson2HttpMessageConverter, If you insist on current solution, then other 3 AbstractJackson2HttpMessageConverter (smile cbor xml) should also be considered.

[wilkinsona]

In what way do you believe that the Smile, CBOR, and XML converters need to be considered?

[quaff]

In what way do you believe that the Smile, CBOR, and XML converters need to be considered?

Spring MVC add 4 default AbstractJackson2HttpMessageConverter, Spring Boot only override one, the other 3 converters still using default ObjectMapper.
more beans should added like:

 @Bean 
 @ConditionalOnMissingBean(value = MappingJackson2CborHttpMessageConverter.class) 
// other conditions
 MappingJackson2CborHttpMessageConverter mappingJackson2CborHttpMessageConverter(ObjectMapper objectMapper) { 
 	return new MappingJackson2CborHttpMessageConverter(objectMapper); 
 } 

[wilkinsona]

That's not the case. Spring Boot's auto-configured ObjectMapper is specifically for JSON. Using it for CBOR, Smile, and XML would lead to a failure as the object mapper or its factory will be of the wrong type.

[quaff]

Because it is intentional that there are multiple converters with different configurations. They may, for example, support different media types. I'm just repeating here what's already been said in #21374 and the issues to which it links. Please take the time to read those other issues in full.

I took some time to check boot flavored StringHttpMessageConverter and MappingJackson2HttpMessageConverter, they accept same media types as default converters, that means it's safe to remove those two duplicated default converters.

according to

spring-boot/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/http/HttpMessageConverters.java

Lines 129 to 136 in 47516b5

	private boolean isReplacement(HttpMessageConverter<?> defaultConverter, HttpMessageConverter<?> candidate) {
	for (Class<?> nonReplacingConverter : NON_REPLACING_CONVERTERS) {
	if (nonReplacingConverter.isInstance(candidate)) {
	return false;
	}
	}
	return ClassUtils.isAssignableValue(defaultConverter.getClass(), candidate);
	}

I suggest add some logic like this:

	private boolean isDuplication(HttpMessageConverter<?> defaultConverter, HttpMessageConverter<?> candidate) {
		if (defaultConverter.getClass() != candidate.getClass()) {
			return false;
		}
		// SAFE_REPLACING_CONVERTERS: [StringHttpMessageConverter, MappingJackson2HttpMessageConverter]
		return SAFE_REPLACING_CONVERTERS.contains(candidate.getClass());
	}

[wilkinsona]

As described in the javadoc of HttpMessageConverters and the various issues to which #21374 links, this is intentional. The javadoc states the following:

Items are added just before any default converter of the same type (or at the front of the list if no default converter is found).

Your proposal above suggests that it's safe for one MappingJackson2HttpMessageConverter to replace another but this isn't the case. They may have been configured with different supported media types and with different ObjectMappers. We don't want HttpMessageConverters to have to understand the matching logic in every HttpMessageConverter implementation so, instead, we deliberately place the converters picked up by auto-configuration ahead of the default converters.

If you have a specific real-world problem that this behaviour is causing then please let us know. Otherwise, your concern appears to be purely theoretical and isn't sufficient reason to increase the complexity of HttpMessageConverters.