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.
Comment From: filiphr
I just stumbled onto this one during the upgrade from Spring Boot 4 RC2 to Spring Boot 4.
From the naming perspective it was not clear to me that withJsonConverter and the likes will not work if registerDefaults is set to false. It is not something that I would expect on a builder that offers those methods.
I would suggest that there should be some kind of validation in the builder to check that if registerDefaults is set to false and the default converters are set then there should be some kind of exception thrown. Otherwise, you get unexpected things happening later when the message conversion is happening and you have no easy way of realizing that you are receiving UNSUPPORTED_MEDIA_TYPE because your json converter did not get registered.
Or the alternative should be that if someone sets the additional converters they should be used. I don't see why registerDefaults should have an impact on whether or not something that I explicitly set on the builder gets registered on not. The registerDefaults to me is only whether or not they should be automatically detected.
I did check CodecConfigurer and there it is slightly different, it is not a fluent API like here and it has mainly 3 methods
DefaultCodecs defaultCodecs()CustomCodecs customCodecs()void registerDefaults(boolean registerDefaults)
i.e. you cannot get to the jacksonJsonDecoder without going through the DefaultCodecs. I do agree that this would be a bit too much and I only wanted to provide my experience with the API.