Skip to content

Commit 4a774fd

Browse files
tommyk-gearstommyk-gears
committed
Fix multiple HTTP message converters of the same type being lost
 When multiple HttpMessageConverters of the same specialized type (e.g., two JSON converters) were configured, only the last one was registered correctly. The first converter would be discarded because the specialized registration method was called for all converters of the same type, causing each to overwrite the previous one. This fix ensures that only the first converter of each specialized type (String, Kotlin Serialization JSON, JSON, XML) is registered using the specialized method (e.g., withJsonConverter()). Subsequent converters of the same type are registered as custom converters via addCustomConverter().
1 parent 1e96c6c commit 4a774fd

File tree

3 files changed

+243
-26
lines changed

3 files changed

+243
-26
lines changed

‎module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultClientHttpMessageConvertersCustomizer.java‎

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
package org.springframework.boot.http.converter.autoconfigure;
1818

1919
import java.util.Collection;
20+
import java.util.EnumSet;
21+
import java.util.function.BiConsumer;
22+
import java.util.function.Predicate;
2023

2124
import org.jspecify.annotations.Nullable;
2225

@@ -47,24 +50,32 @@ public void customize(ClientBuilder builder) {
4750
}
4851
else {
4952
builder.registerDefaults();
50-
this.converters.forEach((converter) -> {
51-
if (converter instanceof StringHttpMessageConverter) {
52-
builder.withStringConverter(converter);
53-
}
54-
else if (converter instanceof KotlinSerializationJsonHttpMessageConverter) {
55-
builder.withKotlinSerializationJsonConverter(converter);
56-
}
57-
else if (supportsMediaType(converter, MediaType.APPLICATION_JSON)) {
58-
builder.withJsonConverter(converter);
59-
}
60-
else if (supportsMediaType(converter, MediaType.APPLICATION_XML)) {
61-
builder.withXmlConverter(converter);
53+
EnumSet<ConverterType> registered = EnumSet.noneOf(ConverterType.class);
54+
for (HttpMessageConverter<?> converter : this.converters) {
55+
ConverterType type = findConverterType(converter);
56+
if (type != null) {
57+
if (!registered.contains(type)) {
58+
type.registerWith(builder, converter);
59+
registered.add(type);
60+
}
61+
else {
62+
builder.addCustomConverter(converter);
63+
}
6264
}
6365
else {
6466
builder.addCustomConverter(converter);
6567
}
66-
});
68+
}
69+
}
70+
}
71+
72+
private static @Nullable ConverterType findConverterType(HttpMessageConverter<?> converter) {
73+
for (ConverterType type : ConverterType.values()) {
74+
if (type.matches(converter)) {
75+
return type;
76+
}
6777
}
78+
return null;
6879
}
6980

7081
private static boolean supportsMediaType(HttpMessageConverter<?> converter, MediaType mediaType) {
@@ -76,4 +87,36 @@ private static boolean supportsMediaType(HttpMessageConverter<?> converter, Medi
7687
return false;
7788
}
7889

90+
private enum ConverterType {
91+
92+
STRING(StringHttpMessageConverter.class::isInstance, ClientBuilder::withStringConverter),
93+
94+
KOTLIN_SERIALIZATION_JSON(KotlinSerializationJsonHttpMessageConverter.class::isInstance,
95+
ClientBuilder::withKotlinSerializationJsonConverter),
96+
97+
JSON(converter -> supportsMediaType(converter, MediaType.APPLICATION_JSON),
98+
ClientBuilder::withJsonConverter),
99+
100+
XML(converter -> supportsMediaType(converter, MediaType.APPLICATION_XML), ClientBuilder::withXmlConverter);
101+
102+
private final Predicate<HttpMessageConverter<?>> matcher;
103+
104+
private final BiConsumer<ClientBuilder, HttpMessageConverter<?>> registrar;
105+
106+
ConverterType(Predicate<HttpMessageConverter<?>> matcher,
107+
BiConsumer<ClientBuilder, HttpMessageConverter<?>> registrar) {
108+
this.matcher = matcher;
109+
this.registrar = registrar;
110+
}
111+
112+
boolean matches(HttpMessageConverter<?> converter) {
113+
return this.matcher.test(converter);
114+
}
115+
116+
void registerWith(ClientBuilder builder, HttpMessageConverter<?> converter) {
117+
this.registrar.accept(builder, converter);
118+
}
119+
120+
}
121+
79122
}

‎module/spring-boot-http-converter/src/main/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizer.java‎

Lines changed: 56 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
package org.springframework.boot.http.converter.autoconfigure;
1818

1919
import java.util.Collection;
20+
import java.util.EnumSet;
21+
import java.util.function.BiConsumer;
22+
import java.util.function.Predicate;
2023

2124
import org.jspecify.annotations.Nullable;
2225

@@ -47,24 +50,32 @@ public void customize(ServerBuilder builder) {
4750
}
4851
else {
4952
builder.registerDefaults();
50-
this.converters.forEach((converter) -> {
51-
if (converter instanceof StringHttpMessageConverter) {
52-
builder.withStringConverter(converter);
53-
}
54-
else if (converter instanceof KotlinSerializationJsonHttpMessageConverter) {
55-
builder.withKotlinSerializationJsonConverter(converter);
56-
}
57-
else if (supportsMediaType(converter, MediaType.APPLICATION_JSON)) {
58-
builder.withJsonConverter(converter);
59-
}
60-
else if (supportsMediaType(converter, MediaType.APPLICATION_XML)) {
61-
builder.withXmlConverter(converter);
53+
EnumSet<ConverterType> registered = EnumSet.noneOf(ConverterType.class);
54+
for (HttpMessageConverter<?> converter : this.converters) {
55+
ConverterType type = findConverterType(converter);
56+
if (type != null) {
57+
if (!registered.contains(type)) {
58+
type.registerWith(builder, converter);
59+
registered.add(type);
60+
}
61+
else {
62+
builder.addCustomConverter(converter);
63+
}
6264
}
6365
else {
6466
builder.addCustomConverter(converter);
6567
}
66-
});
68+
}
69+
}
70+
}
71+
72+
private static @Nullable ConverterType findConverterType(HttpMessageConverter<?> converter) {
73+
for (ConverterType type : ConverterType.values()) {
74+
if (type.matches(converter)) {
75+
return type;
76+
}
6777
}
78+
return null;
6879
}
6980

7081
private static boolean supportsMediaType(HttpMessageConverter<?> converter, MediaType mediaType) {
@@ -76,4 +87,36 @@ private static boolean supportsMediaType(HttpMessageConverter<?> converter, Medi
7687
return false;
7788
}
7889

90+
private enum ConverterType {
91+
92+
STRING(StringHttpMessageConverter.class::isInstance, ServerBuilder::withStringConverter),
93+
94+
KOTLIN_SERIALIZATION_JSON(KotlinSerializationJsonHttpMessageConverter.class::isInstance,
95+
ServerBuilder::withKotlinSerializationJsonConverter),
96+
97+
JSON(converter -> supportsMediaType(converter, MediaType.APPLICATION_JSON),
98+
ServerBuilder::withJsonConverter),
99+
100+
XML(converter -> supportsMediaType(converter, MediaType.APPLICATION_XML), ServerBuilder::withXmlConverter);
101+
102+
private final Predicate<HttpMessageConverter<?>> matcher;
103+
104+
private final BiConsumer<ServerBuilder, HttpMessageConverter<?>> registrar;
105+
106+
ConverterType(Predicate<HttpMessageConverter<?>> matcher,
107+
BiConsumer<ServerBuilder, HttpMessageConverter<?>> registrar) {
108+
this.matcher = matcher;
109+
this.registrar = registrar;
110+
}
111+
112+
boolean matches(HttpMessageConverter<?> converter) {
113+
return this.matcher.test(converter);
114+
}
115+
116+
void registerWith(ServerBuilder builder, HttpMessageConverter<?> converter) {
117+
this.registrar.accept(builder, converter);
118+
}
119+
120+
}
121+
79122
}

‎module/spring-boot-http-converter/src/test/java/org/springframework/boot/http/converter/autoconfigure/DefaultServerHttpMessageConvertersCustomizerTests.java‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
* Copyright 2012-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.http.converter.autoconfigure;
18+
19+
import java.util.List;
20+
21+
import org.junit.jupiter.api.Test;
22+
23+
import org.springframework.http.MediaType;
24+
import org.springframework.http.converter.HttpMessageConverter;
25+
import org.springframework.http.converter.HttpMessageConverters.ServerBuilder;
26+
import org.springframework.http.converter.json.GsonHttpMessageConverter;
27+
import org.springframework.http.converter.json.JacksonJsonHttpMessageConverter;
28+
29+
import static org.mockito.ArgumentMatchers.any;
30+
import static org.mockito.BDDMockito.then;
31+
import static org.mockito.Mockito.mock;
32+
import static org.mockito.Mockito.times;
33+
34+
/**
35+
* Tests for {@link DefaultServerHttpMessageConvertersCustomizer}.
36+
*
37+
* @author Tommy Karlsson
38+
*/
39+
@SuppressWarnings("deprecation")
40+
class DefaultServerHttpMessageConvertersCustomizerTests {
41+
42+
@Test
43+
void customizeWithTwoJsonConvertersConfiguresFirstAsJsonConverterAndSecondAsCustomConverter() {
44+
// Create two JSON converters
45+
JacksonJsonHttpMessageConverter jsonConverter1 = new JacksonJsonHttpMessageConverter();
46+
GsonHttpMessageConverter jsonConverter2 = new GsonHttpMessageConverter();
47+
48+
// Create the customizer with both converters
49+
List<HttpMessageConverter<?>> converters = List.of(jsonConverter1, jsonConverter2);
50+
DefaultServerHttpMessageConvertersCustomizer customizer = new DefaultServerHttpMessageConvertersCustomizer(null,
51+
converters);
52+
53+
// Mock the ServerBuilder
54+
ServerBuilder builder = mock(ServerBuilder.class);
55+
56+
// Execute the customize method
57+
customizer.customize(builder);
58+
59+
// Verify that registerDefaults was called
60+
then(builder).should().registerDefaults();
61+
62+
// Verify that the first JSON converter is configured as the JSON converter
63+
then(builder).should(times(1)).withJsonConverter(jsonConverter1);
64+
65+
// Verify that the second JSON converter is configured as a custom converter
66+
// (since there should only be one JSON converter registered via withJsonConverter)
67+
then(builder).should(times(1)).addCustomConverter(jsonConverter2);
68+
69+
// Verify that withJsonConverter was only called once (for the first converter)
70+
then(builder).should(times(1)).withJsonConverter(any());
71+
}
72+
73+
@Test
74+
void customizeWithJsonConverterVerifiesJsonConverterConfiguration() {
75+
// Create a single JSON converter
76+
JacksonJsonHttpMessageConverter jsonConverter = new JacksonJsonHttpMessageConverter();
77+
78+
// Verify it supports JSON
79+
boolean supportsJson = jsonConverter.getSupportedMediaTypes()
80+
.stream()
81+
.anyMatch(mediaType -> mediaType.equalsTypeAndSubtype(MediaType.APPLICATION_JSON));
82+
assert supportsJson : "Converter should support APPLICATION_JSON";
83+
84+
// Create the customizer
85+
List<HttpMessageConverter<?>> converters = List.of(jsonConverter);
86+
DefaultServerHttpMessageConvertersCustomizer customizer = new DefaultServerHttpMessageConvertersCustomizer(null,
87+
converters);
88+
89+
// Mock the ServerBuilder
90+
ServerBuilder builder = mock(ServerBuilder.class);
91+
92+
// Execute the customize method
93+
customizer.customize(builder);
94+
95+
// Verify that registerDefaults was called
96+
then(builder).should().registerDefaults();
97+
98+
// Verify that withJsonConverter was called with the JSON converter
99+
then(builder).should().withJsonConverter(jsonConverter);
100+
101+
// Verify that addCustomConverter was not called for the JSON converter
102+
then(builder).should(times(0)).addCustomConverter(any());
103+
}
104+
105+
@Test
106+
void customizeWithNonJsonConverterConfiguresAsCustomConverter() {
107+
// Create a custom converter that doesn't support JSON
108+
HttpMessageConverter<?> customConverter = mock(HttpMessageConverter.class);
109+
110+
// Create the customizer
111+
List<HttpMessageConverter<?>> converters = List.of(customConverter);
112+
DefaultServerHttpMessageConvertersCustomizer customizer = new DefaultServerHttpMessageConvertersCustomizer(null,
113+
converters);
114+
115+
// Mock the ServerBuilder
116+
ServerBuilder builder = mock(ServerBuilder.class);
117+
118+
// Execute the customize method
119+
customizer.customize(builder);
120+
121+
// Verify that registerDefaults was called
122+
then(builder).should().registerDefaults();
123+
124+
// Verify that addCustomConverter was called with the custom converter
125+
then(builder).should().addCustomConverter(customConverter);
126+
127+
// Verify that withJsonConverter was not called
128+
then(builder).should(times(0)).withJsonConverter(any());
129+
}
130+
131+
}

0 commit comments

Comments
 (0)