Fix multiple HTTP message converters of the same type being lost

[tommyk-gears]

When multiple HttpMessageConverters of the same specialized type
(e.g., two JSON converters) were configured, only the last one was
registered correctly. The first converter would be discarded because
the specialized registration method was called for all converters
of the same type, causing each to overwrite the previous one.

This fix ensures that only the first converter of each specialized
type (String, Kotlin Serialization JSON, JSON, XML) is registered
using the specialized method (e.g., withJsonConverter()). Subsequent
converters of the same type are registered as custom converters via
addCustomConverter().

[wilkinsona]

The behaviour you have described is intentional. Converters can be ordered to control which converter wins. What's your use case for having multiple JSON-compatibile converters registered at the same time and what logic do they have in place to ensure that they'll both be used?

[tommyk-gears]

The behaviour you have described is intentional. Converters can be ordered to control which converter wins. What's your use case for having multiple JSON-compatibile converters registered at the same time and what logic do they have in place to ensure that they'll both be used?

I have for example both org.springframework.http.converter.protobuf.ProtobufHttpMessageConverter and org.springframework.http.converter.json.JacksonJsonHttpMessageConverter. The former supports json when protobuf-java-util is on classpath. In my case, the ProtobufHttpMessageConverter is first registered as jsonConverter (since it happens to support json), then the JacksonJsonHttpMessageConverter is also registered as jsonConverter, and in the end there is no trace of a ProtobufHttpMessageConverter in the resulting HttpMessageConverters.

[wilkinsona]

Your proposed fix would result in JacksonJsonHttpMessageConverter being registered as a custom converter. For JSON, it would then be used ahead of ProtobufHttpMessageConverter as custom converters take priority. That would be a breaking change for anyone who wants the Protobuf converter to be used and has com.google.protobuf.util.JsonFormat on the classpath for that very reason.

I think there's room for improvement here, but I don't think this is the right approach. Please open an issue with as much detail as possible about the problem that you're facing (a small sample the reproduces it would be ideal) and we can take a further look.

[bclozel]

I don't think we do anything special with ProtobufHttpMessageConverter in Spring Boot. Have you tried contributing that converter with a ServerHttpMessageConvertersCustomizer that sets ProtobufHttpMessageConverter as a custom one directly?

I think we're doing our best here with regards to the order of beans in the application context, but trying to sort out all possible combinations there and guessing the original intent won't work in my opinion.

[tommyk-gears]

Have you tried contributing that converter with a ServerHttpMessageConvertersCustomizer that sets ProtobufHttpMessageConverter as a custom one directly?

Yes that was my initial workaround when I found this issue. It works ofc. But I still believe the current behaviour in Default(Server|Client)HttpMessageConvertersCustomizer is incorrect.

The spring boot documentation clearly states;

Any HttpMessageConverter bean that is present in the context is added to the list of converters. You can also override default converters in the same way.

But that is not the case. The current behaviour is that e.g. all HttpMessageConverter beans that support json are ignored, except the last one.
What I ask for is simply to not ignore some HttpMessageConverter beans just because there happen to be another converter supporting the same mime type. It is at the core of the HttpMessageConverter interface that the combination of Class and MediaType is used to determine if it can do a given conversion, hence ignoring some converters simply based on MimeType is not correct in my opinion.

[bclozel]

Yes that was my initial workaround when I found this issue. It works ofc. But I still believe the current behaviour in Default(Server|Client)HttpMessageConvertersCustomizer is incorrect.

I'm not sure we should consider that as a workaround. We can probably improve the built-in customizers, but we should discuss that in a new issue with a minimal sample that demonstrates the problem.

The spring boot documentation clearly states;

Any HttpMessageConverter bean that is present in the context is added to the list of converters. You can also override default converters in the same way.

We can probably improve the documentation there as the Spring Boot 3.x behavior was wrong in several ways. For example, the same list of converters was configured for both clients and server. The ordering and set of converters could be then wrong.

What I ask for is simply to not ignore some HttpMessageConverter beans just because there happen to be another converter supporting the same mime type.

I think there is now a different mindset. I'm not sure the order of message converter beans in the application context should be necessarily 100% reflected in how HttpMessageConverters exposes them.

It is at the core of the HttpMessageConverter interface that the combination of Class and MediaType is used to determine if it can do a given conversion, hence ignoring some converters simply based on MimeType is not correct in my opinion.

I'm not denying that. At startup time, we can only consider the media type supported by the converter for ordering. Those are at best heuristics, so we'll have to refine without making things too surprising for developers. I think that the customizer approach provides more control and is probably preferrable to complex heuristics.