Align HttpMessageConverters builder with WebFlux codecs variants

As of #33894, we introduced a new `HttpMessageConverters` API.
While this achieved our goal of focusing converters classpath detection
in a single place and avoiding waste, a single `HttpMessageConverters`
instance for both client and server added more complexity for developers.

This commit aligns the API here with the WebFlux `CodecsConfigurer` to
opt for a client/server flavor as the first step in the builder.
While this make the sharing of converter instances between server and
client impossible, this allows for a simpler API and separates concerns.

Closes gh-35187

spring-web/src/test/java/org/springframework/http/converter/DefaultHttpMessageConvertersTests.java

@@ -17,11 +17,6 @@
 package org.springframework.http.converter;
 
 import java.io.IOException;
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.Set;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
@@ -30,7 +25,6 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
-import org.springframework.core.SmartClassLoader;
 import org.springframework.http.HttpInputMessage;
 import org.springframework.http.HttpOutputMessage;
 import org.springframework.http.converter.cbor.JacksonCborHttpMessageConverter;
@@ -60,59 +54,50 @@ class DefaultHttpMessageConvertersTests {
 
 	static Stream<Iterable<HttpMessageConverter<?>>> emptyMessageConverters() {
 		return Stream.of(
-				HttpMessageConverters.create().build().forClient(),
-				HttpMessageConverters.create().build().forServer()
+				HttpMessageConverters.forClient().build(),
+				HttpMessageConverters.forServer().build()
 		);
 	}
 
-	@Test
-	void clientAndServerConvertersAreShared() {
-		var converters = HttpMessageConverters.withDefaults().build();
-		Set<HttpMessageConverter<?>> allConverters = new HashSet<>();
-		converters.forClient().forEach(allConverters::add);
-		converters.forServer().forEach(allConverters::add);
-		assertThat(allConverters).hasSize(15);
-	}
-
 	@Test
 	void failsWhenStringConverterDoesNotSupportMediaType() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> HttpMessageConverters.create().stringMessageConverter(new CustomHttpMessageConverter()).build())
+				.isThrownBy(() -> HttpMessageConverters.forClient().stringMessageConverter(new CustomHttpMessageConverter()).build())
 				.withMessage("stringMessageConverter should support 'text/plain'");
 	}
 
 	@Test
 	void failsWhenJsonConverterDoesNotSupportMediaType() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> HttpMessageConverters.create().jsonMessageConverter(new CustomHttpMessageConverter()).build())
+				.isThrownBy(() -> HttpMessageConverters.forClient().jsonMessageConverter(new CustomHttpMessageConverter()).build())
 				.withMessage("jsonMessageConverter should support 'application/json'");
 	}
 
 	@Test
 	void failsWhenXmlConverterDoesNotSupportMediaType() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> HttpMessageConverters.create().xmlMessageConverter(new CustomHttpMessageConverter()).build())
+				.isThrownBy(() -> HttpMessageConverters.forClient().xmlMessageConverter(new CustomHttpMessageConverter()).build())
 				.withMessage("xmlMessageConverter should support 'text/xml'");
 	}
 
 	@Test
 	void failsWhenSmileConverterDoesNotSupportMediaType() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> HttpMessageConverters.create().smileMessageConverter(new CustomHttpMessageConverter()).build())
+				.isThrownBy(() -> HttpMessageConverters.forClient().smileMessageConverter(new CustomHttpMessageConverter()).build())
 				.withMessage("smileMessageConverter should support 'application/x-jackson-smile'");
 	}
 
 	@Test
 	void failsWhenCborConverterDoesNotSupportMediaType() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> HttpMessageConverters.create().cborMessageConverter(new CustomHttpMessageConverter()).build())
+				.isThrownBy(() -> HttpMessageConverters.forClient().cborMessageConverter(new CustomHttpMessageConverter()).build())
 				.withMessage("cborMessageConverter should support 'application/cbor'");
 	}
 
 	@Test
 	void failsWhenYamlConverterDoesNotSupportMediaType() {
 		assertThatIllegalArgumentException()
-				.isThrownBy(() -> HttpMessageConverters.create().yamlMessageConverter(new CustomHttpMessageConverter()).build())
+				.isThrownBy(() -> HttpMessageConverters.forClient().yamlMessageConverter(new CustomHttpMessageConverter()).build())
 				.withMessage("yamlMessageConverter should support 'application/yaml'");
 	}
 
@@ -122,8 +107,8 @@ class DefaultHttpMessageConvertersTests {
 
 		@Test
 		void defaultConverters() {
-			var converters = HttpMessageConverters.withDefaults().build();
-			assertThat(converters.forClient()).hasExactlyElementsOfTypes(ByteArrayHttpMessageConverter.class,
+			var converters = HttpMessageConverters.forClient().registerDefaults().build();
+			assertThat(converters).hasExactlyElementsOfTypes(ByteArrayHttpMessageConverter.class,
 					StringHttpMessageConverter.class, ResourceHttpMessageConverter.class,
 					AllEncompassingFormHttpMessageConverter.class,
 					JacksonJsonHttpMessageConverter.class, JacksonSmileHttpMessageConverter.class,
@@ -134,8 +119,8 @@ class DefaultHttpMessageConvertersTests {
 
 		@Test
 		void multipartConverterContainsOtherConverters() {
-			var converters = HttpMessageConverters.withDefaults().build();
-			AllEncompassingFormHttpMessageConverter multipartConverter = findMessageConverter(AllEncompassingFormHttpMessageConverter.class, converters.forClient());
+			var converters = HttpMessageConverters.forClient().registerDefaults().build();
+			var multipartConverter = findMessageConverter(AllEncompassingFormHttpMessageConverter.class, converters);
 
 			assertThat(multipartConverter.getPartConverters()).hasExactlyElementsOfTypes(
 					ByteArrayHttpMessageConverter.class, StringHttpMessageConverter.class,
@@ -148,16 +133,16 @@ class DefaultHttpMessageConvertersTests {
 
 		@Test
 		void registerCustomMessageConverter() {
-			var converters = HttpMessageConverters.create()
-					.additionalMessageConverter(new CustomHttpMessageConverter()).build();
-			assertThat(converters.forClient()).hasExactlyElementsOfTypes(CustomHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class);
+			var converters = HttpMessageConverters.forClient()
+					.customMessageConverter(new CustomHttpMessageConverter()).build();
+			assertThat(converters).hasExactlyElementsOfTypes(CustomHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class);
 		}
 
 		@Test
 		void registerCustomMessageConverterAheadOfDefaults() {
-			var converters = HttpMessageConverters.withDefaults()
-					.additionalMessageConverter(new CustomHttpMessageConverter()).build();
-			assertThat(converters.forClient()).hasExactlyElementsOfTypes(
+			var converters = HttpMessageConverters.forClient().registerDefaults()
+					.customMessageConverter(new CustomHttpMessageConverter()).build();
+			assertThat(converters).hasExactlyElementsOfTypes(
 					CustomHttpMessageConverter.class, ByteArrayHttpMessageConverter.class,
 					StringHttpMessageConverter.class, ResourceHttpMessageConverter.class,
 					AllEncompassingFormHttpMessageConverter.class,
@@ -169,33 +154,31 @@ class DefaultHttpMessageConvertersTests {
 
 		@Test
 		void registerCustomConverterInMultipartConverter() {
-			var converters = HttpMessageConverters.withDefaults()
-					.additionalMessageConverter(new CustomHttpMessageConverter()).build();
-			AllEncompassingFormHttpMessageConverter multipartConverter = findMessageConverter(AllEncompassingFormHttpMessageConverter.class, converters.forClient());
+			var converters = HttpMessageConverters.forClient().registerDefaults()
+					.customMessageConverter(new CustomHttpMessageConverter()).build();
+			var multipartConverter = findMessageConverter(AllEncompassingFormHttpMessageConverter.class, converters);
 			assertThat(multipartConverter.getPartConverters()).hasAtLeastOneElementOfType(CustomHttpMessageConverter.class);
 		}
 
 		@Test
-		void shouldUseServerSpecificConverter() {
-			JacksonJsonHttpMessageConverter jacksonConverter = new JacksonJsonHttpMessageConverter();
-			var converters = HttpMessageConverters.withDefaults()
-					.configureClient(configurer -> configurer.jsonMessageConverter(jacksonConverter)).build();
+		void shouldUseSpecificConverter() {
+			var jacksonConverter = new JacksonJsonHttpMessageConverter();
+			var converters = HttpMessageConverters.forClient().registerDefaults()
+					.jsonMessageConverter(jacksonConverter).build();
 
-			JacksonJsonHttpMessageConverter customConverter = findMessageConverter(JacksonJsonHttpMessageConverter.class, converters.forClient());
+			var customConverter = findMessageConverter(JacksonJsonHttpMessageConverter.class, converters);
 			assertThat(customConverter).isEqualTo(jacksonConverter);
 		}
 
 		@Test
 		void shouldConfigureConverter() {
-			CustomHttpMessageConverter customConverter = new CustomHttpMessageConverter();
-			HttpMessageConverters.withDefaults()
-					.additionalMessageConverter(customConverter)
-					.configureClient(configurer -> {
-						configurer.configureClientMessageConverters(converter -> {
-							if (converter instanceof CustomHttpMessageConverter custom) {
-								custom.processed = true;
-							}
-						});
+			var customConverter = new CustomHttpMessageConverter();
+			HttpMessageConverters.forClient()
+					.customMessageConverter(customConverter)
+					.configureMessageConverters(converter -> {
+						if (converter instanceof CustomHttpMessageConverter custom) {
+							custom.processed = true;
+						}
 					}).build();
 
 			assertThat(customConverter.processed).isTrue();
@@ -209,8 +192,8 @@ class DefaultHttpMessageConvertersTests {
 
 		@Test
 		void defaultConverters() {
-			var converters = HttpMessageConverters.withDefaults().build();
-			assertThat(converters.forServer()).hasExactlyElementsOfTypes(
+			var converters = HttpMessageConverters.forServer().registerDefaults().build();
+			assertThat(converters).hasExactlyElementsOfTypes(
 					ByteArrayHttpMessageConverter.class, StringHttpMessageConverter.class,
 					ResourceHttpMessageConverter.class, ResourceRegionHttpMessageConverter.class,
 					AllEncompassingFormHttpMessageConverter.class,
@@ -222,8 +205,8 @@ class DefaultHttpMessageConvertersTests {
 
 		@Test
 		void multipartConverterContainsOtherConverters() {
-			var converters = HttpMessageConverters.withDefaults().build();
-			AllEncompassingFormHttpMessageConverter multipartConverter = findMessageConverter(AllEncompassingFormHttpMessageConverter.class, converters.forServer());
+			var converters = HttpMessageConverters.forServer().registerDefaults().build();
+			var multipartConverter = findMessageConverter(AllEncompassingFormHttpMessageConverter.class, converters);
 
 			assertThat(multipartConverter.getPartConverters()).hasExactlyElementsOfTypes(
 					ByteArrayHttpMessageConverter.class, StringHttpMessageConverter.class,
@@ -236,16 +219,16 @@ class DefaultHttpMessageConvertersTests {
 
 		@Test
 		void registerCustomMessageConverter() {
-			var converters = HttpMessageConverters.create()
-					.additionalMessageConverter(new CustomHttpMessageConverter()).build();
-			assertThat(converters.forServer()).hasExactlyElementsOfTypes(CustomHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class);
+			var converters = HttpMessageConverters.forServer()
+					.customMessageConverter(new CustomHttpMessageConverter()).build();
+			assertThat(converters).hasExactlyElementsOfTypes(CustomHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class);
 		}
 
 		@Test
 		void registerCustomMessageConverterAheadOfDefaults() {
-			var converters = HttpMessageConverters.withDefaults()
-					.additionalMessageConverter(new CustomHttpMessageConverter()).build();
-			assertThat(converters.forServer()).hasExactlyElementsOfTypes(
+			var converters = HttpMessageConverters.forServer().registerDefaults()
+					.customMessageConverter(new CustomHttpMessageConverter()).build();
+			assertThat(converters).hasExactlyElementsOfTypes(
 					CustomHttpMessageConverter.class,
 					ByteArrayHttpMessageConverter.class, StringHttpMessageConverter.class,
 					ResourceHttpMessageConverter.class, ResourceRegionHttpMessageConverter.class,
@@ -258,33 +241,31 @@ class DefaultHttpMessageConvertersTests {
 
 		@Test
 		void registerCustomConverterInMultipartConverter() {
-			var converters = HttpMessageConverters.withDefaults()
-					.additionalMessageConverter(new CustomHttpMessageConverter()).build();
-			AllEncompassingFormHttpMessageConverter multipartConverter = findMessageConverter(AllEncompassingFormHttpMessageConverter.class, converters.forServer());
+			var converters = HttpMessageConverters.forServer().registerDefaults()
+					.customMessageConverter(new CustomHttpMessageConverter()).build();
+			var multipartConverter = findMessageConverter(AllEncompassingFormHttpMessageConverter.class, converters);
 			assertThat(multipartConverter.getPartConverters()).hasAtLeastOneElementOfType(CustomHttpMessageConverter.class);
 		}
 
 		@Test
 		void shouldUseServerSpecificConverter() {
-			JacksonJsonHttpMessageConverter jacksonConverter = new JacksonJsonHttpMessageConverter();
-			var converters = HttpMessageConverters.withDefaults()
-					.configureServer(configurer -> configurer.jsonMessageConverter(jacksonConverter)).build();
+			var jacksonConverter = new JacksonJsonHttpMessageConverter();
+			var converters = HttpMessageConverters.forServer().registerDefaults()
+					.jsonMessageConverter(jacksonConverter).build();
 
-			JacksonJsonHttpMessageConverter customConverter = findMessageConverter(JacksonJsonHttpMessageConverter.class, converters.forServer());
+			var customConverter = findMessageConverter(JacksonJsonHttpMessageConverter.class, converters);
 			assertThat(customConverter).isEqualTo(jacksonConverter);
 		}
 
 		@Test
 		void shouldConfigureConverter() {
-			CustomHttpMessageConverter customConverter = new CustomHttpMessageConverter();
-			HttpMessageConverters.withDefaults()
-					.additionalMessageConverter(customConverter)
-					.configureServer(configurer -> {
-						configurer.configureServerMessageConverters(converter -> {
-							if (converter instanceof CustomHttpMessageConverter custom) {
-								custom.processed = true;
-							}
-						});
+			var customConverter = new CustomHttpMessageConverter();
+			HttpMessageConverters.forServer().registerDefaults()
+					.customMessageConverter(customConverter)
+					.configureMessageConverters(converter -> {
+						if (converter instanceof CustomHttpMessageConverter custom) {
+							custom.processed = true;
+						}
 					}).build();
 
 			assertThat(customConverter.processed).isTrue();
@@ -299,32 +280,6 @@ class DefaultHttpMessageConvertersTests {
 				.findFirst().orElseThrow();
 	}
 
-
-	static class FilteredClassLoader extends URLClassLoader implements SmartClassLoader {
-
-		private final Collection<Class<?>> hiddenClasses;
-
-		public FilteredClassLoader(Class<?>... hiddenClasses) {
-			this(java.util.Arrays.asList(hiddenClasses));
-		}
-
-		FilteredClassLoader(Collection<Class<?>> hiddenClasses) {
-			super(new URL[0], FilteredClassLoader.class.getClassLoader());
-			this.hiddenClasses = hiddenClasses;
-		}
-
-		@Override
-		protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
-			for (Class<?> hiddenClass : this.hiddenClasses) {
-				if (hiddenClass.getName().equals(name)) {
-					throw new ClassNotFoundException();
-				}
-			}
-			return super.loadClass(name, resolve);
-		}
-
-	}
-
 	static class CustomHttpMessageConverter extends AbstractHttpMessageConverter<Object> {
 
 		boolean processed = false;

spring-web/src/test/java/org/springframework/web/client/RestClientBuilderTests.java

@@ -41,6 +41,7 @@ import static org.assertj.core.api.Assertions.fail;
 
 
 /**
+ * Tests for {@link DefaultRestClientBuilder}.
  * @author Arjen Poutsma
  * @author Sebastien Deleuze
  * @author Nicklas Wiegandt
@@ -139,6 +140,19 @@ public class RestClientBuilderTests {
 		assertThatIllegalArgumentException().isThrownBy(() -> builder.messageConverters(converters));
 	}
 
+	@Test
+	void configureMessageConverters() {
+		StringHttpMessageConverter stringConverter = new StringHttpMessageConverter();
+		RestClient.Builder builder = RestClient.builder();
+		builder.configureMessageConverters(clientBuilder -> clientBuilder.stringMessageConverter(stringConverter));
+		assertThat(builder).isInstanceOf(DefaultRestClientBuilder.class);
+		DefaultRestClientBuilder defaultBuilder = (DefaultRestClientBuilder) builder;
+
+		assertThat(fieldValue("messageConverters", defaultBuilder))
+				.asInstanceOf(InstanceOfAssertFactories.LIST)
+				.containsExactly(stringConverter);
+	}
+
 	@Test
 	void defaultCookieAddsCookieToDefaultCookiesMap() {
 		RestClient.Builder builder = RestClient.builder();

spring-web/src/test/java/org/springframework/web/client/RestClientObservationTests.java

@@ -44,7 +44,7 @@ import org.springframework.http.client.ClientHttpResponse;
 import org.springframework.http.client.observation.ClientRequestObservationContext;
 import org.springframework.http.client.observation.ClientRequestObservationConvention;
 import org.springframework.http.client.observation.DefaultClientRequestObservationConvention;
-import org.springframework.http.converter.HttpMessageConverter;
+import org.springframework.http.converter.StringHttpMessageConverter;
 import org.springframework.util.StreamUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -70,8 +70,6 @@ class RestClientObservationTests {
 
 	private final ClientHttpResponse response = mock();
 
-	private final HttpMessageConverter<String> converter = mock();
-
 	private RestClient client;
 
 
@@ -84,7 +82,7 @@ class RestClientObservationTests {
 	RestClient.Builder createBuilder() {
 		return RestClient.builder()
 				.baseUrl("https://example.com/base")
-				.messageConverters(converters -> converters.add(0, this.converter))
+				.configureMessageConverters(converters -> converters.customMessageConverter(new StringHttpMessageConverter()))
 				.requestFactory(this.requestFactory)
 				.observationRegistry(this.observationRegistry);
 	}
@@ -203,7 +201,7 @@ class RestClientObservationTests {
 		assertThatExceptionOfType(RestClientException.class).isThrownBy(() ->
 				client.get().uri(url).retrieve().body(User.class));
 
-		assertThatHttpObservation().hasLowCardinalityKeyValue("exception", "RestClientException");
+		assertThatHttpObservation().hasLowCardinalityKeyValue("exception", "UnknownContentTypeException");
 	}
 
 	@Test