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

Merge fix 255 to 21x (#273)

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

* Fix after cherry-pick.

* Add Ahmad Mozafarnia to authors.

* Refactor tests.

Co-authored-by: Ahmad Mozafarnia <ahmad.mozafarnia@gmail.com>
This commit is contained in:
Olga Maciaszek-Sharma
2020-01-09 17:59:15 +01:00
committed by GitHub
parent f97cbb9a73
commit 80d2067134
2 changed files with 169 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -35,10 +35,12 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.ObjectFactory;
import org.springframework.boot.autoconfigure.http.HttpMessageConverters;
import org.springframework.cloud.openfeign.encoding.HttpEncoding;
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;
@@ -49,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 {
@@ -67,8 +70,8 @@ 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("Content-Type");
Collection<String> contentTypes = request.headers()
.get(HttpEncoding.CONTENT_TYPE);
MediaType requestContentType = null;
if (contentTypes != null && !contentTypes.isEmpty()) {
@@ -89,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
@@ -139,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 + "]";
}
@@ -147,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();

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

@@ -16,10 +16,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.nio.charset.StandardCharsets;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import feign.RequestTemplate;
@@ -27,7 +28,6 @@ import feign.codec.EncodeException;
import feign.codec.Encoder;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentMatcher;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
@@ -35,12 +35,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 +63,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, webEnvironment = WebEnvironment.RANDOM_PORT, value = {
@@ -74,6 +78,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);
@@ -93,7 +101,35 @@ public class SpringEncoderTests {
assertThat(request.requestCharset()).as("request charset is null").isNotNull();
assertThat(request.requestCharset()).as("request charset is wrong")
.isEqualTo(Charset.forName("UTF-8"));
.isEqualTo(StandardCharsets.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(StandardCharsets.UTF_8);
}
@Test
@@ -178,6 +214,11 @@ public class SpringEncoderTests {
return new MyHttpMessageConverter();
}
@Bean
GenericHttpMessageConverter<?> myGenericHttpMessageConverter() {
return new MyGenericHttpMessageConverter();
}
private static class MyHttpMessageConverter
extends AbstractGenericHttpMessageConverter<Object> {
@@ -203,20 +244,77 @@ public class SpringEncoderTests {
@Override
protected void writeInternal(Object o, Type type,
HttpOutputMessage outputMessage)
throws IOException, HttpMessageNotWritableException {
throws HttpMessageNotWritableException {
}
@Override
protected Object readInternal(Class<?> clazz, HttpInputMessage inputMessage)
throws IOException, HttpMessageNotReadableException {
throws HttpMessageNotReadableException {
return null;
}
@Override
public Object read(Type type, Class<?> contextClass,
HttpInputMessage inputMessage)
throws IOException, HttpMessageNotReadableException {
throws HttpMessageNotReadableException {
return null;
}
}
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 HttpMessageNotWritableException {
}
@Override
public Object read(Type type, Class<?> contextClass,
HttpInputMessage inputMessage)
throws HttpMessageNotReadableException {
return null;
}
@Override
protected Object readInternal(Class<?> clazz, HttpInputMessage inputMessage)
throws HttpMessageNotReadableException {
return null;
}
@@ -224,27 +322,4 @@ public class SpringEncoderTests {
}
class MediaTypeMatcher implements ArgumentMatcher<MediaType> {
private MediaType mediaType;
MediaTypeMatcher(String type, String subtype) {
this.mediaType = new MediaType(type, subtype);
}
@Override
public boolean matches(MediaType argument) {
return this.mediaType.equals(argument);
}
@Override
public String toString() {
final StringBuffer sb = new StringBuffer("MediaTypeMatcher{");
sb.append("mediaType=").append(this.mediaType);
sb.append('}');
return sb.toString();
}
}
}