RestClient.Builder.configureMessageConverters disables the implicit registration of default converters.

[Rebwon]

In Spring Framework 7, I noticed that calling RestClient.Builder.configureMessageConverters(...) changes how default message converters are registered.

In DefaultRestClientBuilder.initMessageConverters(), defaults are registered only when both messageConverters and convertersConfigurer are null:

private List<HttpMessageConverter<?>> initMessageConverters() {
    HttpMessageConverters.ClientBuilder builder = HttpMessageConverters.forClient();
    if (this.messageConverters == null && this.convertersConfigurer == null) {
        builder.registerDefaults();
    }
    else {
        if (this.messageConverters != null) {
            this.messageConverters.forEach(builder::addCustomConverter);
        }
        if (this.convertersConfigurer != null) {
            this.convertersConfigurer.accept(builder);
        }
    }
    List<HttpMessageConverter<?>> result = new ArrayList<>();
    builder.build().forEach(result::add);
    return result;
}

This means that as soon as configureMessageConverters(...) is called, the implicit registerDefaults() path is skipped. For example:

RestClient client = RestClient.builder()
    .configureMessageConverters(builder -> {
        builder.withJsonConverter(new JacksonJsonHttpMessageConverter(jsonMapper));
    })
    .build();

My initial expectation was that this would keep the default converters and replace/configure the JSON converter. However, since registerDefaults() is not called, the configured JSON converter may not be included in the final converter list unless the user explicitly calls registerDefaults():

RestClient client = RestClient.builder()
    .configureMessageConverters(builder -> {
        builder.registerDefaults();
        builder.withJsonConverter(new JacksonJsonHttpMessageConverter(jsonMapper));
    })
    .build();

Is this the intended behavior?

The part that feels surprising is that the method name configureMessageConverters(...) can be interpreted as “configure the message converters used by the RestClient”, possibly starting from the default converter set. In practice, it appears to mean “fully configure the HttpMessageConverters.ClientBuilder, including deciding whether defaults should be registered”.

I understand that there are valid use cases where users may want to call disableDefaults() or provide a fully custom converter configuration. However, I am wondering why the implicit default registration is disabled merely by the presence of convertersConfigurer, rather than allowing the configurer to operate on the default converter setup by default.

More specifically, I would like to understand the design rationale behind this condition:

if (this.messageConverters == null && this.convertersConfigurer == null) {
    builder.registerDefaults();
}

Would it be possible or desirable for configureMessageConverters(...) to start with defaults unless the user explicitly calls disableDefaults()? Or is the intended contract that any use of configureMessageConverters(...) requires users to explicitly call registerDefaults() when they want to retain the default converters?

This is especially relevant when migrating from the previous list-based messageConverters(...) customization style, where users could modify an already initialized converter list. With the new HttpMessageConverters.ClientBuilder based API, it is easy to accidentally configure a JSON converter while ending up with no default converters at all.

If this is intended, it may be helpful to clarify in the Javadoc that configureMessageConverters(...) disables implicit default registration and that callers should invoke registerDefaults() explicitly when they want to customize the default converter set.

[RockyMM]

I think it's solved in 7.0.7

[Rebwon]

@bclozel

With this commit, it is now possible to customize configureMessageConverters. However, it seems that the use case of mutating an existing RestClient into a Builder and then reconfiguring already-registered HttpMessageConverters has not been fully considered.

For example, in the following case, after mutating the builder, it is possible to modify the previously registered converters via configureMessageConvertersList:

RestClient restClient = builder.build();
RestClient.Builder builder = restClient.mutate()
    .configureMessageConverters(configurer -> {
        configurer.withJsonConverter(new JacksonJsonHttpMessageConverter(JsonMapperFactory.create()));
    });

RestClient restClient1 = builder.build();

RestClient.Builder builder1 = restClient1.mutate();
builder1.configureMessageConverters(configurer -> {
    configurer.configureMessageConvertersList(listConfigurer -> {
        ListIterator<HttpMessageConverter<?>> iterator = listConfigurer.listIterator();

        while (iterator.hasNext()) {
            HttpMessageConverter<?> converter = iterator.next();
            if (converter instanceof JacksonJsonHttpMessageConverter httpMessageConverter) {
                JsonMapper jsonMapper = httpMessageConverter.getMapper()
                    .rebuild()
                    .changeDefaultPropertyInclusion(it -> it.withContentInclusion(JsonInclude.Include.ALWAYS))
                    .changeDefaultPropertyInclusion(it -> it.withValueInclusion(JsonInclude.Include.ALWAYS))
                    .build();
                iterator.set(new JacksonJsonHttpMessageConverter(jsonMapper));
            }
        }
    });
});

However, if no prior configuration is applied and only configureMessageConvertersList is used, the default converters are still not registered. As a result, it is not possible to modify the default configuration:

RestClient restClient = RestClient.builder().build();

RestClient.Builder defaultBuilder = restClient.mutate();
defaultBuilder.configureMessageConverters(configurer -> {
    configurer.configureMessageConvertersList(listConfigurer -> {
        ListIterator<HttpMessageConverter<?>> iterator = listConfigurer.listIterator();

        while (iterator.hasNext()) {
            HttpMessageConverter<?> converter = iterator.next();
            if (converter instanceof JacksonJsonHttpMessageConverter httpMessageConverter) {
                JsonMapper jsonMapper = httpMessageConverter.getMapper()
                    .rebuild()
                    .changeDefaultPropertyInclusion(it -> it.withContentInclusion(JsonInclude.Include.ALWAYS))
                    .changeDefaultPropertyInclusion(it -> it.withValueInclusion(JsonInclude.Include.ALWAYS))
                    .build();
                iterator.set(new JacksonJsonHttpMessageConverter(jsonMapper));
            }
        }
    });
});

Given this, I have a couple of questions:

• Are there any plans to provide an extension API on RestClient.Builder similar to configureMessageConvertersList that allows modifying the existing list more directly?
• Alternatively, would it be possible to allow access to the default initialized message converter list (similar to the previous initMessageConverters) when constructing a RestClient.Builder, so that users can modify the default configuration?

[bclozel]

@Rebwon I think what you're describing here is the expected behavior, isn't it? configureMessageConvertersList is letting you update the resulting list. At this point, the in-flight configuration information of the with* methods are lost.

[Rebwon]

It’s clear that configureMessageConvertersList is intended to modify the final list that gets created. However, when you call mutate() on a RestClient to obtain a builder and then try to modify it using configureMessageConverters provided by RestClient.Builder, the builder does not actually hold a reference to the messageConverters themselves. Instead, it only keeps a reference to a Consumer<HttpMessageConverters.ClientBuilder> called convertersConfigurer.

As a result, if you run the second code example above, no messageConverters end up being registered at all.

Am I misunderstanding something here?

[bclozel]

Let me experiment and get back to you. This report looks really confusing to me right now because the two parts are not strictly related.