From e11cb2d85658d49f3179ffd7ab919c11672401fc Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 7 Oct 2025 14:36:39 +0100 Subject: [PATCH] Improve converter config in RestClient.Builder Internally maintain a chain of HttpMessageConverters.ClientBuilder consumers in addition to the List of converters. List based methods apply to the list. HttpMessageConverters based methods are composed into a Consumer. At build() time prepare a single HttpMessageConverters.ClientBuilder. Insert list based converters first. Apply HttpMessageConverters consumers after that. Deprecate both List methods. Eventually, HttpMessageConverters should be the main mechanism. In the mean time we layer them as described. Closes gh-35578 --- .../web/client/DefaultRestClientBuilder.java | 64 +++++++++++-------- .../web/client/RestClient.java | 38 +++++++---- .../web/client/RestClientBuilderTests.java | 7 +- 3 files changed, 69 insertions(+), 40 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/client/DefaultRestClientBuilder.java b/spring-web/src/main/java/org/springframework/web/client/DefaultRestClientBuilder.java index c36cc83e8cc..1074838a637 100644 --- a/spring-web/src/main/java/org/springframework/web/client/DefaultRestClientBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/client/DefaultRestClientBuilder.java @@ -19,7 +19,6 @@ package org.springframework.web.client; import java.net.URI; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -111,6 +110,8 @@ final class DefaultRestClientBuilder implements RestClient.Builder { private @Nullable List> messageConverters; + private @Nullable Consumer convertersConfigurer; + private ObservationRegistry observationRegistry = ObservationRegistry.NOOP; private @Nullable ClientRequestObservationConvention observationConvention; @@ -142,6 +143,7 @@ final class DefaultRestClientBuilder implements RestClient.Builder { this.initializers = (other.initializers != null) ? new ArrayList<>(other.initializers) : null; this.requestFactory = other.requestFactory; this.messageConverters = (other.messageConverters != null ? new ArrayList<>(other.messageConverters) : null); + this.convertersConfigurer = other.convertersConfigurer; this.observationRegistry = other.observationRegistry; this.observationConvention = other.observationConvention; } @@ -363,25 +365,30 @@ final class DefaultRestClientBuilder implements RestClient.Builder { @Override @SuppressWarnings("removal") public RestClient.Builder messageConverters(Consumer>> configurer) { - configurer.accept(initMessageConverters()); + if (this.messageConverters == null) { + this.messageConverters = new ArrayList<>(); + HttpMessageConverters.forClient().registerDefaults().build().forEach(this.messageConverters::add); + } + configurer.accept(this.messageConverters); validateConverters(this.messageConverters); return this; } @Override + @SuppressWarnings("removal") public RestClient.Builder messageConverters(Iterable> messageConverters) { validateConverters(messageConverters); List> converters = new ArrayList<>(); - messageConverters.forEach(converter -> converters.add(converter)); - this.messageConverters = Collections.unmodifiableList(converters); + messageConverters.forEach(converters::add); + this.messageConverters = converters; return this; } @Override public RestClient.Builder configureMessageConverters(Consumer configurer) { - HttpMessageConverters.ClientBuilder clientBuilder = HttpMessageConverters.forClient(); - configurer.accept(clientBuilder); - return messageConverters(clientBuilder.build()); + this.convertersConfigurer = (this.convertersConfigurer != null ? + this.convertersConfigurer.andThen(configurer) : configurer); + return this; } @Override @@ -403,20 +410,11 @@ final class DefaultRestClientBuilder implements RestClient.Builder { return this; } - @SuppressWarnings("removal") - private List> initMessageConverters() { - if (this.messageConverters == null) { - this.messageConverters = new ArrayList<>(); - HttpMessageConverters.forClient().registerDefaults().build().forEach(this.messageConverters::add); - } - return this.messageConverters; - } - - private void validateConverters(@Nullable Iterable> messageConverters) { - Assert.notNull(messageConverters, "At least one HttpMessageConverter is required"); - Assert.isTrue(messageConverters.iterator().hasNext(), "At least one HttpMessageConverter is required"); - messageConverters.forEach(converter -> - Assert.notNull(converter, "The HttpMessageConverter list must not contain null elements")); + private void validateConverters(@Nullable Iterable> converters) { + Assert.notNull(converters, "At least one HttpMessageConverter is required"); + Assert.isTrue(converters.iterator().hasNext(), "At least one HttpMessageConverter is required"); + converters.forEach(converter -> Assert.notNull(converter, + "The HttpMessageConverter list must not contain null elements")); } @@ -427,14 +425,12 @@ final class DefaultRestClientBuilder implements RestClient.Builder { @Override public RestClient build() { + ClientHttpRequestFactory requestFactory = initRequestFactory(); UriBuilderFactory uriBuilderFactory = initUriBuilderFactory(); - HttpHeaders defaultHeaders = copyDefaultHeaders(); MultiValueMap defaultCookies = copyDefaultCookies(); - - List> converters = - (this.messageConverters != null ? this.messageConverters : initMessageConverters()); + List> converters = initMessageConverters(); return new DefaultRestClient( requestFactory, this.interceptors, this.bufferingPredicate, this.initializers, @@ -495,4 +491,22 @@ final class DefaultRestClientBuilder implements RestClient.Builder { return CollectionUtils.unmodifiableMultiValueMap(copy); } + private List> initMessageConverters() { + HttpMessageConverters.ClientBuilder builder = HttpMessageConverters.forClient(); + if (this.messageConverters == null && this.convertersConfigurer == null) { + builder.registerDefaults(); + } + else { + if (this.messageConverters != null) { + this.messageConverters.forEach(builder::customMessageConverter); + } + if (this.convertersConfigurer != null) { + this.convertersConfigurer.accept(builder); + } + } + List> result = new ArrayList<>(); + builder.build().forEach(result::add); + return result; + } + } diff --git a/spring-web/src/main/java/org/springframework/web/client/RestClient.java b/spring-web/src/main/java/org/springframework/web/client/RestClient.java index 61652c6cf29..ca7c257de57 100644 --- a/spring-web/src/main/java/org/springframework/web/client/RestClient.java +++ b/spring-web/src/main/java/org/springframework/web/client/RestClient.java @@ -449,31 +449,43 @@ public interface RestClient { /** * Configure the message converters for the {@code RestClient} to use. - * @param configurer the configurer to apply on the list of default - * {@link HttpMessageConverter} pre-initialized + * Multiple consumers are composed together and applied to a single + * {@link HttpMessageConverters.ClientBuilder} instance. + * @param configurer the configurer to apply on an empty {@link HttpMessageConverters.ClientBuilder}. * @return this builder - * @see #messageConverters(Iterable) - * @deprecated since 7.0 in favor of {@link #configureMessageConverters(Consumer)} + * @since 7.0 */ - @Deprecated(since = "7.0", forRemoval = true) - Builder messageConverters(Consumer>> configurer); + Builder configureMessageConverters(Consumer configurer); /** - * Set the message converters for the {@code RestClient} to use. - * @param messageConverters the list of {@link HttpMessageConverter} to use + * Set the message converters to use. + *

Note: As of 7.0, the converters provided here + * populate a {@link HttpMessageConverters.ClientBuilder} initially, and + * after that the same builder is initialized further through the + * configurers provided via {@link #configureMessageConverters(Consumer)}. + * @param messageConverters the converters to use * @return this builder * @since 6.2 - * @see #configureMessageConverters(Consumer) + * @deprecated since 7.0 in favor of {@link #configureMessageConverters(Consumer)} */ + @Deprecated(since = "7.0", forRemoval = true) Builder messageConverters(Iterable> messageConverters); /** - * Configure the message converters for the {@code RestClient} to use. - * @param configurer the configurer to apply on an empty {@link HttpMessageConverters.ClientBuilder}. + * Customize the message converters to use, which is either the default + * converters, or those provided via {@link #messageConverters(Iterable)}. + * The consumer is applied immediately to the internal list + *

Note: As of 7.0, the list of converters customized + * here is used to populate a {@link HttpMessageConverters.ClientBuilder} + * initially, and after that the same builder is initialized further + * through the configurers provided via {@link #configureMessageConverters(Consumer)}. + * @param configurer the configurer to apply on the list of default + * {@link HttpMessageConverter} pre-initialized * @return this builder - * @since 7.0 + * @deprecated since 7.0 in favor of {@link #configureMessageConverters(Consumer)} */ - Builder configureMessageConverters(Consumer configurer); + @Deprecated(since = "7.0", forRemoval = true) + Builder messageConverters(Consumer>> configurer); /** * Configure the {@link io.micrometer.observation.ObservationRegistry} to use diff --git a/spring-web/src/test/java/org/springframework/web/client/RestClientBuilderTests.java b/spring-web/src/test/java/org/springframework/web/client/RestClientBuilderTests.java index c0f33f9a76d..6ead666d005 100644 --- a/spring-web/src/test/java/org/springframework/web/client/RestClientBuilderTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/RestClientBuilderTests.java @@ -112,6 +112,7 @@ public class RestClientBuilderTests { assertThat(fieldValue("baseUrl", defaultBuilder)).isEqualTo(baseUrl.toString()); } + @SuppressWarnings("removal") @Test void messageConvertersList() { StringHttpMessageConverter stringConverter = new StringHttpMessageConverter(); @@ -126,6 +127,7 @@ public class RestClientBuilderTests { .containsExactly(stringConverter); } + @SuppressWarnings("removal") @Test void messageConvertersListEmpty() { RestClient.Builder builder = RestClient.builder(); @@ -133,6 +135,7 @@ public class RestClientBuilderTests { assertThatIllegalArgumentException().isThrownBy(() -> builder.messageConverters(converters)); } + @SuppressWarnings("removal") @Test void messageConvertersListWithNullElement() { RestClient.Builder builder = RestClient.builder(); @@ -147,9 +150,9 @@ public class RestClientBuilderTests { RestClient.Builder builder = RestClient.builder(); builder.configureMessageConverters(clientBuilder -> clientBuilder.stringMessageConverter(stringConverter)); assertThat(builder).isInstanceOf(DefaultRestClientBuilder.class); - DefaultRestClientBuilder defaultBuilder = (DefaultRestClientBuilder) builder; + DefaultRestClient restClient = (DefaultRestClient) builder.build(); - assertThat(fieldValue("messageConverters", defaultBuilder)) + assertThat(fieldValue("messageConverters", restClient)) .asInstanceOf(InstanceOfAssertFactories.LIST) .hasExactlyElementsOfTypes(StringHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class); }