SpringCloud/spring-cloud-openfeign
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge fix gh 225 (#272)

Browse Source 
* 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 <ahmad.mozafarnia@gmail.com>
This commit is contained in:
Olga Maciaszek-Sharma
2020-01-09 15:16:51 +01:00
committed by GitHub
parent 2b364367cb
commit 887ceefa60
2 changed files with 161 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

85
spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringEncoder.java
View File

@@ -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<String> 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<Object> copy = (HttpMessageConverter<Object>) 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();

100
spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringEncoderTests.java
View File

@@ -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<List<String>> stringListType = new ParameterizedTypeReference<List<String>>() {
};
request.header(HttpEncoding.CONTENT_TYPE, "application/mygenerictype");
encoder.encode(Collections.singletonList("hi"), stringListType.getType(),
request);
Collection<String> 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<Object> {
@@ -223,6 +266,63 @@ public class SpringEncoderTests {
}
private static class MyGenericHttpMessageConverter
extends AbstractGenericHttpMessageConverter<Object> {
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<MediaType> {