From 769ec03f3ca4eca3768f08cc1a17e7126d16731f Mon Sep 17 00:00:00 2001 From: spencergibb Date: Thu, 20 Nov 2025 09:50:38 -0500 Subject: [PATCH] Updates for HttpMessageConverters changes in framework 7/boot 4. Since HttpMessageConverter instances are no longer registered as beans, moved to a new strategy backed by FeignHttpMessageConverters which is mostly copied from boot. Fixes gh-1269 --- .../openfeign/FeignClientsConfiguration.java | 34 ++++--- .../support/FeignHttpMessageConverters.java | 93 +++++++++++++++++++ .../openfeign/support/SpringDecoder.java | 21 +---- .../openfeign/support/SpringEncoder.java | 29 ++---- .../cloud/openfeign/SpringDecoderTests.java | 11 +-- .../proto/ProtobufNotInClasspathTest.java | 5 +- .../proto/ProtobufSpringEncoderTests.java | 16 +++- .../openfeign/support/SpringEncoderTests.java | 39 +++++--- 8 files changed, 172 insertions(+), 76 deletions(-) create mode 100644 spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/FeignHttpMessageConverters.java diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java index a9edc6c33..bee2e2af1 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientsConfiguration.java @@ -47,6 +47,7 @@ import org.springframework.cloud.openfeign.clientconfig.FeignClientConfigurer; import org.springframework.cloud.openfeign.support.AbstractFormWriter; import org.springframework.cloud.openfeign.support.FeignEncoderProperties; +import org.springframework.cloud.openfeign.support.FeignHttpMessageConverters; import org.springframework.cloud.openfeign.support.HttpMessageConverterCustomizer; import org.springframework.cloud.openfeign.support.PageableSpringEncoder; import org.springframework.cloud.openfeign.support.PageableSpringQueryMapEncoder; @@ -98,16 +99,24 @@ public class FeignClientsConfiguration { @Bean @ConditionalOnMissingBean - public Decoder feignDecoder(ObjectProvider customizers) { - return new OptionalDecoder(new ResponseEntityDecoder(new SpringDecoder(messageConverters, customizers))); + public FeignHttpMessageConverters feignHttpMessageConverters( + ObjectProvider> messageConverters, + ObjectProvider customizers) { + return new FeignHttpMessageConverters(messageConverters, customizers); + } + + @Bean + @ConditionalOnMissingBean + public Decoder feignDecoder(ObjectProvider messageConverters) { + return new OptionalDecoder(new ResponseEntityDecoder(new SpringDecoder(messageConverters))); } @Bean @ConditionalOnMissingBean @ConditionalOnMissingClass("org.springframework.data.domain.Pageable") public Encoder feignEncoder(ObjectProvider formWriterProvider, - ObjectProvider customizers) { - return springEncoder(formWriterProvider, messageConverters, encoderProperties, customizers); + ObjectProvider feignHttpMessageConverters) { + return springEncoder(formWriterProvider, encoderProperties, feignHttpMessageConverters); } @Bean @@ -145,16 +154,16 @@ public FeignClientConfigurer feignClientConfigurer() { } private static Encoder springEncoder(ObjectProvider formWriterProvider, - ObjectProvider> messageConverters, FeignEncoderProperties encoderProperties, - ObjectProvider customizers) { + FeignEncoderProperties encoderProperties, + ObjectProvider feignHttpMessageConverters) { AbstractFormWriter formWriter = formWriterProvider.getIfAvailable(); if (formWriter != null) { - return new SpringEncoder(new SpringPojoFormEncoder(formWriter), messageConverters, encoderProperties, - customizers); + return new SpringEncoder(new SpringPojoFormEncoder(formWriter), encoderProperties, + feignHttpMessageConverters); } else { - return new SpringEncoder(new SpringFormEncoder(), messageConverters, encoderProperties, customizers); + return new SpringEncoder(new SpringFormEncoder(), encoderProperties, feignHttpMessageConverters); } } @@ -182,11 +191,10 @@ static class SpringDataConfiguration { @Bean @ConditionalOnMissingBean public Encoder feignEncoderPageable(ObjectProvider formWriterProvider, - ObjectProvider> messageConverters, ObjectProvider encoderProperties, - ObjectProvider customizers) { - PageableSpringEncoder encoder = new PageableSpringEncoder(springEncoder(formWriterProvider, - messageConverters, encoderProperties.getIfAvailable(), customizers)); + ObjectProvider feignHttpMessageConverters) { + PageableSpringEncoder encoder = new PageableSpringEncoder( + springEncoder(formWriterProvider, encoderProperties.getIfAvailable(), feignHttpMessageConverters)); if (dataWebProperties != null) { encoder.setPageParameter(dataWebProperties.getPageable().getPageParameter()); diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/FeignHttpMessageConverters.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/FeignHttpMessageConverters.java new file mode 100644 index 000000000..55a9a1b0b --- /dev/null +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/FeignHttpMessageConverters.java @@ -0,0 +1,93 @@ +/* + * Copyright 2013-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.openfeign.support; + +import java.util.ArrayList; +import java.util.List; + +import org.springframework.beans.factory.ObjectProvider; +import org.springframework.http.MediaType; +import org.springframework.http.converter.HttpMessageConverter; +import org.springframework.http.converter.HttpMessageConverters; +import org.springframework.http.converter.StringHttpMessageConverter; + +/** + * Class that mimics {@link HttpMessageConverters} and the default implementation there. + * Applies the {@link HttpMessageConverterCustomizer}s and gathers all the converters into + * a {@link List}. + */ +public class FeignHttpMessageConverters { + + private final ObjectProvider> messageConverters; + + private final ObjectProvider customizers; + + private List> converters; + + public FeignHttpMessageConverters(ObjectProvider> messageConverters, + ObjectProvider customizers) { + this.messageConverters = messageConverters; + this.customizers = customizers; + } + + public List> getConverters() { + initConvertersIfRequired(); + return converters; + } + + private void initConvertersIfRequired() { + if (this.converters == null) { + this.converters = new ArrayList<>(); + HttpMessageConverters.ClientBuilder builder = HttpMessageConverters.forClient(); + // TODO: allow disabling of registerDefaults + builder.registerDefaults(); + // TODO: check if already added? Howto order? + this.messageConverters.forEach((converter) -> { + if (converter instanceof StringHttpMessageConverter) { + builder.withStringConverter(converter); + } + /* + * else if (converter instanceof + * KotlinSerializationJsonHttpMessageConverter) { + * builder.withKotlinSerializationJsonConverter(converter); } + */ + else if (supportsMediaType(converter, MediaType.APPLICATION_JSON)) { + builder.withJsonConverter(converter); + } + else if (supportsMediaType(converter, MediaType.APPLICATION_XML)) { + builder.withXmlConverter(converter); + } + else { + builder.addCustomConverter(converter); + } + }); + HttpMessageConverters hmc = builder.build(); + hmc.forEach(converter -> converters.add(converter)); + customizers.forEach(customizer -> customizer.accept(this.converters)); + } + } + + private static boolean supportsMediaType(HttpMessageConverter converter, MediaType mediaType) { + for (MediaType supportedMediaType : converter.getSupportedMediaTypes()) { + if (supportedMediaType.equalsTypeAndSubtype(mediaType)) { + return true; + } + } + return false; + } + +} diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringDecoder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringDecoder.java index c79e867c6..57d875f68 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringDecoder.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringDecoder.java @@ -21,7 +21,6 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.lang.reflect.WildcardType; -import java.util.List; import feign.FeignException; import feign.Response; @@ -32,7 +31,6 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatusCode; import org.springframework.http.client.ClientHttpResponse; -import org.springframework.http.converter.HttpMessageConverter; import org.springframework.web.client.HttpMessageConverterExtractor; import static org.springframework.cloud.openfeign.support.FeignUtils.getHttpHeaders; @@ -44,27 +42,18 @@ */ public class SpringDecoder implements Decoder { - private final ObjectProvider> messageConverters; + private final ObjectProvider converters; - private final ObjectProvider customizers; - - public SpringDecoder(ObjectProvider> messageConverters) { - this(messageConverters, new EmptyObjectProvider<>()); - } - - public SpringDecoder(ObjectProvider> messageConverters, - ObjectProvider customizers) { - this.messageConverters = messageConverters; - this.customizers = customizers; + public SpringDecoder(ObjectProvider converters) { + this.converters = converters; } @Override public Object decode(final Response response, Type type) throws IOException, FeignException { if (type instanceof Class || type instanceof ParameterizedType || type instanceof WildcardType) { - List> converters = messageConverters.orderedStream().toList(); - customizers.forEach(customizer -> customizer.accept(converters)); @SuppressWarnings({ "unchecked", "rawtypes" }) - HttpMessageConverterExtractor extractor = new HttpMessageConverterExtractor(type, converters); + HttpMessageConverterExtractor extractor = new HttpMessageConverterExtractor(type, + converters.getObject().getConverters()); return extractor.extractData(new FeignResponseAdapter(response)); } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java index e0722e5a3..1c2194d93 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java @@ -25,7 +25,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.LinkedHashMap; -import java.util.List; import java.util.Objects; import java.util.stream.Stream; @@ -72,29 +71,23 @@ public class SpringEncoder implements Encoder { private final SpringFormEncoder springFormEncoder; - private final ObjectProvider> messageConverters; + private final ObjectProvider converters; private final FeignEncoderProperties encoderProperties; - private final ObjectProvider customizers; - - private List> converters; - - public SpringEncoder(ObjectProvider> messageConverters) { - this(new SpringFormEncoder(), messageConverters, new FeignEncoderProperties(), new EmptyObjectProvider<>()); + public SpringEncoder(ObjectProvider converters) { + this(new SpringFormEncoder(), new FeignEncoderProperties(), converters); } - public SpringEncoder(SpringFormEncoder springFormEncoder, ObjectProvider> messageConverters, - FeignEncoderProperties encoderProperties, ObjectProvider customizers) { + public SpringEncoder(SpringFormEncoder springFormEncoder, FeignEncoderProperties encoderProperties, + ObjectProvider converters) { this.springFormEncoder = springFormEncoder; - this.messageConverters = messageConverters; this.encoderProperties = encoderProperties; - this.customizers = customizers; + this.converters = converters; } @Override public void encode(Object requestBody, Type bodyType, RequestTemplate request) throws EncodeException { - // template.body(conversionService.convert(object, String.class)); if (requestBody != null) { Collection contentTypes = request.headers().get(HttpEncoding.CONTENT_TYPE); @@ -120,8 +113,7 @@ public void encode(Object requestBody, Type bodyType, RequestTemplate request) t private void encodeWithMessageConverter(Object requestBody, Type bodyType, RequestTemplate request, MediaType requestContentType) { - initConvertersIfRequired(); - for (HttpMessageConverter messageConverter : converters) { + for (HttpMessageConverter messageConverter : converters.getObject().getConverters()) { FeignOutputMessage outputMessage; try { if (messageConverter instanceof GenericHttpMessageConverter) { @@ -176,13 +168,6 @@ else if (shouldHaveNullCharset(messageConverter, outputMessage)) { throw new EncodeException(message); } - private void initConvertersIfRequired() { - if (converters == null) { - converters = messageConverters.orderedStream().toList(); - customizers.forEach(customizer -> customizer.accept(converters)); - } - } - private boolean shouldHaveNullCharset(HttpMessageConverter messageConverter, FeignOutputMessage outputMessage) { return binaryContentType(outputMessage) || messageConverter instanceof ByteArrayHttpMessageConverter || messageConverter instanceof ProtobufHttpMessageConverter && ProtobufHttpMessageConverter.PROTOBUF diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/SpringDecoderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/SpringDecoderTests.java index 4c0a55cd7..8814c4c07 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/SpringDecoderTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/SpringDecoderTests.java @@ -22,13 +22,12 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.ObjectProvider; -import org.springframework.boot.http.converter.autoconfigure.HttpMessageConverters; +import org.springframework.cloud.loadbalancer.support.SimpleObjectProvider; +import org.springframework.cloud.openfeign.support.FeignHttpMessageConverters; import org.springframework.cloud.openfeign.support.SpringDecoder; -import org.springframework.http.converter.HttpMessageConverter; import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; /** * Tests for {@link SpringDecoder}. @@ -41,9 +40,9 @@ class SpringDecoderTests { @BeforeEach void setUp() { - ObjectProvider> factory = mock(); - when(factory.orderedStream()).thenReturn(new HttpMessageConverters().getConverters().stream()); - decoder = new SpringDecoder(factory); + ObjectProvider converters = new SimpleObjectProvider<>( + new FeignHttpMessageConverters(mock(), mock())); + decoder = new SpringDecoder(converters); } // Issue: https://github.com/spring-cloud/spring-cloud-openfeign/issues/972 diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/proto/ProtobufNotInClasspathTest.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/proto/ProtobufNotInClasspathTest.java index e40039635..c6f63d660 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/proto/ProtobufNotInClasspathTest.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/proto/ProtobufNotInClasspathTest.java @@ -22,6 +22,8 @@ import org.junit.jupiter.api.Test; import org.springframework.beans.factory.ObjectProvider; +import org.springframework.cloud.loadbalancer.support.SimpleObjectProvider; +import org.springframework.cloud.openfeign.support.FeignHttpMessageConverters; import org.springframework.cloud.openfeign.support.SpringEncoder; import org.springframework.cloud.test.ClassPathExclusions; import org.springframework.http.converter.HttpMessageConverter; @@ -46,7 +48,8 @@ void testEncodeWhenProtobufNotInClasspath() { when(factory.orderedStream()).thenReturn(protobufHttpMessageConverters.stream()); RequestTemplate requestTemplate = new RequestTemplate(); requestTemplate.method(POST); - new SpringEncoder(factory).encode("a=b", String.class, requestTemplate); + new SpringEncoder(new SimpleObjectProvider<>(new FeignHttpMessageConverters(factory, mock()))).encode("a=b", + String.class, requestTemplate); } } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/proto/ProtobufSpringEncoderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/proto/ProtobufSpringEncoderTests.java index b66e4d9d0..2607a0897 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/proto/ProtobufSpringEncoderTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/encoding/proto/ProtobufSpringEncoderTests.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import java.util.stream.Stream; import com.google.protobuf.InvalidProtocolBufferException; import feign.RequestTemplate; @@ -43,6 +44,8 @@ import org.mockito.stubbing.Answer; import org.springframework.beans.factory.ObjectProvider; +import org.springframework.cloud.loadbalancer.support.SimpleObjectProvider; +import org.springframework.cloud.openfeign.support.FeignHttpMessageConverters; import org.springframework.cloud.openfeign.support.SpringEncoder; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.protobuf.ProtobufHttpMessageConverter; @@ -51,7 +54,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.fail; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; /** * Test {@link SpringEncoder} with {@link ProtobufHttpMessageConverter} @@ -111,10 +113,14 @@ void testProtobufWithCharsetWillFail() throws IOException { } private SpringEncoder newEncoder() { - ObjectProvider> factory = mock(); - List> protobufHttpMessageConverters = List.of(new ProtobufHttpMessageConverter()); - when(factory.orderedStream()).thenReturn(protobufHttpMessageConverters.stream()); - return new SpringEncoder(factory); + ObjectProvider> factory = new SimpleObjectProvider<>( + new ProtobufHttpMessageConverter()) { + @Override + public Stream> stream() { + return Stream.of(getObject()); + } + }; + return new SpringEncoder(new SimpleObjectProvider<>(new FeignHttpMessageConverters(factory, mock()))); } private RequestTemplate newRequestTemplate() { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java index f81b994b7..f4af2ce89 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java @@ -27,7 +27,6 @@ import feign.codec.EncodeException; import feign.codec.Encoder; import org.assertj.core.api.Assertions; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -40,6 +39,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; import org.springframework.core.ParameterizedTypeReference; import org.springframework.core.io.Resource; import org.springframework.http.HttpInputMessage; @@ -100,14 +100,18 @@ void testCustomHttpMessageConverter() { encoder.encode("hi", MyType.class, request); Collection contentTypeHeader = request.headers().get("Content-Type"); - assertThat(contentTypeHeader).as("missing content type header").isNotNull(); - assertThat(contentTypeHeader.isEmpty()).as("missing content type header").isFalse(); + assertThat(contentTypeHeader).as("missing content type header") + .isNotNull() + .as("missing content type header") + .isNotEmpty(); String header = contentTypeHeader.iterator().next(); assertThat(header).as("content type header is wrong").isEqualTo("application/mytype"); - assertThat(request.requestCharset()).as("request charset is null").isNotNull(); - assertThat(request.requestCharset()).as("request charset is wrong").isEqualTo(StandardCharsets.UTF_8); + assertThat(request.requestCharset()).as("request charset is null") + .isNotNull() + .as("request charset is wrong") + .isEqualTo(StandardCharsets.UTF_8); } // gh-225 @@ -124,17 +128,20 @@ void testCustomGenericHttpMessageConverter() { encoder.encode(Collections.singletonList("hi"), stringListType.getType(), request); Collection contentTypeHeader = request.headers().get("Content-Type"); - assertThat(contentTypeHeader).as("missing content type header").isNotNull(); - assertThat(contentTypeHeader.isEmpty()).as("missing content type header").isFalse(); + assertThat(contentTypeHeader).as("missing content type header") + .isNotNull() + .as("missing content type header") + .isNotEmpty(); String header = contentTypeHeader.iterator().next(); assertThat(header).as("content type header is wrong").isEqualTo("application/mygenerictype"); - assertThat(request.requestCharset()).as("request charset is null").isNotNull(); - assertThat(request.requestCharset()).as("request charset is wrong").isEqualTo(StandardCharsets.UTF_8); + assertThat(request.requestCharset()).as("request charset is null") + .isNotNull() + .as("request charset is wrong") + .isEqualTo(StandardCharsets.UTF_8); } - @Disabled("FIXME: https://github.com/spring-cloud/spring-cloud-openfeign/issues/1269") @Test void testBinaryData() { Encoder encoder = this.context.getInstance("foo", Encoder.class); @@ -175,8 +182,8 @@ void testMultipartFile2() { assertThat((String) ((List) request.headers().get(CONTENT_TYPE)).get(0)) .as("Request Content-Type is not multipart/form-data") .contains("multipart/form-data; charset=UTF-8; boundary="); - assertThat(request.headers().get(CONTENT_TYPE).size()).as("There is more than one Content-Type request header") - .isEqualTo(1); + assertThat(request.headers().get(CONTENT_TYPE)).as("There is more than one Content-Type request header") + .hasSize(1); assertThat(((List) request.headers().get(ACCEPT)).get(0)).as("Request Accept header is not multipart/form-data") .isEqualTo(MULTIPART_FORM_DATA_VALUE); assertThat(((List) request.headers().get(CONTENT_LENGTH)).get(0)) @@ -260,12 +267,18 @@ GenericHttpMessageConverter myGenericHttpMessageConverter() { return new MyGenericHttpMessageConverter(); } - private static class MyHttpMessageConverter extends AbstractGenericHttpMessageConverter { + private static class MyHttpMessageConverter extends AbstractGenericHttpMessageConverter + implements Ordered { MyHttpMessageConverter() { super(new MediaType("application", "mytype")); } + @Override + public int getOrder() { + return Ordered.HIGHEST_PRECEDENCE; + } + @Override protected boolean supports(Class clazz) { return false;