From 887ceefa606239d684b78c9394cca8ce71dcaa67 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Thu, 9 Jan 2020 15:16:51 +0100 Subject: [PATCH] Merge fix gh 225 (#272) * Fix bug in SpringEncoder (#225) This commit fixes a bug in the SpringEncoder that causes the bodyType to be ignored and not passed to generic message converters. Fixes gh-225 * Refactor changes based on Olga Maciaszek's review * Add Ahmad Mozafarnia to authors. Co-authored-by: Ahmad Mozafarnia --- .../openfeign/support/SpringEncoder.java | 85 ++++++++++----- .../openfeign/support/SpringEncoderTests.java | 100 ++++++++++++++++++ 2 files changed, 161 insertions(+), 24 deletions(-) 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 4bcf9a5f..bb82c76c 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 @@ -40,6 +40,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpOutputMessage; import org.springframework.http.MediaType; import org.springframework.http.converter.ByteArrayHttpMessageConverter; +import org.springframework.http.converter.GenericHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.protobuf.ProtobufHttpMessageConverter; import org.springframework.web.multipart.MultipartFile; @@ -50,6 +51,7 @@ import static org.springframework.cloud.openfeign.support.FeignUtils.getHttpHead /** * @author Spencer Gibb * @author Scien Jus + * @author Ahmad Mozafarnia */ public class SpringEncoder implements Encoder { @@ -68,7 +70,6 @@ public class SpringEncoder implements Encoder { throws EncodeException { // template.body(conversionService.convert(object, String.class)); if (requestBody != null) { - Class requestType = requestBody.getClass(); Collection contentTypes = request.headers() .get(HttpEncoding.CONTENT_TYPE); @@ -91,31 +92,24 @@ public class SpringEncoder implements Encoder { } } - for (HttpMessageConverter messageConverter : this.messageConverters + for (HttpMessageConverter messageConverter : this.messageConverters .getObject().getConverters()) { - if (messageConverter.canWrite(requestType, requestContentType)) { - if (log.isDebugEnabled()) { - if (requestContentType != null) { - log.debug("Writing [" + requestBody + "] as \"" - + requestContentType + "\" using [" + messageConverter - + "]"); - } - else { - log.debug("Writing [" + requestBody + "] using [" - + messageConverter + "]"); - } - + FeignOutputMessage outputMessage; + try { + if (messageConverter instanceof GenericHttpMessageConverter) { + outputMessage = checkAndWrite(requestBody, bodyType, + requestContentType, + (GenericHttpMessageConverter) messageConverter, request); } - - FeignOutputMessage outputMessage = new FeignOutputMessage(request); - try { - @SuppressWarnings("unchecked") - HttpMessageConverter copy = (HttpMessageConverter) messageConverter; - copy.write(requestBody, requestContentType, outputMessage); - } - catch (IOException ex) { - throw new EncodeException("Error converting request body", ex); + else { + outputMessage = checkAndWrite(requestBody, requestContentType, + messageConverter, request); } + } + catch (IOException ex) { + throw new EncodeException("Error converting request body", ex); + } + if (outputMessage != null) { // clear headers request.headers(null); // converters can modify headers, so update the request @@ -141,7 +135,7 @@ public class SpringEncoder implements Encoder { } } String message = "Could not write request: no suitable HttpMessageConverter " - + "found for request type [" + requestType.getName() + "]"; + + "found for request type [" + requestBody.getClass().getName() + "]"; if (requestContentType != null) { message += " and content type [" + requestContentType + "]"; } @@ -149,6 +143,49 @@ public class SpringEncoder implements Encoder { } } + @SuppressWarnings("unchecked") + private FeignOutputMessage checkAndWrite(Object body, MediaType contentType, + HttpMessageConverter converter, RequestTemplate request) throws IOException { + if (converter.canWrite(body.getClass(), contentType)) { + logBeforeWrite(body, contentType, converter); + FeignOutputMessage outputMessage = new FeignOutputMessage(request); + converter.write(body, contentType, outputMessage); + return outputMessage; + } + else { + return null; + } + } + + @SuppressWarnings("unchecked") + private FeignOutputMessage checkAndWrite(Object body, Type genericType, + MediaType contentType, GenericHttpMessageConverter converter, + RequestTemplate request) throws IOException { + if (converter.canWrite(genericType, body.getClass(), contentType)) { + logBeforeWrite(body, contentType, converter); + FeignOutputMessage outputMessage = new FeignOutputMessage(request); + converter.write(body, genericType, contentType, outputMessage); + return outputMessage; + } + else { + return null; + } + } + + private void logBeforeWrite(Object requestBody, MediaType requestContentType, + HttpMessageConverter messageConverter) { + if (log.isDebugEnabled()) { + if (requestContentType != null) { + log.debug("Writing [" + requestBody + "] as \"" + requestContentType + + "\" using [" + messageConverter + "]"); + } + else { + log.debug( + "Writing [" + requestBody + "] using [" + messageConverter + "]"); + } + } + } + private final class FeignOutputMessage implements HttpOutputMessage { private final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); 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 1ae4db0a..154e59fa 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 @@ -17,9 +17,11 @@ package org.springframework.cloud.openfeign.support; import java.io.IOException; +import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.nio.charset.Charset; import java.util.Collection; +import java.util.Collections; import java.util.List; import feign.RequestTemplate; @@ -35,12 +37,15 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.cloud.openfeign.FeignContext; +import org.springframework.cloud.openfeign.encoding.HttpEncoding; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.ParameterizedTypeReference; import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpOutputMessage; import org.springframework.http.MediaType; import org.springframework.http.converter.AbstractGenericHttpMessageConverter; +import org.springframework.http.converter.GenericHttpMessageConverter; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.http.converter.HttpMessageNotWritableException; @@ -60,6 +65,7 @@ import static org.springframework.http.MediaType.MULTIPART_FORM_DATA_VALUE; /** * @author Spencer Gibb * @author Olga Maciaszek-Sharma + * @author Ahmad Mozafarnia */ @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = SpringEncoderTests.Application.class, @@ -75,6 +81,10 @@ public class SpringEncoderTests { @Qualifier("myHttpMessageConverter") private HttpMessageConverter myConverter; + @Autowired + @Qualifier("myGenericHttpMessageConverter") + private GenericHttpMessageConverter myGenericConverter; + @Test public void testCustomHttpMessageConverter() { Encoder encoder = this.context.getInstance("foo", Encoder.class); @@ -97,6 +107,34 @@ public class SpringEncoderTests { .isEqualTo(Charset.forName("UTF-8")); } + // gh-225 + @Test + public void testCustomGenericHttpMessageConverter() { + Encoder encoder = this.context.getInstance("foo", Encoder.class); + assertThat(encoder).isNotNull(); + RequestTemplate request = new RequestTemplate(); + + ParameterizedTypeReference> stringListType = new ParameterizedTypeReference>() { + }; + + request.header(HttpEncoding.CONTENT_TYPE, "application/mygenerictype"); + 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(); + + 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(Charset.forName("UTF-8")); + } + @Test public void testBinaryData() { Encoder encoder = this.context.getInstance("foo", Encoder.class); @@ -179,6 +217,11 @@ public class SpringEncoderTests { return new MyHttpMessageConverter(); } + @Bean + GenericHttpMessageConverter myGenericHttpMessageConverter() { + return new MyGenericHttpMessageConverter(); + } + private static class MyHttpMessageConverter extends AbstractGenericHttpMessageConverter { @@ -223,6 +266,63 @@ public class SpringEncoderTests { } + private static class MyGenericHttpMessageConverter + extends AbstractGenericHttpMessageConverter { + + MyGenericHttpMessageConverter() { + super(new MediaType("application", "mygenerictype")); + } + + private boolean isStringList(Type type) { + if (type instanceof ParameterizedType) { + ParameterizedType parameterizedType = (ParameterizedType) type; + return parameterizedType.getRawType() == List.class + && parameterizedType + .getActualTypeArguments()[0] == String.class; + } + else { + return false; + } + } + + @Override + protected boolean supports(Class clazz) { + return clazz == List.class; + } + + @Override + public boolean canWrite(Type type, Class clazz, MediaType mediaType) { + return canWrite(mediaType) && isStringList(type); + } + + @Override + public boolean canRead(Type type, Class contextClass, + MediaType mediaType) { + return canRead(mediaType) && isStringList(type); + } + + @Override + protected void writeInternal(Object o, Type type, + HttpOutputMessage outputMessage) + throws IOException, HttpMessageNotWritableException { + + } + + @Override + public Object read(Type type, Class contextClass, + HttpInputMessage inputMessage) + throws IOException, HttpMessageNotReadableException { + return null; + } + + @Override + protected Object readInternal(Class clazz, HttpInputMessage inputMessage) + throws IOException, HttpMessageNotReadableException { + return null; + } + + } + } class MediaTypeMatcher implements ArgumentMatcher {