Skip to content

Commit 0a705f4

Browse files
bclozelbclozel
committed
Configure custom converters ahead in HttpMessageConverters
This commit aligns `HttpMessageConverters` with the WebFlux codecs configuration by adding custom converters first, before the auto-detected and well-known ones. This is a general approach in Spring Framework, where custom implementations are more likely to handle specific uses cases and should be involved before considering the more general implementations. Fixes gh-35177
1 parent 440bf94 commit 0a705f4

File tree

3 files changed

+73
-17
lines changed

3 files changed

+73
-17
lines changed

‎spring-web/src/main/java/org/springframework/http/converter/DefaultHttpMessageConverters.java

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public Builder yamlMessageConverter(HttpMessageConverter<?> yamlMessageConverter
149149
@Override
150150
public DefaultBuilder additionalMessageConverter(HttpMessageConverter<?> customConverter) {
151151
Assert.notNull(customConverter, "'customConverter' must not be null");
152-
this.commonMessageConverters.additionalMessageConverters.add(customConverter);
152+
this.commonMessageConverters.customMessageConverters.add(customConverter);
153153
return this;
154154
}
155155

@@ -216,7 +216,13 @@ static class DefaultMessageConverterConfigurer {
216216

217217
private @Nullable HttpMessageConverter<?> yamlMessageConverter;
218218

219-
private final List<HttpMessageConverter<?>> additionalMessageConverters = new ArrayList<>();
219+
private @Nullable HttpMessageConverter<?> protobufMessageConverter;
220+
221+
private @Nullable HttpMessageConverter<?> atomMessageConverter;
222+
223+
private @Nullable HttpMessageConverter<?> rssMessageConverter;
224+
225+
private final List<HttpMessageConverter<?>> customMessageConverters = new ArrayList<>();
220226

221227
static {
222228
ClassLoader classLoader = DefaultClientMessageConverterConfigurer.class.getClassLoader();
@@ -340,13 +346,34 @@ else if (this.inheritedMessageConverters != null &&
340346
this.inheritedMessageConverters.xmlMessageConverter != null) {
341347
converters.add(this.inheritedMessageConverters.xmlMessageConverter);
342348
}
349+
if (this.protobufMessageConverter != null) {
350+
converters.add(this.protobufMessageConverter);
351+
}
352+
else if (this.inheritedMessageConverters != null &&
353+
this.inheritedMessageConverters.protobufMessageConverter != null) {
354+
converters.add(this.inheritedMessageConverters.protobufMessageConverter);
355+
}
356+
if (this.atomMessageConverter != null) {
357+
converters.add(this.atomMessageConverter);
358+
}
359+
else if (this.inheritedMessageConverters != null &&
360+
this.inheritedMessageConverters.atomMessageConverter != null) {
361+
converters.add(this.inheritedMessageConverters.atomMessageConverter);
362+
}
363+
if (this.rssMessageConverter != null) {
364+
converters.add(this.rssMessageConverter);
365+
}
366+
else if (this.inheritedMessageConverters != null &&
367+
this.inheritedMessageConverters.rssMessageConverter != null) {
368+
converters.add(this.inheritedMessageConverters.rssMessageConverter);
369+
}
343370
return converters;
344371
}
345372

346373
List<HttpMessageConverter<?>> getCustomConverters() {
347-
List<HttpMessageConverter<?>> result = new ArrayList<>(this.additionalMessageConverters);
374+
List<HttpMessageConverter<?>> result = new ArrayList<>(this.customMessageConverters);
348375
if (this.inheritedMessageConverters != null) {
349-
result.addAll(this.inheritedMessageConverters.additionalMessageConverters);
376+
result.addAll(this.inheritedMessageConverters.customMessageConverters);
350377
}
351378
return result;
352379
}
@@ -405,12 +432,12 @@ else if (isJackson2YamlPresent) {
405432
}
406433

407434
if (isKotlinSerializationProtobufPresent) {
408-
this.additionalMessageConverters.add(new KotlinSerializationProtobufHttpMessageConverter());
435+
this.protobufMessageConverter = new KotlinSerializationProtobufHttpMessageConverter();
409436
}
410437

411438
if (isRomePresent) {
412-
this.additionalMessageConverters.add(new AtomFeedHttpMessageConverter());
413-
this.additionalMessageConverters.add(new RssChannelHttpMessageConverter());
439+
this.atomMessageConverter = new AtomFeedHttpMessageConverter();
440+
this.rssMessageConverter = new RssChannelHttpMessageConverter();
414441
}
415442
}
416443

@@ -466,7 +493,7 @@ public ClientMessageConverterConfigurer yamlMessageConverter(HttpMessageConverte
466493
@Override
467494
public ClientMessageConverterConfigurer additionalMessageConverter(HttpMessageConverter<?> customConverter) {
468495
Assert.notNull(customConverter, "'customConverter' must not be null");
469-
this.clientMessageConverters.additionalMessageConverters.add(customConverter);
496+
this.clientMessageConverters.customMessageConverters.add(customConverter);
470497
return this;
471498
}
472499

@@ -485,16 +512,16 @@ List<HttpMessageConverter<?>> getMessageConverters() {
485512
List<HttpMessageConverter<?>> allConverters = new ArrayList<>();
486513
List<HttpMessageConverter<?>> partConverters = new ArrayList<>();
487514

488-
partConverters.addAll(this.clientMessageConverters.getCoreConverters());
489515
partConverters.addAll(this.clientMessageConverters.getCustomConverters());
516+
partConverters.addAll(this.clientMessageConverters.getCoreConverters());
490517

518+
allConverters.addAll(this.clientMessageConverters.getCustomConverters());
491519
allConverters.addAll(this.clientMessageConverters.getBaseConverters());
492520
allConverters.addAll(this.resourceMessageConverters);
493521
if (!partConverters.isEmpty()) {
494522
allConverters.add(new AllEncompassingFormHttpMessageConverter(partConverters));
495523
}
496524
allConverters.addAll(this.clientMessageConverters.getCoreConverters());
497-
allConverters.addAll(this.clientMessageConverters.getCustomConverters());
498525

499526
if (this.configurer != null) {
500527
allConverters.forEach(this.configurer);
@@ -553,7 +580,7 @@ public ServerMessageConverterConfigurer yamlMessageConverter(HttpMessageConverte
553580
@Override
554581
public ServerMessageConverterConfigurer additionalMessageConverter(HttpMessageConverter<?> customConverter) {
555582
Assert.notNull(customConverter, "'customConverter' must not be null");
556-
this.serverMessageConverters.additionalMessageConverters.add(customConverter);
583+
this.serverMessageConverters.customMessageConverters.add(customConverter);
557584
return this;
558585
}
559586

@@ -572,16 +599,16 @@ List<HttpMessageConverter<?>> getMessageConverters() {
572599
List<HttpMessageConverter<?>> allConverters = new ArrayList<>();
573600
List<HttpMessageConverter<?>> partConverters = new ArrayList<>();
574601

575-
partConverters.addAll(this.serverMessageConverters.getCoreConverters());
576602
partConverters.addAll(this.serverMessageConverters.getCustomConverters());
603+
partConverters.addAll(this.serverMessageConverters.getCoreConverters());
577604

605+
allConverters.addAll(this.serverMessageConverters.getCustomConverters());
578606
allConverters.addAll(this.serverMessageConverters.getBaseConverters());
579607
allConverters.addAll(this.resourceMessageConverters);
580608
if (!partConverters.isEmpty()) {
581609
allConverters.add(new AllEncompassingFormHttpMessageConverter(partConverters));
582610
}
583611
allConverters.addAll(this.serverMessageConverters.getCoreConverters());
584-
allConverters.addAll(this.serverMessageConverters.getCustomConverters());
585612
if (this.configurer != null) {
586613
allConverters.forEach(this.configurer);
587614
}

‎spring-web/src/main/java/org/springframework/http/converter/HttpMessageConverters.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
* <p>The following HTTP message converters will be detected and registered if available, in order.
3232
* For {@link #forClient() client side converters}:
3333
* <ol>
34+
* <li>All custom message converters configured with the builder
3435
* <li>{@link ByteArrayHttpMessageConverter}
3536
* <li>{@link StringHttpMessageConverter} with the {@link java.nio.charset.StandardCharsets#ISO_8859_1} charset
3637
* <li>{@link ResourceHttpMessageConverter}, with resource streaming support disabled
@@ -42,11 +43,11 @@
4243
* <li>An XML converter
4344
* <li>An ProtoBuf converter
4445
* <li>ATOM and RSS converters
45-
* <li>All custom message converters configured with the builder
4646
* </ol>
4747
*
4848
* For {@link #forClient() client side converters}:
4949
* <ol>
50+
* <li>All custom message converters configured with the builder
5051
* <li>{@link ByteArrayHttpMessageConverter}
5152
* <li>{@link StringHttpMessageConverter} with the {@link java.nio.charset.StandardCharsets#ISO_8859_1} charset
5253
* <li>{@link ResourceHttpMessageConverter}
@@ -58,7 +59,6 @@
5859
* <li>An XML converter
5960
* <li>An ProtoBuf converter
6061
* <li>ATOM and RSS converters
61-
* <li>All custom message converters configured with the builder
6262
* <li>a Multipart converter, using all detected and custom converters for part conversion
6363
* </ol>
6464
*

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

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,21 @@ void multipartConverterContainsOtherConverters() {
150150
void registerCustomMessageConverter() {
151151
var converters = HttpMessageConverters.create()
152152
.additionalMessageConverter(new CustomHttpMessageConverter()).build();
153-
assertThat(converters.forClient()).hasExactlyElementsOfTypes(AllEncompassingFormHttpMessageConverter.class, CustomHttpMessageConverter.class);
153+
assertThat(converters.forClient()).hasExactlyElementsOfTypes(CustomHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class);
154+
}
155+
156+
@Test
157+
void registerCustomMessageConverterAheadOfDefaults() {
158+
var converters = HttpMessageConverters.withDefaults()
159+
.additionalMessageConverter(new CustomHttpMessageConverter()).build();
160+
assertThat(converters.forClient()).hasExactlyElementsOfTypes(
161+
CustomHttpMessageConverter.class, ByteArrayHttpMessageConverter.class,
162+
StringHttpMessageConverter.class, ResourceHttpMessageConverter.class,
163+
AllEncompassingFormHttpMessageConverter.class,
164+
JacksonJsonHttpMessageConverter.class, JacksonSmileHttpMessageConverter.class,
165+
JacksonCborHttpMessageConverter.class, JacksonYamlHttpMessageConverter.class,
166+
JacksonXmlHttpMessageConverter.class, KotlinSerializationProtobufHttpMessageConverter.class,
167+
AtomFeedHttpMessageConverter.class, RssChannelHttpMessageConverter.class);
154168
}
155169

156170
@Test
@@ -224,7 +238,22 @@ void multipartConverterContainsOtherConverters() {
224238
void registerCustomMessageConverter() {
225239
var converters = HttpMessageConverters.create()
226240
.additionalMessageConverter(new CustomHttpMessageConverter()).build();
227-
assertThat(converters.forServer()).hasExactlyElementsOfTypes(AllEncompassingFormHttpMessageConverter.class, CustomHttpMessageConverter.class);
241+
assertThat(converters.forServer()).hasExactlyElementsOfTypes(CustomHttpMessageConverter.class, AllEncompassingFormHttpMessageConverter.class);
242+
}
243+
244+
@Test
245+
void registerCustomMessageConverterAheadOfDefaults() {
246+
var converters = HttpMessageConverters.withDefaults()
247+
.additionalMessageConverter(new CustomHttpMessageConverter()).build();
248+
assertThat(converters.forServer()).hasExactlyElementsOfTypes(
249+
CustomHttpMessageConverter.class,
250+
ByteArrayHttpMessageConverter.class, StringHttpMessageConverter.class,
251+
ResourceHttpMessageConverter.class, ResourceRegionHttpMessageConverter.class,
252+
AllEncompassingFormHttpMessageConverter.class,
253+
JacksonJsonHttpMessageConverter.class, JacksonSmileHttpMessageConverter.class,
254+
JacksonCborHttpMessageConverter.class, JacksonYamlHttpMessageConverter.class,
255+
JacksonXmlHttpMessageConverter.class, KotlinSerializationProtobufHttpMessageConverter.class,
256+
AtomFeedHttpMessageConverter.class, RssChannelHttpMessageConverter.class);
228257
}
229258

230259
@Test

0 commit comments

Comments
 (0)