Update SNAPSHOT to 2.1.5.RELEASE

* 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>

.circleci/config.yml

@@ -9,7 +9,6 @@ jobs:
     branches:
       ignore:
         - gh-pages # list of branches to ignore
-    resource_class: large
     steps:
       - checkout
       - restore_cache:
@@ -36,4 +35,4 @@ jobs:
           destination: artifacts
       - store_test_results:
           path: /junit/
-          destination: testartifacts
+          destination: testartifacts

README.adoc

@@ -286,7 +286,7 @@ image::https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/{spring
 
 Go to `File` -> `Settings` -> `Other settings` -> `Checkstyle`. There click on the `+` icon in the `Configuration file` section. There, you'll have to define where the checkstyle rules should be picked from. In the image above, we've picked the rules from the cloned Spring Cloud Build repository. However, you can point to the Spring Cloud Build's GitHub repository (e.g. for the `checkstyle.xml` : `https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/master/spring-cloud-build-tools/src/main/resources/checkstyle.xml`). We need to provide the following variables:
 
-- `checkstyle.header.file` - please point it to the Spring Cloud Build's, `spring-cloud-build-tools/src/main/resources/checkstyle/checkstyle-header.txt` file either in your cloned repo or via the `https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/master/spring-cloud-build-tools/src/main/resources/checkstyle-header.txt` URL.
+- `checkstyle.header.file` - please point it to the Spring Cloud Build's, `spring-cloud-build-tools/src/main/resources/checkstyle-header.txt` file either in your cloned repo or via the `https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/master/spring-cloud-build-tools/src/main/resources/checkstyle-header.txt` URL.
 - `checkstyle.suppressions.file` - default suppressions. Please point it to the Spring Cloud Build's, `spring-cloud-build-tools/src/checkstyle/checkstyle-suppressions.xml` file either in your cloned repo or via the `https://raw.githubusercontent.com/spring-cloud/spring-cloud-build/master/spring-cloud-build-tools/src/checkstyle/checkstyle-suppressions.xml` URL.
 - `checkstyle.additional.suppressions.file` - this variable corresponds to suppressions in your local project. E.g. you're working on `spring-cloud-contract`. Then point to the `project-root/src/checkstyle/checkstyle-suppressions.xml` folder. Example for `spring-cloud-contract` would be: `/home/username/spring-cloud-contract/src/checkstyle/checkstyle-suppressions.xml`.
 

docs/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-openfeign</artifactId>
-		<version>2.1.3.RELEASE</version>
+		<version>2.1.5.RELEASE</version>
 	</parent>
 	<artifactId>spring-cloud-openfeign-docs</artifactId>
 	<packaging>pom</packaging>

docs/src/main/asciidoc/appendix.adoc

@@ -0,0 +1,14 @@
+:numbered!:
+[appendix]
+[[common-application-properties]]
+== Common application properties
+
+include::_attributes.adoc[]
+
+Various properties can be specified inside your `application.properties` file, inside your `application.yml` file, or as command line switches.
+This appendix provides a list of common {project-full-name} properties and references to the underlying classes that consume them.
+
+NOTE: Property contributions can come from additional jar files on your classpath, so you should not consider this an exhaustive list.
+Also, you can define your own properties.
+
+include::_configprops.adoc[]

docs/src/main/asciidoc/spring-cloud-openfeign.adoc

@@ -487,3 +487,16 @@ public class DemoTemplate {
     String demoEndpoint(@SpringQueryMap Params params);
 }
 ----
+
+=== Troubleshooting
+
+==== Early Initialization Errors
+
+Depending on how you are using your Feign clients you may see initialization errors when starting your application.
+To work around this problem you can use an `ObjectProvider` when autowiring your client.
+
+[source,java,indent=0]
+----
+@Autowired
+ObjectProvider<TestFeginClient> testFeginClient;
+----

pom.xml

@@ -4,14 +4,14 @@
 		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
 	<modelVersion>4.0.0</modelVersion>
 	<artifactId>spring-cloud-openfeign</artifactId>
-	<version>2.1.3.RELEASE</version>
+	<version>2.1.5.RELEASE</version>
 	<packaging>pom</packaging>
 	<name>Spring Cloud OpenFeign</name>
 	<description>Spring Cloud OpenFeign</description>
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-build</artifactId>
-		<version>2.1.7.RELEASE</version>
+		<version>2.1.10.RELEASE</version>
 		<relativePath/>
 	</parent>
 	<scm>
@@ -26,8 +26,8 @@
 	<properties>
 		<main.basedir>${basedir}</main.basedir>
 		<jackson.version>2.7.3</jackson.version>
-		<spring-cloud-commons.version>2.1.3.RELEASE</spring-cloud-commons.version>
-		<spring-cloud-netflix.version>2.1.3.RELEASE</spring-cloud-netflix.version>
+		<spring-cloud-commons.version>2.1.5.RELEASE</spring-cloud-commons.version>
+		<spring-cloud-netflix.version>2.1.5.RELEASE</spring-cloud-netflix.version>
 
 		<!-- Plugin versions -->
 		<maven-compiler-plugin.version>3.6.1</maven-compiler-plugin.version>

spring-cloud-openfeign-core/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-openfeign</artifactId>
-		<version>2.1.3.RELEASE</version>
+		<version>2.1.5.RELEASE</version>
 		<relativePath>..</relativePath> <!-- lookup parent from repository -->
 	</parent>
 	<artifactId>spring-cloud-openfeign-core</artifactId>

spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/HystrixTargeter.java

@@ -22,6 +22,8 @@ import feign.hystrix.FallbackFactory;
 import feign.hystrix.HystrixFeign;
 import feign.hystrix.SetterFactory;
 
+import org.springframework.util.StringUtils;
+
 /**
  * @author Spencer Gibb
  * @author Erik Kringen
@@ -36,19 +38,19 @@ class HystrixTargeter implements Targeter {
 			return feign.target(target);
 		}
 		feign.hystrix.HystrixFeign.Builder builder = (feign.hystrix.HystrixFeign.Builder) feign;
-		SetterFactory setterFactory = getOptional(factory.getName(), context,
-				SetterFactory.class);
+		String name = StringUtils.isEmpty(factory.getContextId()) ? factory.getName()
+				: factory.getContextId();
+		SetterFactory setterFactory = getOptional(name, context, SetterFactory.class);
 		if (setterFactory != null) {
 			builder.setterFactory(setterFactory);
 		}
 		Class<?> fallback = factory.getFallback();
 		if (fallback != void.class) {
-			return targetWithFallback(factory.getName(), context, target, builder,
-					fallback);
+			return targetWithFallback(name, context, target, builder, fallback);
 		}
 		Class<?> fallbackFactory = factory.getFallbackFactory();
 		if (fallbackFactory != void.class) {
-			return targetWithFallbackFactory(factory.getName(), context, target, builder,
+			return targetWithFallbackFactory(name, context, target, builder,
 					fallbackFactory);
 		}
 

spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringDecoder.java

@@ -59,7 +59,8 @@ public class SpringDecoder implements Decoder {
 			return extractor.extractData(new FeignResponseAdapter(response));
 		}
 		throw new DecodeException(response.status(),
-				"type is not an instance of Class or ParameterizedType: " + type);
+				"type is not an instance of Class or ParameterizedType: " + type,
+				response.request());
 	}
 
 	private final class FeignResponseAdapter implements ClientHttpResponse {

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

@@ -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();

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

@@ -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();
-		}
-
-	}
-
 }

spring-cloud-openfeign-dependencies/pom.xml

@@ -6,16 +6,16 @@
 	<parent>
 		<artifactId>spring-cloud-dependencies-parent</artifactId>
 		<groupId>org.springframework.cloud</groupId>
-		<version>2.1.7.RELEASE</version>
+		<version>2.1.10.RELEASE</version>
 		<relativePath/>
 	</parent>
 	<artifactId>spring-cloud-openfeign-dependencies</artifactId>
-	<version>2.1.3.RELEASE</version>
+	<version>2.1.5.RELEASE</version>
 	<packaging>pom</packaging>
 	<name>spring-cloud-openfeign-dependencies</name>
 	<description>Spring Cloud OpenFeign Dependencies</description>
 	<properties>
-		<feign.version>10.2.3</feign.version>
+		<feign.version>10.4.0</feign.version>
 		<feign-form.version>3.8.0</feign-form.version>
 	</properties>
 	<dependencyManagement>

spring-cloud-starter-openfeign/pom.xml

@@ -5,7 +5,7 @@
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-openfeign</artifactId>
-		<version>2.1.3.RELEASE</version>
+		<version>2.1.5.RELEASE</version>
 		<relativePath>..</relativePath>
 	</parent>
 	<artifactId>spring-cloud-starter-openfeign</artifactId>

spring-cloud-starter-openfeign/src/main/resources/META-INF/spring.provides

@@ -1 +0,0 @@
-provides: spring-cloud-openfeign-core