From e0c67b30f88c1946175202c05bc9d60693b23ee4 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Wed, 20 Apr 2016 06:46:23 -0600 Subject: [PATCH] Pass headers back to feign request. Spring HttpMessageConverters can modify headers. This change passes the modified headersr back to the feign request, so the proper headers are sent on the request. fixes gh-977 --- .../netflix/feign/support/FeignUtils.java | 12 ++ .../netflix/feign/support/SpringEncoder.java | 20 ++- .../feign/support/SpringEncoderTests.java | 154 ++++++++++++++++++ .../feign/valid/FeignHttpClientTests.java | 2 +- 4 files changed, 180 insertions(+), 8 deletions(-) create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringEncoderTests.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FeignUtils.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FeignUtils.java index c7b6526a..bb0440fb 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FeignUtils.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FeignUtils.java @@ -18,6 +18,8 @@ package org.springframework.cloud.netflix.feign.support; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import org.springframework.http.HttpHeaders; @@ -35,4 +37,14 @@ public class FeignUtils { return httpHeaders; } + static Map> getHeaders(HttpHeaders httpHeaders) { + LinkedHashMap> headers = new LinkedHashMap<>(); + + for (Map.Entry> entry : httpHeaders.entrySet()) { + headers.put(entry.getKey(), entry.getValue()); + } + + return headers; + } + } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringEncoder.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringEncoder.java index 939995c8..df43302f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringEncoder.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringEncoder.java @@ -16,8 +16,6 @@ package org.springframework.cloud.netflix.feign.support; -import static org.springframework.cloud.netflix.feign.support.FeignUtils.getHttpHeaders; - import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; @@ -25,8 +23,6 @@ import java.lang.reflect.Type; import java.nio.charset.Charset; import java.util.Collection; -import lombok.extern.apachecommons.CommonsLog; - import org.springframework.beans.factory.ObjectFactory; import org.springframework.boot.autoconfigure.web.HttpMessageConverters; import org.springframework.http.HttpHeaders; @@ -37,6 +33,10 @@ import org.springframework.http.converter.HttpMessageConverter; import feign.RequestTemplate; import feign.codec.EncodeException; import feign.codec.Encoder; +import lombok.extern.apachecommons.CommonsLog; + +import static org.springframework.cloud.netflix.feign.support.FeignUtils.getHeaders; +import static org.springframework.cloud.netflix.feign.support.FeignUtils.getHttpHeaders; /** * @author Spencer Gibb @@ -89,6 +89,12 @@ public class SpringEncoder implements Encoder { catch (IOException ex) { throw new EncodeException("Error converting request body", ex); } + // clear headers + request.headers(null); + // converters can modify headers, so update the request + // with the modified headers + request.headers(getHeaders(outputMessage.getHeaders())); + request.body(outputMessage.getOutputStream().toByteArray(), Charset.forName("UTF-8")); // TODO: set charset return; @@ -107,10 +113,10 @@ public class SpringEncoder implements Encoder { private final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - private final RequestTemplate request; + private final HttpHeaders httpHeaders; private FeignOutputMessage(RequestTemplate request) { - this.request = request; + httpHeaders = getHttpHeaders(request.headers()); } @Override @@ -120,7 +126,7 @@ public class SpringEncoder implements Encoder { @Override public HttpHeaders getHeaders() { - return getHttpHeaders(this.request.headers()); + return this.httpHeaders; } public ByteArrayOutputStream getOutputStream() { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringEncoderTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringEncoderTests.java new file mode 100644 index 00000000..a101e691 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringEncoderTests.java @@ -0,0 +1,154 @@ +package org.springframework.cloud.netflix.feign.support; + +import java.io.IOException; +import java.lang.reflect.Type; +import java.util.Collection; + +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; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.IntegrationTest; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.cloud.netflix.feign.FeignContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +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.HttpMessageConverter; +import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.http.converter.HttpMessageNotWritableException; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.web.bind.annotation.RestController; + +import feign.RequestTemplate; +import lombok.Data; + +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.junit.Assert.assertThat; + +/** + * @author Spencer Gibb + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = SpringEncoderTests.Application.class) +@WebAppConfiguration +@IntegrationTest({ "server.port=0", "spring.application.name=springencodertest", + "spring.jmx.enabled=true" }) +@DirtiesContext +public class SpringEncoderTests { + + @Autowired + private FeignContext context; + + @Autowired + @Qualifier("myHttpMessageConverter") + private HttpMessageConverter myConverter; + + @Test + public void testCustomHttpMessageConverter() { + SpringEncoder encoder = this.context.getInstance("foo", SpringEncoder.class); + assertThat(encoder, is(notNullValue())); + RequestTemplate request = new RequestTemplate(); + + encoder.encode("hi", MyType.class, request); + + Collection contentTypeHeader = request.headers().get("Content-Type"); + assertThat("missing content type header", contentTypeHeader, is(notNullValue())); + assertThat("missing content type header", contentTypeHeader.isEmpty(), is(false)); + + String header = contentTypeHeader.iterator().next(); + assertThat("content type header is wrong", header, is("application/mytype")); + } + + class MediaTypeMatcher extends ArgumentMatcher { + + private MediaType mediaType; + + public MediaTypeMatcher(String type, String subtype) { + mediaType = new MediaType(type, subtype); + } + + @Override + public boolean matches(Object argument) { + if (argument instanceof MediaType) { + MediaType other = (MediaType) argument; + return mediaType.equals(other); + } + return false; + } + + @Override + public String toString() { + final StringBuffer sb = new StringBuffer("MediaTypeMatcher{"); + sb.append("mediaType=").append(mediaType); + sb.append('}'); + return sb.toString(); + } + } + + @Data + protected static class MyType { + private String value; + } + + protected interface TestClient { + + } + + @Configuration + @EnableAutoConfiguration + @RestController + protected static class Application implements TestClient { + + @Bean + HttpMessageConverter myHttpMessageConverter() { + return new MyHttpMessageConverter(); + } + + private static class MyHttpMessageConverter extends AbstractGenericHttpMessageConverter { + + public MyHttpMessageConverter() { + super(new MediaType("application", "mytype")); + } + + @Override + protected boolean supports(Class clazz) { + return false; + } + + @Override + public boolean canRead(Class clazz, MediaType mediaType) { + return true; + } + + @Override + public boolean canWrite(Class clazz, MediaType mediaType) { + return true; + } + + @Override + protected void writeInternal(Object o, Type type, HttpOutputMessage outputMessage) throws IOException, HttpMessageNotWritableException { + + } + + @Override + protected Object readInternal(Class clazz, HttpInputMessage inputMessage) throws IOException, HttpMessageNotReadableException { + return null; + } + + @Override + public Object read(Type type, Class contextClass, HttpInputMessage inputMessage) throws IOException, HttpMessageNotReadableException { + return null; + } + } + } + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java index b46cab20..43dc9c95 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java @@ -86,7 +86,7 @@ public class FeignHttpClientTests { @RequestMapping(method = RequestMethod.GET, value = "/hello") Hello getHello(); - @RequestMapping(method = RequestMethod.PATCH, value = "/hellop") + @RequestMapping(method = RequestMethod.PATCH, value = "/hellop", consumes = "application/json") ResponseEntity patchHello(Hello hello); }