The builder class has two types of methods. There are a number that have override semantics:
jsonMessageConverter(HttpMessageConverter<?>)stringMessageConverter(HttpMessageConverter<?>)xmlMessageConverter(HttpMessageConverter<?>)- etc
And one that is additive:
customMessageConverter(HttpMessageConverter<?>)
It's not clear from the method names that they have these different semantics. A contributing factor here is that the HttpMessageConverter contract doesn't use just the format (JSON, XML, etc) of the message, it also considers the type that's being read or written. That would suggest that it should be possible to register two JSON message converters that handle different types but doing so using jsonMessageConverter isn't possible and one or both of them must be registered using customMessageConverter instead.
After some discussion, @rstoyanchev suggested renaming the methods to be named set… for the overriding case and add… for the additive case. We also considered dropping Message for brevity:
setJsonConverter(HttpMessageConverter<?>)setStringConverter(HttpMessageConverter<?>)setXmlConverter(HttpMessageConverter<?>)addCustomConverter(HttpMessageConverter<?>)
Comment From: rstoyanchev
In CodecConfigurer, the specific codec type methods are used exclusively to customize (i.e. replace) default registrations. They play no role when defaults are switched off. That means either it's only custom codecs when defaults are off, or it's a list of custom codecs before default ones (possibly customized) when defaults are on.
HttpMessageConverters mirrors the override semantics for specific message converter methods, but they are used not only to replace defaults, but also when defaults are off. This is where it gets confusing because when defaults are off, it becomes unclear a) whether to add a specific type of converter through its dedicated method, or as a custom converter, and b) why the specific type of converter have override semantics.
I think it would be more clear for the specific types of methods to be clearly labeled for replacing default registrations only when default registrations are on. That explains their override semantics.
Or when defaults are off, then it would be down to a simple list of custom converters. There is no benefit I can see to use specific converter type method when defaults are off vs registering it as a custom converter and participating in the same additive list. The override semantics also suffers from additional issues as mentioned by Andy that it isn't just about the media type and you can legitimately support JSON through multiple message converters.
Comment From: bclozel
Thanks both, I think we're in a better place now.
I have applied the following changes.
The builder methods are renamed to withJsonConverter and variants, to better convey the fact that those are replacing the default converter for a given format. I didn't go with setJsonConverter because it felt inconsistent with other builder APIs. The customMessageConverter is renamed to addCustomConverter to better reflect the additive aspect.
We could consider updating the API to be even closer to the reactive variants, but nesting must of the builder API under a defaultConverters() sub-DSL seems a bit too much at this point.
As suggested by Rossen, the withJsonConverter and others are now only effective if the default registration of auto-detected converters is requested.